Blame SOURCES/gdb-rhbz2022177-dprintf-1.patch

ab2726
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
ab2726
From: Kevin Buettner <kevinb@redhat.com>
ab2726
Date: Wed, 10 Nov 2021 18:52:22 -0700
ab2726
Subject: gdb-rhbz2022177-dprintf-1.patch
ab2726
ab2726
;; Backport fix for dprintf bug (RH BZ 2022177).
ab2726
ab2726
Fix PR 28308 - dprintf breakpoints not working when run from script
ab2726
ab2726
This commit fixes Bug 28308, titled "Strange interactions with
ab2726
dprintf and break/commands":
ab2726
ab2726
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28308
ab2726
ab2726
Since creating that bug report, I've found a somewhat simpler way of
ab2726
reproducing the problem.  I've encapsulated it into the GDB test case
ab2726
which I've created along with this bug fix.  The name of the new test
ab2726
is gdb.base/dprintf-execution-x-script.exp, I'll demonstrate the
ab2726
problem using this test case, though for brevity, I've placed all
ab2726
relevant files in the same directory and have renamed the files to all
ab2726
start with 'dp-bug' instead of 'dprintf-execution-x-script'.
ab2726
ab2726
The script file, named dp-bug.gdb, consists of the following commands:
ab2726
ab2726
dprintf increment, "dprintf in increment(), vi=%d\n", vi
ab2726
break inc_vi
ab2726
commands
ab2726
  continue
ab2726
end
ab2726
run
ab2726
ab2726
Note that the final command in this script is 'run'.  When 'run' is
ab2726
instead issued interactively, the  bug does not occur.  So, let's look
ab2726
at the interactive case first in order to see the correct/expected
ab2726
output:
ab2726
ab2726
$ gdb -q -x dp-bug.gdb dp-bug
ab2726
... eliding buggy output which I'll discuss later ...
ab2726
(gdb) run
ab2726
Starting program: /mesquite2/sourceware-git/f34-master/bld/gdb/tmp/dp-bug
ab2726
vi=0
ab2726
dprintf in increment(), vi=0
ab2726
ab2726
Breakpoint 2, inc_vi () at dprintf-execution-x-script.c:26
ab2726
26	in dprintf-execution-x-script.c
ab2726
vi=1
ab2726
dprintf in increment(), vi=1
ab2726
ab2726
Breakpoint 2, inc_vi () at dprintf-execution-x-script.c:26
ab2726
26	in dprintf-execution-x-script.c
ab2726
vi=2
ab2726
dprintf in increment(), vi=2
ab2726
ab2726
Breakpoint 2, inc_vi () at dprintf-execution-x-script.c:26
ab2726
26	in dprintf-execution-x-script.c
ab2726
vi=3
ab2726
[Inferior 1 (process 1539210) exited normally]
ab2726
ab2726
In this run, in which 'run' was issued from the gdb prompt (instead
ab2726
of at the end of the script), there are three dprintf messages along
ab2726
with three 'Breakpoint 2' messages.  This is the correct output.
ab2726
ab2726
Now let's look at the output that I snipped above; this is the output
ab2726
when 'run' is issued from the script loaded via GDB's -x switch:
ab2726
ab2726
$ gdb -q -x dp-bug.gdb dp-bug
ab2726
Reading symbols from dp-bug...
ab2726
Dprintf 1 at 0x40116e: file dprintf-execution-x-script.c, line 38.
ab2726
Breakpoint 2 at 0x40113a: file dprintf-execution-x-script.c, line 26.
ab2726
vi=0
ab2726
dprintf in increment(), vi=0
ab2726
ab2726
Breakpoint 2, inc_vi () at dprintf-execution-x-script.c:26
ab2726
26	dprintf-execution-x-script.c: No such file or directory.
ab2726
vi=1
ab2726
ab2726
Breakpoint 2, inc_vi () at dprintf-execution-x-script.c:26
ab2726
26	in dprintf-execution-x-script.c
ab2726
vi=2
ab2726
ab2726
Breakpoint 2, inc_vi () at dprintf-execution-x-script.c:26
ab2726
26	in dprintf-execution-x-script.c
ab2726
vi=3
ab2726
[Inferior 1 (process 1539175) exited normally]
ab2726
ab2726
In the output shown above, only the first dprintf message is printed.
ab2726
The 2nd and 3rd dprintf messages are missing!  However, all three
ab2726
'Breakpoint 2...' messages are still printed.
ab2726
ab2726
Why does this happen?
ab2726
ab2726
bpstat_do_actions_1() in gdb/breakpoint.c contains the following
ab2726
comment and code near the start of the function:
ab2726
ab2726
  /* Avoid endless recursion if a `source' command is contained
ab2726
     in bs->commands.  */
ab2726
  if (executing_breakpoint_commands)
ab2726
    return 0;
ab2726
ab2726
  scoped_restore save_executing
ab2726
    = make_scoped_restore (&executing_breakpoint_commands, 1);
ab2726
ab2726
Also, as described by this comment prior to the 'async' field
ab2726
in 'struct ui' in top.h, the main UI starts off in sync mode
ab2726
when processing command line arguments:
ab2726
ab2726
  /* True if the UI is in async mode, false if in sync mode.  If in
ab2726
     sync mode, a synchronous execution command (e.g, "next") does not
ab2726
     return until the command is finished.  If in async mode, then
ab2726
     running a synchronous command returns right after resuming the
ab2726
     target.  Waiting for the command's completion is later done on
ab2726
     the top event loop.  For the main UI, this starts out disabled,
ab2726
     until all the explicit command line arguments (e.g., `gdb -ex
ab2726
     "start" -ex "next"') are processed.  */
ab2726
ab2726
This combination of things, the state of the static global
ab2726
'executing_breakpoint_commands' plus the state of the async
ab2726
field in the main UI causes this behavior.
ab2726
ab2726
This is a backtrace after hitting the dprintf breakpoint for
ab2726
the second time when doing 'run' from the script file, i.e.
ab2726
non-interactively:
ab2726
ab2726
Thread 1 "gdb" hit Breakpoint 3, bpstat_do_actions_1 (bsp=0x7fffffffc2b8)
ab2726
    at /ironwood1/sourceware-git/f34-master/bld/../../worktree-master/gdb/breakpoint.c:4431
ab2726
4431	  if (executing_breakpoint_commands)
ab2726
ab2726
 #0  bpstat_do_actions_1 (bsp=0x7fffffffc2b8)
ab2726
     at gdb/breakpoint.c:4431
ab2726
 #1  0x00000000004d8bc6 in dprintf_after_condition_true (bs=0x1538090)
ab2726
     at gdb/breakpoint.c:13048
ab2726
 #2  0x00000000004c5caa in bpstat_stop_status (aspace=0x116dbc0, bp_addr=0x40116e, thread=0x137f450, ws=0x7fffffffc718,
ab2726
     stop_chain=0x1538090) at gdb/breakpoint.c:5498
ab2726
 #3  0x0000000000768d98 in handle_signal_stop (ecs=0x7fffffffc6f0)
ab2726
     at gdb/infrun.c:6172
ab2726
 #4  0x00000000007678d3 in handle_inferior_event (ecs=0x7fffffffc6f0)
ab2726
     at gdb/infrun.c:5662
ab2726
 #5  0x0000000000763cd5 in fetch_inferior_event ()
ab2726
     at gdb/infrun.c:4060
ab2726
 #6  0x0000000000746d7d in inferior_event_handler (event_type=INF_REG_EVENT)
ab2726
     at gdb/inf-loop.c:41
ab2726
 #7  0x00000000007a702f in handle_target_event (error=0, client_data=0x0)
ab2726
     at gdb/linux-nat.c:4207
ab2726
 #8  0x0000000000b8cd6e in gdb_wait_for_event (block=block@entry=0)
ab2726
     at gdbsupport/event-loop.cc:701
ab2726
 #9  0x0000000000b8d032 in gdb_wait_for_event (block=0)
ab2726
     at gdbsupport/event-loop.cc:597
ab2726
 #10 gdb_do_one_event () at gdbsupport/event-loop.cc:212
ab2726
 #11 0x00000000009d19b6 in wait_sync_command_done ()
ab2726
     at gdb/top.c:528
ab2726
 #12 0x00000000009d1a3f in maybe_wait_sync_command_done (was_sync=0)
ab2726
     at gdb/top.c:545
ab2726
 #13 0x00000000009d2033 in execute_command (p=0x7fffffffcb18 "", from_tty=0)
ab2726
     at gdb/top.c:676
ab2726
 #14 0x0000000000560d5b in execute_control_command_1 (cmd=0x13b9bb0, from_tty=0)
ab2726
     at gdb/cli/cli-script.c:547
ab2726
 #15 0x000000000056134a in execute_control_command (cmd=0x13b9bb0, from_tty=0)
ab2726
     at gdb/cli/cli-script.c:717
ab2726
 #16 0x00000000004c3bbe in bpstat_do_actions_1 (bsp=0x137f530)
ab2726
     at gdb/breakpoint.c:4469
ab2726
 #17 0x00000000004c3d40 in bpstat_do_actions ()
ab2726
     at gdb/breakpoint.c:4533
ab2726
 #18 0x00000000006a473a in command_handler (command=0x1399ad0 "run")
ab2726
     at gdb/event-top.c:624
ab2726
 #19 0x00000000009d182e in read_command_file (stream=0x113e540)
ab2726
     at gdb/top.c:443
ab2726
 #20 0x0000000000563697 in script_from_file (stream=0x113e540, file=0x13bb0b0 "dp-bug.gdb")
ab2726
     at gdb/cli/cli-script.c:1642
ab2726
 #21 0x00000000006abd63 in source_gdb_script (extlang=0xc44e80 <extension_language_gdb>, stream=0x113e540,
ab2726
     file=0x13bb0b0 "dp-bug.gdb") at gdb/extension.c:188
ab2726
 #22 0x0000000000544400 in source_script_from_stream (stream=0x113e540, file=0x7fffffffd91a "dp-bug.gdb",
ab2726
     file_to_open=0x13bb0b0 "dp-bug.gdb")
ab2726
     at gdb/cli/cli-cmds.c:692
ab2726
 #23 0x0000000000544557 in source_script_with_search (file=0x7fffffffd91a "dp-bug.gdb", from_tty=1, search_path=0)
ab2726
     at gdb/cli/cli-cmds.c:750
ab2726
 #24 0x00000000005445cf in source_script (file=0x7fffffffd91a "dp-bug.gdb", from_tty=1)
ab2726
     at gdb/cli/cli-cmds.c:759
ab2726
 #25 0x00000000007cf6d9 in catch_command_errors (command=0x5445aa <source_script(char const*, int)>,
ab2726
     arg=0x7fffffffd91a "dp-bug.gdb", from_tty=1, do_bp_actions=false)
ab2726
     at gdb/main.c:523
ab2726
 #26 0x00000000007cf85d in execute_cmdargs (cmdarg_vec=0x7fffffffd1b0, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND,
ab2726
     ret=0x7fffffffd18c) at gdb/main.c:615
ab2726
 #27 0x00000000007d0c8e in captured_main_1 (context=0x7fffffffd3f0)
ab2726
     at gdb/main.c:1322
ab2726
 #28 0x00000000007d0eba in captured_main (data=0x7fffffffd3f0)
ab2726
     at gdb/main.c:1343
ab2726
 #29 0x00000000007d0f25 in gdb_main (args=0x7fffffffd3f0)
ab2726
     at gdb/main.c:1368
ab2726
 #30 0x00000000004186dd in main (argc=5, argv=0x7fffffffd508)
ab2726
     at gdb/gdb.c:32
ab2726
ab2726
There are two frames for bpstat_do_actions_1(), one at frame #16 and
ab2726
the other at frame #0.  The one at frame #16 is processing the actions
ab2726
for Breakpoint 2, which is a 'continue'.  The one at frame #0 is attempting
ab2726
to process the dprintf breakpoint action.  However, at this point,
ab2726
the value of 'executing_breakpoint_commands' is 1, forcing an early
ab2726
return, i.e. prior to executing the command(s) associated with the dprintf
ab2726
breakpoint.
ab2726
ab2726
For the sake of comparison, this is what the stack looks like when hitting
ab2726
the dprintf breakpoint for the second time when issuing the 'run'
ab2726
command from the GDB prompt.
ab2726
ab2726
Thread 1 "gdb" hit Breakpoint 3, bpstat_do_actions_1 (bsp=0x7fffffffccd8)
ab2726
    at /ironwood1/sourceware-git/f34-master/bld/../../worktree-master/gdb/breakpoint.c:4431
ab2726
4431	  if (executing_breakpoint_commands)
ab2726
ab2726
 #0  bpstat_do_actions_1 (bsp=0x7fffffffccd8)
ab2726
     at gdb/breakpoint.c:4431
ab2726
 #1  0x00000000004d8bc6 in dprintf_after_condition_true (bs=0x16b0290)
ab2726
     at gdb/breakpoint.c:13048
ab2726
 #2  0x00000000004c5caa in bpstat_stop_status (aspace=0x116dbc0, bp_addr=0x40116e, thread=0x13f0e60, ws=0x7fffffffd138,
ab2726
     stop_chain=0x16b0290) at gdb/breakpoint.c:5498
ab2726
 #3  0x0000000000768d98 in handle_signal_stop (ecs=0x7fffffffd110)
ab2726
     at gdb/infrun.c:6172
ab2726
 #4  0x00000000007678d3 in handle_inferior_event (ecs=0x7fffffffd110)
ab2726
     at gdb/infrun.c:5662
ab2726
 #5  0x0000000000763cd5 in fetch_inferior_event ()
ab2726
     at gdb/infrun.c:4060
ab2726
 #6  0x0000000000746d7d in inferior_event_handler (event_type=INF_REG_EVENT)
ab2726
     at gdb/inf-loop.c:41
ab2726
 #7  0x00000000007a702f in handle_target_event (error=0, client_data=0x0)
ab2726
     at gdb/linux-nat.c:4207
ab2726
 #8  0x0000000000b8cd6e in gdb_wait_for_event (block=block@entry=0)
ab2726
     at gdbsupport/event-loop.cc:701
ab2726
 #9  0x0000000000b8d032 in gdb_wait_for_event (block=0)
ab2726
     at gdbsupport/event-loop.cc:597
ab2726
 #10 gdb_do_one_event () at gdbsupport/event-loop.cc:212
ab2726
 #11 0x00000000007cf512 in start_event_loop ()
ab2726
     at gdb/main.c:421
ab2726
 #12 0x00000000007cf631 in captured_command_loop ()
ab2726
     at gdb/main.c:481
ab2726
 #13 0x00000000007d0ebf in captured_main (data=0x7fffffffd3f0)
ab2726
     at gdb/main.c:1353
ab2726
 #14 0x00000000007d0f25 in gdb_main (args=0x7fffffffd3f0)
ab2726
     at gdb/main.c:1368
ab2726
 #15 0x00000000004186dd in main (argc=5, argv=0x7fffffffd508)
ab2726
     at gdb/gdb.c:32
ab2726
ab2726
This relatively short backtrace is due to the current UI's async field
ab2726
being set to 1.
ab2726
ab2726
Yet another thing to be aware of regarding this problem is the
ab2726
difference in the way that commands associated to dprintf breakpoints
ab2726
versus regular breakpoints are handled.  While they both use a command
ab2726
list associated with the breakpoint, regular breakpoints will place
ab2726
the commands to be run on the bpstat chain constructed in
ab2726
bp_stop_status().  These commands are run later on.  For dprintf
ab2726
breakpoints, commands are run via the 'after_condition_true' function
ab2726
pointer directly from bpstat_stop_status().  (The 'commands' field in
ab2726
the bpstat is cleared in dprintf_after_condition_true().  This
ab2726
prevents the dprintf commands from being run again later on when other
ab2726
commands on the bpstat chain are processed.)
ab2726
ab2726
Another thing that I noticed is that dprintf breakpoints are the only
ab2726
type of breakpoint which use 'after_condition_true'.  This suggests
ab2726
that one possible way of fixing this problem, that of making dprintf
ab2726
breakpoints work more like regular breakpoints, probably won't work.
ab2726
(I must admit, however, that my understanding of this code isn't
ab2726
complete enough to say why.  I'll trust that whoever implemented it
ab2726
had a good reason for doing it this way.)
ab2726
ab2726
The comment referenced earlier regarding 'executing_breakpoint_commands'
ab2726
states that the reason for checking this variable is to avoid
ab2726
potential endless recursion when a 'source' command appears in
ab2726
bs->commands.  We know that a dprintf command is constrained to either
ab2726
1) execution of a GDB printf command, 2) an inferior function call of
ab2726
a printf-like function, or 3) execution of an agent-printf command.
ab2726
Therefore, infinite recursion due to a 'source' command cannot happen
ab2726
when executing commands upon hitting a dprintf breakpoint.
ab2726
ab2726
I chose to fix this problem by having dprintf_after_condition_true()
ab2726
directly call execute_control_commands().  This means that it no
ab2726
longer attempts to go through bpstat_do_actions_1() avoiding the
ab2726
infinite recursion check for potential 'source' commands on the
ab2726
command chain.  I think it simplifies this code a little bit too, a
ab2726
definite bonus.
ab2726
ab2726
Summary:
ab2726
ab2726
	* breakpoint.c (dprintf_after_condition_true): Don't call
ab2726
	bpstat_do_actions_1().  Call execute_control_commands()
ab2726
	instead.
ab2726
ab2726
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
ab2726
--- a/gdb/breakpoint.c
ab2726
+++ b/gdb/breakpoint.c
ab2726
@@ -13043,9 +13043,6 @@ dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)
ab2726
 static void
ab2726
 dprintf_after_condition_true (struct bpstats *bs)
ab2726
 {
ab2726
-  struct bpstats tmp_bs;
ab2726
-  struct bpstats *tmp_bs_p = &tmp_bs;
ab2726
-
ab2726
   /* dprintf's never cause a stop.  This wasn't set in the
ab2726
      check_status hook instead because that would make the dprintf's
ab2726
      condition not be evaluated.  */
ab2726
@@ -13056,14 +13053,9 @@ dprintf_after_condition_true (struct bpstats *bs)
ab2726
      bpstat_do_actions, if a breakpoint that causes a stop happens to
ab2726
      be set at same address as this dprintf, or even if running the
ab2726
      commands here throws.  */
ab2726
-  tmp_bs.commands = bs->commands;
ab2726
-  bs->commands = NULL;
ab2726
-
ab2726
-  bpstat_do_actions_1 (&tmp_bs_p);
ab2726
-
ab2726
-  /* 'tmp_bs.commands' will usually be NULL by now, but
ab2726
-     bpstat_do_actions_1 may return early without processing the whole
ab2726
-     list.  */
ab2726
+  counted_command_line cmds = std::move (bs->commands);
ab2726
+  gdb_assert (cmds != nullptr);
ab2726
+  execute_control_commands (cmds.get (), 0);
ab2726
 }
ab2726
 
ab2726
 /* The breakpoint_ops structure to be used on static tracepoints with