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

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