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

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