diff options
author | Andrew Burgess <aburgess@redhat.com> | 2023-07-12 21:56:50 +0100 |
---|---|---|
committer | Andrew Burgess <aburgess@redhat.com> | 2023-08-03 14:55:31 +0100 |
commit | b1c0ab20809a502b2d2224fecb0dca3ada2e9b22 (patch) | |
tree | 5bf5fd6db3c2f95f8e94b09edd2c4adc68cfff01 /gdb/breakpoint.c | |
parent | f559e52a8e79585e0b13156e949a7fd46060a714 (diff) | |
download | fsf-binutils-gdb-b1c0ab20809a502b2d2224fecb0dca3ada2e9b22.zip fsf-binutils-gdb-b1c0ab20809a502b2d2224fecb0dca3ada2e9b22.tar.gz fsf-binutils-gdb-b1c0ab20809a502b2d2224fecb0dca3ada2e9b22.tar.bz2 |
gdb: avoid double stop after failed breakpoint condition check
This commit replaces this earlier commit:
commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
Date: Fri Oct 14 14:53:15 2022 +0100
gdb: don't always print breakpoint location after failed condition check
and is a result of feedback received here[1].
The original commit addressed a problem where, if a breakpoint
condition included an inferior function call, and if the inferior
function call failed, then GDB would announce the stop twice. Here's
an example of GDB's output before the above commit that shows the
problem being addressed:
(gdb) break foo if (some_func ())
Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
(gdb) r
Starting program: /tmp/bpcond
Program received signal SIGSEGV, Segmentation fault.
0x0000000000401116 in some_func () at bpcond.c:5
5 return *p;
Error in testing condition for breakpoint 1:
The program being debugged stopped while in a function called from GDB.
Evaluation of the expression containing the function
(some_func) will be abandoned.
When the function is done executing, GDB will silently stop.
Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5
5 return *p;
(gdb)
The original commit addressed this issue in breakpoint.c, by spotting
that the $pc had changed while evaluating the breakpoint condition,
and inferring from this that GDB must have stopped elsewhere.
However, the way in which the original commit suppressed the second
stop announcement was to set bpstat::print to true -- this tells GDB
not to print the frame during the stop announcement, and for the CLI
this is fine, however, it was pointed out that for the MI this still
isn't really enough. Below is an example from an MI session after the
above commit was applied, this shows the problem with the above
commit:
-break-insert -c "cond_fail()" foo
^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000040111e",func="foo",file="/tmp/mi-condbreak-fail.c",line="30",thread-groups=["i1"],cond="cond_fail()",times="0",original-location="foo"}
(gdb)
-exec-run
=thread-group-started,id="i1",pid="2636270"
=thread-created,id="1",group-id="i1"
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1",ranges=[{from="0x00007ffff7fd3110",to="0x00007ffff7ff2bb4"}]
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name="/lib64/libm.so.6",symbols-loaded="0",thread-group="i1",ranges=[{from="0x00007ffff7e59390",to="0x00007ffff7ef4f98"}]
=library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name="/lib64/libc.so.6",symbols-loaded="0",thread-group="i1",ranges=[{from="0x00007ffff7ca66b0",to="0x00007ffff7df3c5f"}]
~"\nProgram"
~" received signal SIGSEGV, Segmentation fault.\n"
~"0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24\n"
~"24\t return *p;\t\t\t/* Crash here. */\n"
*stopped,reason="signal-received",signal-name="SIGSEGV",signal-meaning="Segmentation fault",frame={addr="0x0000000000401116",func="cond_fail",args=[],file="/tmp/mi-condbreak-fail.c",fullname="/tmp/mi-condbreak-fail.c",line="24",arch="i386:x86-64"},thread-id="1",stopped-threads="all",core="9"
&"Error in testing condition for breakpoint 1:\n"
&"The program being debugged was signaled while in a function called from GDB.\n"
&"GDB remains in the frame where the signal was received.\n"
&"To change this behavior use \"set unwindonsignal on\".\n"
&"Evaluation of the expression containing the function\n"
&"(cond_fail) will be abandoned.\n"
&"When the function is done executing, GDB will silently stop.\n"
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000040111e",func="foo",file="/tmp/mi-condbreak-fail.c",fullname="/tmp/mi-condbreak-fail.c",line="30",thread-groups=["i1"],cond="cond_fail()",times="1",original-location="foo"}
*stopped
(gdb)
Notice that we still see two '*stopped' lines, the first includes the
full frame information, while the second has no frame information,
this is a result of bpstat::print having been set. Ideally, the
second '*stopped' line should not be present.
By setting bpstat::print I was addressing the problem too late, this
flag really only changes how interp::on_normal_stop prints the stop
event, and interp::on_normal_stop is called (indirectly) from the
normal_stop function in infrun.c. A better solution is to avoid
calling normal_stop at all for the stops which should not be reported
to the user, and this is what I do in this commit.
This commit has 3 parts:
1. In breakpoint.c, revert the above commit,
2. In fetch_inferior_event (infrun.c), capture the stop-id before
calling handle_inferior_event. If, after calling
handle_inferior_event, the stop-id has changed, then this indicates
that somewhere within handle_inferior_event, a stop was announced to
the user. If this is the case then GDB should not call normal_stop,
and we should rely on whoever announced the stop to ensure that we
are in a PROMPT_NEEDED state, which means the prompt will be
displayed once fetch_inferior_event returns. And,
3. In infcall.c, do two things:
(a) In run_inferior_call, after making the inferior call, ensure
that either async_disable_stdin or async_enable_stdin is called
to put the prompt state, and stdin handling into the correct
state based on whether the inferior call completed successfully
or not, and
(b) In call_thread_fsm::should_stop, call async_enable_stdin
rather than changing the prompt state directly. This isn't
strictly necessary, but helped me understand this code more.
This async_enable_stdin call is only reached if normal_stop is
not going to be called, and replaces the async_enable_stdin call
that exists in normal_stop. Though we could just adjust the
prompt state if felt (to me) much easier to understand when I
could see this call and the corresponding call in normal_stop.
With these changes in place now, when the inferior call (from the
breakpoint condition) fails, infcall.c leaves the prompt state as
PROMPT_NEEDED, and leaves stdin registered with the event loop.
Back in fetch_inferior_event GDB notices that the stop-id has changed
and so avoids calling normal_stop.
And on return from fetch_inferior_event GDB will display the prompt
and handle input from stdin.
As normal_stop is not called the MI problem is solved, and the test
added in the earlier mentioned commit still passes just fine, so the
CLI has not regressed.
[1] https://inbox.sourceware.org/gdb-patches/6fd4aa13-6003-2563-5841-e80d5a55d59e@palves.net/
Diffstat (limited to 'gdb/breakpoint.c')
-rw-r--r-- | gdb/breakpoint.c | 12 |
1 files changed, 0 insertions, 12 deletions
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index d898167..93634bd 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5535,7 +5535,6 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) else within_current_scope = false; } - CORE_ADDR pc_before_check = get_frame_pc (get_selected_frame (nullptr)); if (within_current_scope) { try @@ -5555,17 +5554,6 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) (gdb_stderr, ex, "Error in testing condition for breakpoint %d:\n", b->number); - - /* If the pc value changed as a result of evaluating the - condition then we probably stopped within an inferior - function call due to some unexpected stop, e.g. the thread - hit another breakpoint, or the thread received an - unexpected signal. In this case we don't want to also - print the information about this breakpoint. */ - CORE_ADDR pc_after_check - = get_frame_pc (get_selected_frame (nullptr)); - if (pc_before_check != pc_after_check) - bs->print = 0; } } else |