You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
219 lines
9.7 KiB
219 lines
9.7 KiB
http://sourceware.org/ml/gdb-patches/2015-02/msg00730.html |
|
Subject: [PATCH 2/8] Teach GDB about targets that can tell whether a trap is a breakpoint event |
|
|
|
The moribund locations heuristics are problematic. This patch teaches |
|
GDB about targets that can reliably tell whether a trap was caused by |
|
a software or hardware breakpoint, and thus don't need moribund |
|
locations, thus bypassing all the problems that mechanism has. |
|
|
|
The non-stop-fair-events.exp test is frequently failing currently. |
|
E.g., see https://sourceware.org/ml/gdb-testers/2015-q1/msg03148.html. |
|
|
|
The root cause is a fundamental problem with moribund locations. For |
|
example, the stepped_breakpoint logic added by af48d08f breaks in this |
|
case (which is what happens with that test): |
|
|
|
- Step thread A, no breakpoint is set at PC. |
|
|
|
- The kernel doesn't schedule thread A yet. |
|
|
|
- Insert breakpoint at A's PC, for some reason (e.g., a step-resume |
|
breakpoint for thread B). |
|
|
|
- Kernel finally schedules thread A. |
|
|
|
- thread A's stepped_breakpoint flag is not set, even though it now |
|
stepped a breakpoint instruction. |
|
|
|
- adjust_pc_after_break gets the PC wrong, because PC == PREV_PC, but |
|
stepped_breakpoint is not set. |
|
|
|
We needed the stepped_breakpoint logic to workaround moribund |
|
locations, because otherwise adjust_pc_after_break could apply an |
|
adjustment when it shouldn't just because there _used_ to be a |
|
breakpoint at PC (a moribund breakpoint location). For example, on |
|
x86, that's wrong if the thread really hasn't executed an int3, but |
|
instead executed some other 1-byte long instruction. Getting the PC |
|
adjustment wrong of course leads to the inferior executing the wrong |
|
instruction. |
|
|
|
Other problems with moribund locations are: |
|
|
|
- if a true SIGTRAP happens to be raised when the program is |
|
executing the PC that used to have a breakpoint, GDB will assume |
|
that is a trap for a breakpoint that has recently been removed, and |
|
thus we miss reporting the random signal to the user. |
|
|
|
- to minimize that, we get rid of moribund location after a while. |
|
That while is defined as just a certain number of events being |
|
processed. That number of events sometimes passes by before a |
|
delayed breakpoint is processed, and GDB confuses the trap for a |
|
random signal, thus reporting the random trap. Once the user |
|
resumes the thread, the program crashes because the PC was not |
|
adjusted... |
|
|
|
The fix for all this is to bite the bullet and get rid of heuristics |
|
and instead rely on the target knowing accurately what caused the |
|
SIGTRAP. The target/kernel/stub is in the best position to know what |
|
that, because it can e.g. consult priviledged CPU flags GDB has no |
|
access to, or by knowing which exception vector entry was called when |
|
the instruction trapped, etc. Most debug APIs I've seen to date |
|
report breakpoint hits as a distinct event in some fashion. For |
|
example, on the Linux kernel, whether a breakpoint was executed is |
|
exposed to userspace in the si_code field of the SIGTRAP's siginfo. |
|
On Windows, the debug API reports a EXCEPTION_BREAKPOINT exception |
|
code. |
|
|
|
We needed to keep around deleted breakpoints in an on-the-side list |
|
(the moribund locations) for two main reasons: |
|
|
|
- Know that a SIGTRAP actually is a delayed event for a hit of a |
|
breakpoint that was removed before the event was processed, and |
|
thus should not be reported as a random signal. |
|
|
|
- So we still do the decr_pc_after_break adjustment in that case, so |
|
that the thread is resumed at the correct address. |
|
|
|
In the new model, if GDB processes an event the target tells is a |
|
breakpoint trap, and GDB doesn't find the corresponding breakpoint in |
|
its breakpoint tables, it means that event is a delayed event for a |
|
breakpoint that has since been removed, and thus the event should be |
|
ignored. |
|
|
|
For the decr_pc_after_after issue, it ends up being much simpler that |
|
on targets that can reliably tell whether a breakpoint trapped, for |
|
the breakpoint trap to present the PC already adjusted. Proper |
|
multi-threading support already implies that targets needs to be doing |
|
decr_pc_after_break adjustment themselves, otherwise for example, in |
|
all-stop if two threads hit a breakpoint simultaneously, and the user |
|
does "info threads", he'll see the non-event thread that hit the |
|
breakpoint stopped at the wrong PC. |
|
|
|
This way (target adjusts) also ends up eliminating the need for some |
|
awkward re-incrementing of the PC in the record-full and Linux targets |
|
that we do today, and the need for the target_decr_pc_after_break |
|
hook. |
|
|
|
If the target always adjusts, then there's a case where GDB needs to |
|
re-increment the PC. Say, on x86, an "int3" instruction that was |
|
explicitly written in the program traps. In this case, GDB should |
|
report a random SIGTRAP signal to the user, with the PC pointing at |
|
the instruction past the int3, just like if GDB was not debugging the |
|
program. The user may well decide to pass the SIGTRAP to the program |
|
because the program being debugged has a SIGTRAP handler that handles |
|
its own breakpoints, and expects the PC to be unadjusted. |
|
|
|
Tested on x86-64 Fedora 20. |
|
|
|
gdb/ChangeLog: |
|
2015-02-25 Pedro Alves <palves@redhat.com> |
|
|
|
* breakpoint.c (need_moribund_for_location_type): New function. |
|
(bpstat_stop_status): Don't skipping checking moribund locations |
|
of breakpoint types which the target tell caused a stop. |
|
(program_breakpoint_here_p): New function, factored out from ... |
|
(bp_loc_is_permanent): ... this. |
|
(update_global_location_list): Don't create a moribund location if |
|
the target supports reporting stops of the type of the removed |
|
breakpoint. |
|
* breakpoint.h (program_breakpoint_here_p): New declaration. |
|
* infrun.c (adjust_pc_after_break): Return early if the target has |
|
already adjusted the PC. Add comments. |
|
(handle_signal_stop): If nothing explains a signal, and the target |
|
tells us the stop was caused by a software breakpoint, check if |
|
there's a breakpoint instruction in the memory. If so, adjust the |
|
PC before presenting the stop to the user. Otherwise, ignore the |
|
trap. If nothing explains a signal, and the target tells us the |
|
stop was caused by a hardware breakpoint, ignore the trap. |
|
* target.h (struct target_ops) <to_stopped_by_sw_breakpoint, |
|
to_supports_stopped_by_sw_breakpoint, to_stopped_by_hw_breakpoint, |
|
to_supports_stopped_by_hw_breakpoint>: New fields. |
|
(target_stopped_by_sw_breakpoint) |
|
(target_supports_stopped_by_sw_breakpoint) |
|
(target_stopped_by_hw_breakpoint) |
|
(target_supports_stopped_by_hw_breakpoint): Define. |
|
* target-delegates.c: Regenerate. |
|
--- |
|
gdb/breakpoint.c | 91 ++++++++++++++++++++++++------------ |
|
gdb/breakpoint.h | 5 ++ |
|
gdb/infrun.c | 70 +++++++++++++++++++++++++++- |
|
gdb/target-delegates.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++ |
|
gdb/target.h | 44 ++++++++++++++++++ |
|
5 files changed, 303 insertions(+), 31 deletions(-) |
|
|
|
Index: gdb-7.6.1/gdb/target.h |
|
=================================================================== |
|
--- gdb-7.6.1.orig/gdb/target.h 2016-03-13 22:01:58.833963789 +0100 |
|
+++ gdb-7.6.1/gdb/target.h 2016-03-13 22:01:59.950971809 +0100 |
|
@@ -462,6 +462,16 @@ |
|
void (*to_files_info) (struct target_ops *); |
|
int (*to_insert_breakpoint) (struct gdbarch *, struct bp_target_info *); |
|
int (*to_remove_breakpoint) (struct gdbarch *, struct bp_target_info *); |
|
+ |
|
+ /* Returns true if the target stopped for a hardware breakpoint. |
|
+ Likewise, if the target supports hardware breakpoints, this |
|
+ method is necessary for correct background execution / non-stop |
|
+ mode operation. Even though hardware breakpoints do not |
|
+ require PC adjustment, GDB needs to be able to tell whether the |
|
+ hardware breakpoint event is a delayed event for a breakpoint |
|
+ that is already gone and should thus be ignored. */ |
|
+ int (*to_stopped_by_hw_breakpoint) (struct target_ops *); |
|
+ |
|
int (*to_can_use_hw_breakpoint) (int, int, int); |
|
int (*to_ranged_break_num_registers) (struct target_ops *); |
|
int (*to_insert_hw_breakpoint) (struct gdbarch *, struct bp_target_info *); |
|
@@ -1548,6 +1558,9 @@ |
|
#define target_stopped_by_watchpoint \ |
|
(*current_target.to_stopped_by_watchpoint) |
|
|
|
+#define target_stopped_by_hw_breakpoint() \ |
|
+ ((*current_target.to_stopped_by_hw_breakpoint) (¤t_target)) |
|
+ |
|
/* Non-zero if we have steppable watchpoints */ |
|
|
|
#define target_have_steppable_watchpoint \ |
|
Index: gdb-7.6.1/gdb/infrun.c |
|
=================================================================== |
|
--- gdb-7.6.1.orig/gdb/infrun.c 2016-03-13 22:01:58.704962863 +0100 |
|
+++ gdb-7.6.1/gdb/infrun.c 2016-03-13 22:17:33.735674697 +0100 |
|
@@ -4295,6 +4295,18 @@ |
|
ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_TRAP; |
|
} |
|
|
|
+ /* Maybe this was a trap for a hardware breakpoint/watchpoint that |
|
+ has since been removed. */ |
|
+ if (ecs->random_signal && target_stopped_by_hw_breakpoint()) |
|
+ { |
|
+ /* A delayed hardware breakpoint event. Ignore the trap. */ |
|
+ if (debug_infrun) |
|
+ fprintf_unfiltered (gdb_stdlog, |
|
+ "infrun: delayed hardware breakpoint/watchpoint " |
|
+ "trap, ignoring\n"); |
|
+ ecs->random_signal = 0; |
|
+ } |
|
+ |
|
process_event_stop_test: |
|
|
|
/* Re-fetch current thread's frame in case we did a |
|
Index: gdb-7.6.1/gdb/target.c |
|
=================================================================== |
|
--- gdb-7.6.1.orig/gdb/target.c 2016-03-13 22:01:58.832963782 +0100 |
|
+++ gdb-7.6.1/gdb/target.c 2016-03-13 22:17:20.367578754 +0100 |
|
@@ -726,6 +726,7 @@ |
|
/* Do not inherit to_memory_map. */ |
|
/* Do not inherit to_flash_erase. */ |
|
/* Do not inherit to_flash_done. */ |
|
+ INHERIT (to_stopped_by_hw_breakpoint, t); |
|
} |
|
#undef INHERIT |
|
|
|
@@ -968,6 +969,9 @@ |
|
(int (*) (void)) |
|
return_zero); |
|
de_fault (to_execution_direction, default_execution_direction); |
|
+ de_fault (to_stopped_by_hw_breakpoint, |
|
+ (int (*) (struct target_ops *)) |
|
+ return_zero); |
|
|
|
#undef de_fault |
|
|
|
|