* [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error)
@ 2016-02-14 18:37 Pedro Alves
2016-02-14 18:37 ` [PATCH 1/4] Simplify remove_breakpoint interface Pedro Alves
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Pedro Alves @ 2016-02-14 18:37 UTC (permalink / raw)
To: gdb-patches
This series fixes:
(gdb) record
break marker2
Breakpoint 2 at 0x40067a: file /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.reverse/waitpid-reverse.c, line 30.
(gdb) PASS: gdb.reverse/waitpid-reverse.exp: set breakpoint at marker2
continue
Continuing.
/home/pedro/gdb/mygit/src/gdb/record-full.c:1720: internal-error: record_full_remove_breakpoint: removing unknown breakpoint
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
KFAIL: gdb.reverse/waitpid-reverse.exp: continue to breakpoint: marker2 (GDB internal error) (PRMS: gdb/19187)
The problem is that the target side can't distinguish between deleting
a breakpoint and detaching a breakpoint.
Right after a fork is detected, we detach breakpoints from the child
(detach_breakpoints), which calls into target_remove_breakpoint with
inferior_ptid pointing at the child process, but leaves the breakpoint
marked inserted (in the parent).
Since record-full.c's target_remove_breakpoint implementation doesn't
know we're detaching a breakpoint, it removes the breakpoint from its
own breakpoint table.
Then when we later remove the breakpoint from the parent, we fail that
assertion, since the breakpoint is unexpectedly not found in the
record-full.c breakpoint table.
The first three patches pass down the reason for removing the
breakpoint all the way to the target.
Patch 4 then simply makes record-full.c not delete the breakpoint from
its breakpoint table when we're detaching the breakpoint from a fork
child.
Pedro Alves (4):
Simplify remove_breakpoint interface
Introduce 'enum remove_bp_reason'
Plumb enum remove_bp_reason all the way to target_remove_breakpoint
Fix PR gdb/19187 (process record over a fork causes internal error)
gdb/break-catch-sig.c | 3 +-
gdb/break-catch-syscall.c | 2 +-
gdb/breakpoint.c | 70 +++++++++++++--------------
gdb/breakpoint.h | 14 +++++-
gdb/corelow.c | 10 +++-
gdb/exec.c | 10 +++-
gdb/mem-break.c | 3 +-
gdb/record-btrace.c | 6 ++-
gdb/record-full.c | 15 ++++--
gdb/remote.c | 5 +-
gdb/target-debug.h | 2 +
gdb/target-delegates.c | 10 ++--
gdb/target.c | 5 +-
gdb/target.h | 9 ++--
gdb/testsuite/gdb.reverse/waitpid-reverse.exp | 4 +-
15 files changed, 105 insertions(+), 63 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/4] Simplify remove_breakpoint interface 2016-02-14 18:37 [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves @ 2016-02-14 18:37 ` Pedro Alves 2016-02-14 18:37 ` [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint Pedro Alves ` (3 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2016-02-14 18:37 UTC (permalink / raw) To: gdb-patches All callers pass mark_uninserted, so there's no need for the 'is' parameter. gdb/ChangeLog: 2016-02-14 Pedro Alves <palves@redhat.com> * breakpoint.c (remove_breakpoint): Remove 'is' parameter and always pass mark_uninserted to remove_breakpoint_1. (insert_breakpoint_locations, remove_breakpoints) (remove_breakpoints_pid, update_global_location_list): Update callers. --- gdb/breakpoint.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index f99a7ab..05da8ed 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -201,7 +201,7 @@ typedef enum } insertion_state_t; -static int remove_breakpoint (struct bp_location *, insertion_state_t); +static int remove_breakpoint (struct bp_location *); static int remove_breakpoint_1 (struct bp_location *, insertion_state_t); static enum print_stop_action print_bp_stop_message (bpstat bs); @@ -3182,7 +3182,7 @@ insert_breakpoint_locations (void) { for (loc = bpt->loc; loc; loc = loc->next) if (loc->inserted) - remove_breakpoint (loc, mark_uninserted); + remove_breakpoint (loc); hw_breakpoint_error = 1; fprintf_unfiltered (tmp_error_stream, @@ -3222,7 +3222,7 @@ remove_breakpoints (void) ALL_BP_LOCATIONS (bl, blp_tmp) { if (bl->inserted && !is_tracepoint (bl->owner)) - val |= remove_breakpoint (bl, mark_uninserted); + val |= remove_breakpoint (bl); } return val; } @@ -3267,7 +3267,7 @@ remove_breakpoints_pid (int pid) if (bl->inserted && !bl->target_info.persist) { - val = remove_breakpoint (bl, mark_uninserted); + val = remove_breakpoint (bl); if (val != 0) return val; } @@ -4081,7 +4081,7 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) } static int -remove_breakpoint (struct bp_location *bl, insertion_state_t is) +remove_breakpoint (struct bp_location *bl) { int ret; struct cleanup *old_chain; @@ -4097,7 +4097,7 @@ remove_breakpoint (struct bp_location *bl, insertion_state_t is) switch_to_program_space_and_thread (bl->pspace); - ret = remove_breakpoint_1 (bl, is); + ret = remove_breakpoint_1 (bl, mark_uninserted); do_cleanups (old_chain); return ret; @@ -12572,7 +12572,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) if (!keep_in_target) { - if (remove_breakpoint (old_loc, mark_uninserted)) + if (remove_breakpoint (old_loc)) { /* This is just about all we can do. We could keep this location on the global list, and try to -- 1.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint 2016-02-14 18:37 [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves 2016-02-14 18:37 ` [PATCH 1/4] Simplify remove_breakpoint interface Pedro Alves @ 2016-02-14 18:37 ` Pedro Alves 2016-02-15 9:12 ` Yao Qi 2016-02-14 18:37 ` [PATCH 2/4] Introduce 'enum remove_bp_reason' Pedro Alves ` (2 subsequent siblings) 4 siblings, 1 reply; 8+ messages in thread From: Pedro Alves @ 2016-02-14 18:37 UTC (permalink / raw) To: gdb-patches So the target knows whether we're detaching breakpoints. Nothing uses the parameter in this patch yet. gdb/ChangeLog: 2016-02-14 Pedro Alves <palves@redhat.com> * break-catch-sig.c (signal_catchpoint_remove_location): Adjust interface. * break-catch-syscall.c (remove_catch_syscall): * breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h. (remove_breakpoint_1): Pass 'reason' down. (remove_catch_fork, remove_catch_vfork, remove_catch_solib) (remove_catch_exec, remove_watchpoint, remove_masked_watchpoint) (base_breakpoint_remove_location, bkpt_remove_location) (bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust interface. * breakpoint.h (enum remove_bp_reason): Moved here from breakpoint.c. (struct breakpoint_ops) <remove_location>: Add 'reason' parameter. * corelow.c (core_remove_breakpoint): New function. (init_core_ops): Install it as to_remove_breakpoint method. * exec.c (exec_remove_breakpoint): New function. (init_exec_ops): Install it as to_remove_breakpoint method. * mem-break.c (memory_remove_breakpoint): Adjust interface. * record-btrace.c (record_btrace_remove_breakpoint): Adjust interface. * record-full.c (record_full_remove_breakpoint) (record_full_core_remove_breakpoint): Adjust interface. * remote.c (remote_remove_breakpoint): Adjust interface. * target-debug.h (target_debug_print_enum_remove_bp_reason): New macro. * target-delegates.c: Regenerate. * target.c (target_remove_breakpoint): Add 'reason' parameter. * target.h (struct target_ops) <to_remove_breakpoint>: Add 'reason' parameter. (target_remove_breakpoint, memory_remove_breakpoint): Add 'reason' parameter. --- gdb/break-catch-sig.c | 3 ++- gdb/break-catch-syscall.c | 2 +- gdb/breakpoint.c | 47 +++++++++++++++++++---------------------------- gdb/breakpoint.h | 14 +++++++++++++- gdb/corelow.c | 10 +++++++++- gdb/exec.c | 10 +++++++++- gdb/mem-break.c | 3 ++- gdb/record-btrace.c | 6 ++++-- gdb/record-full.c | 8 +++++--- gdb/remote.c | 5 +++-- gdb/target-debug.h | 2 ++ gdb/target-delegates.c | 10 ++++++---- gdb/target.c | 5 +++-- gdb/target.h | 9 ++++++--- 14 files changed, 84 insertions(+), 50 deletions(-) diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c index dcfae42..06ee44c 100644 --- a/gdb/break-catch-sig.c +++ b/gdb/break-catch-sig.c @@ -137,7 +137,8 @@ signal_catchpoint_insert_location (struct bp_location *bl) catchpoints. */ static int -signal_catchpoint_remove_location (struct bp_location *bl) +signal_catchpoint_remove_location (struct bp_location *bl, + enum remove_bp_reason reason) { struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner; int i; diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c index dbebdda..ec73a93 100644 --- a/gdb/break-catch-syscall.c +++ b/gdb/break-catch-syscall.c @@ -158,7 +158,7 @@ insert_catch_syscall (struct bp_location *bl) catchpoints. */ static int -remove_catch_syscall (struct bp_location *bl) +remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason) { struct syscall_catchpoint *c = (struct syscall_catchpoint *) bl->owner; struct inferior *inf = current_inferior (); diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 9b37508..311f06e 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -194,18 +194,6 @@ static void commands_command (char *, int); static void condition_command (char *, int); -/* Why are we removing the breakpoint from the target? */ - -enum remove_bp_reason -{ - /* A regular remove. Remove the breakpoint and forget everything - about it. */ - REMOVE_BREAKPOINT, - - /* Detach the breakpoints from a fork child. */ - DETACH_BREAKPOINT, -}; - static int remove_breakpoint (struct bp_location *); static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason); @@ -3989,7 +3977,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) && !memory_validate_breakpoint (bl->gdbarch, &bl->target_info)) val = 0; else - val = bl->owner->ops->remove_location (bl); + val = bl->owner->ops->remove_location (bl, reason); } else { @@ -4007,7 +3995,8 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) &bl->overlay_target_info); else target_remove_breakpoint (bl->gdbarch, - &bl->overlay_target_info); + &bl->overlay_target_info, + reason); } /* Did we set a breakpoint at the VMA? If so, we will have marked the breakpoint 'inserted'. */ @@ -4023,7 +4012,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) wrong code with the saved shadow contents. */ if (bl->loc_type == bp_loc_hardware_breakpoint || section_is_mapped (bl->section)) - val = bl->owner->ops->remove_location (bl); + val = bl->owner->ops->remove_location (bl, reason); else val = 0; } @@ -4061,7 +4050,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) && bl->owner->ops->remove_location != NULL); bl->inserted = (reason == DETACH_BREAKPOINT); - bl->owner->ops->remove_location (bl); + bl->owner->ops->remove_location (bl, reason); /* Failure to remove any of the hardware watchpoints comes here. */ if (reason == REMOVE_BREAKPOINT && bl->inserted) @@ -4075,7 +4064,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) gdb_assert (bl->owner->ops != NULL && bl->owner->ops->remove_location != NULL); - val = bl->owner->ops->remove_location (bl); + val = bl->owner->ops->remove_location (bl, reason); if (val) return val; @@ -8086,7 +8075,7 @@ insert_catch_fork (struct bp_location *bl) catchpoints. */ static int -remove_catch_fork (struct bp_location *bl) +remove_catch_fork (struct bp_location *bl, enum remove_bp_reason reason) { return target_remove_fork_catchpoint (ptid_get_pid (inferior_ptid)); } @@ -8204,7 +8193,7 @@ insert_catch_vfork (struct bp_location *bl) catchpoints. */ static int -remove_catch_vfork (struct bp_location *bl) +remove_catch_vfork (struct bp_location *bl, enum remove_bp_reason reason) { return target_remove_vfork_catchpoint (ptid_get_pid (inferior_ptid)); } @@ -8347,7 +8336,7 @@ insert_catch_solib (struct bp_location *ignore) } static int -remove_catch_solib (struct bp_location *ignore) +remove_catch_solib (struct bp_location *ignore, enum remove_bp_reason reason) { return 0; } @@ -8677,7 +8666,7 @@ insert_catch_exec (struct bp_location *bl) } static int -remove_catch_exec (struct bp_location *bl) +remove_catch_exec (struct bp_location *bl, enum remove_bp_reason reason) { return target_remove_exec_catchpoint (ptid_get_pid (inferior_ptid)); } @@ -10686,7 +10675,7 @@ insert_watchpoint (struct bp_location *bl) /* Implement the "remove" breakpoint_ops method for hardware watchpoints. */ static int -remove_watchpoint (struct bp_location *bl) +remove_watchpoint (struct bp_location *bl, enum remove_bp_reason reason) { struct watchpoint *w = (struct watchpoint *) bl->owner; int length = w->exact ? 1 : bl->length; @@ -10941,7 +10930,7 @@ insert_masked_watchpoint (struct bp_location *bl) masked hardware watchpoints. */ static int -remove_masked_watchpoint (struct bp_location *bl) +remove_masked_watchpoint (struct bp_location *bl, enum remove_bp_reason reason) { struct watchpoint *w = (struct watchpoint *) bl->owner; @@ -12934,7 +12923,8 @@ base_breakpoint_insert_location (struct bp_location *bl) } static int -base_breakpoint_remove_location (struct bp_location *bl) +base_breakpoint_remove_location (struct bp_location *bl, + enum remove_bp_reason reason) { internal_error_pure_virtual_called (); } @@ -13096,12 +13086,12 @@ bkpt_insert_location (struct bp_location *bl) } static int -bkpt_remove_location (struct bp_location *bl) +bkpt_remove_location (struct bp_location *bl, enum remove_bp_reason reason) { if (bl->loc_type == bp_loc_hardware_breakpoint) return target_remove_hw_breakpoint (bl->gdbarch, &bl->target_info); else - return target_remove_breakpoint (bl->gdbarch, &bl->target_info); + return target_remove_breakpoint (bl->gdbarch, &bl->target_info, reason); } static int @@ -13444,7 +13434,8 @@ bkpt_probe_insert_location (struct bp_location *bl) } static int -bkpt_probe_remove_location (struct bp_location *bl) +bkpt_probe_remove_location (struct bp_location *bl, + enum remove_bp_reason reason) { /* Let's clear the semaphore before removing the location. */ if (bl->probe.probe->pops->clear_semaphore != NULL) @@ -13452,7 +13443,7 @@ bkpt_probe_remove_location (struct bp_location *bl) bl->probe.objfile, bl->gdbarch); - return bkpt_remove_location (bl); + return bkpt_remove_location (bl, reason); } static void diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 054eab4d..c7d7462 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -39,6 +39,18 @@ struct linespec_result; struct linespec_sals; struct event_location; +/* Why are we removing the breakpoint from the target? */ + +enum remove_bp_reason +{ + /* A regular remove. Remove the breakpoint and forget everything + about it. */ + REMOVE_BREAKPOINT, + + /* Detach the breakpoints from a fork child. */ + DETACH_BREAKPOINT, +}; + /* This is the maximum number of bytes a breakpoint instruction can take. Feel free to increase it. It's just used in a few places to size arrays that should be independent of the target @@ -519,7 +531,7 @@ struct breakpoint_ops with the "insert" method above. Return 0 for success, 1 if the breakpoint, watchpoint or catchpoint type is not supported, -1 for failure. */ - int (*remove_location) (struct bp_location *); + int (*remove_location) (struct bp_location *, enum remove_bp_reason reason); /* Return true if it the target has stopped due to hitting breakpoint location BL. This function does not check if we diff --git a/gdb/corelow.c b/gdb/corelow.c index 224719e..e8c9cc8 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -920,6 +920,14 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch, return 0; } +static int +core_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) +{ + return 0; +} + /* Okay, let's be honest: threads gleaned from a core file aren't exactly lively, are they? On the other hand, if we don't claim @@ -1043,7 +1051,7 @@ init_core_ops (void) core_ops.to_xfer_partial = core_xfer_partial; core_ops.to_files_info = core_files_info; core_ops.to_insert_breakpoint = ignore; - core_ops.to_remove_breakpoint = ignore; + core_ops.to_remove_breakpoint = core_remove_breakpoint; core_ops.to_thread_alive = core_thread_alive; core_ops.to_read_description = core_read_description; core_ops.to_pid_to_str = core_pid_to_str; diff --git a/gdb/exec.c b/gdb/exec.c index 23c89c2..4dba3e1 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -1006,6 +1006,14 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch, } static int +exec_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) +{ + return 0; +} + +static int exec_has_memory (struct target_ops *ops) { /* We can provide memory if we have any file/target sections to read @@ -1036,7 +1044,7 @@ Specify the filename of the executable file."; exec_ops.to_get_section_table = exec_get_section_table; exec_ops.to_files_info = exec_files_info; exec_ops.to_insert_breakpoint = ignore; - exec_ops.to_remove_breakpoint = ignore; + exec_ops.to_remove_breakpoint = exec_remove_breakpoint; exec_ops.to_stratum = file_stratum; exec_ops.to_has_memory = exec_has_memory; exec_ops.to_make_corefile_notes = exec_make_note_section; diff --git a/gdb/mem-break.c b/gdb/mem-break.c index 9bb4c45..803f62b 100644 --- a/gdb/mem-break.c +++ b/gdb/mem-break.c @@ -93,7 +93,8 @@ memory_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, int memory_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { return gdbarch_memory_remove_breakpoint (gdbarch, bp_tgt); } diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index 77b5180..6a5ffa4 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -1406,7 +1406,8 @@ record_btrace_insert_breakpoint (struct target_ops *ops, static int record_btrace_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { const char *old; int ret; @@ -1419,7 +1420,8 @@ record_btrace_remove_breakpoint (struct target_ops *ops, ret = 0; TRY { - ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt); + ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt, + reason); } CATCH (except, RETURN_MASK_ALL) { diff --git a/gdb/record-full.c b/gdb/record-full.c index f6023bf..418a346 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -1684,7 +1684,8 @@ record_full_insert_breakpoint (struct target_ops *ops, static int record_full_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { struct record_full_breakpoint *bp; int ix; @@ -1704,7 +1705,7 @@ record_full_remove_breakpoint (struct target_ops *ops, old_cleanups = record_full_gdb_operation_disable_set (); ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, - bp_tgt); + bp_tgt, reason); do_cleanups (old_cleanups); if (ret != 0) @@ -2171,7 +2172,8 @@ record_full_core_insert_breakpoint (struct target_ops *ops, static int record_full_core_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { return 0; } diff --git a/gdb/remote.c b/gdb/remote.c index 6d56f19..404a5c6 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -9339,7 +9339,8 @@ Target doesn't support breakpoints that have target side commands.")); static int remote_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { CORE_ADDR addr = bp_tgt->placed_address; struct remote_state *rs = get_remote_state (); @@ -9368,7 +9369,7 @@ remote_remove_breakpoint (struct target_ops *ops, return (rs->buf[0] == 'E'); } - return memory_remove_breakpoint (ops, gdbarch, bp_tgt); + return memory_remove_breakpoint (ops, gdbarch, bp_tgt, reason); } static enum Z_packet_type diff --git a/gdb/target-debug.h b/gdb/target-debug.h index 5f44d11..ef7e14d 100644 --- a/gdb/target-debug.h +++ b/gdb/target-debug.h @@ -158,6 +158,8 @@ target_debug_do_print (plongest (X)) #define target_debug_print_struct_inferior_p(X) \ target_debug_do_print (host_address_to_string (X)) +#define target_debug_print_enum_remove_bp_reason(X) \ + target_debug_do_print (plongest (X)) static void target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status) diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index d23bc75..e21746f 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -262,24 +262,26 @@ debug_insert_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct b } static int -delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2) +delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3) { self = self->beneath; - return self->to_remove_breakpoint (self, arg1, arg2); + return self->to_remove_breakpoint (self, arg1, arg2, arg3); } static int -debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2) +debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3) { int result; fprintf_unfiltered (gdb_stdlog, "-> %s->to_remove_breakpoint (...)\n", debug_target.to_shortname); - result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2); + result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2, arg3); fprintf_unfiltered (gdb_stdlog, "<- %s->to_remove_breakpoint (", debug_target.to_shortname); target_debug_print_struct_target_ops_p (&debug_target); fputs_unfiltered (", ", gdb_stdlog); target_debug_print_struct_gdbarch_p (arg1); fputs_unfiltered (", ", gdb_stdlog); target_debug_print_struct_bp_target_info_p (arg2); + fputs_unfiltered (", ", gdb_stdlog); + target_debug_print_enum_remove_bp_reason (arg3); fputs_unfiltered (") = ", gdb_stdlog); target_debug_print_int (result); fputs_unfiltered ("\n", gdb_stdlog); diff --git a/gdb/target.c b/gdb/target.c index ac66a3a..a0d5937 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2081,7 +2081,8 @@ target_insert_breakpoint (struct gdbarch *gdbarch, int target_remove_breakpoint (struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { /* This is kind of a weird case to handle, but the permission might have been changed after breakpoints were inserted - in which case @@ -2094,7 +2095,7 @@ target_remove_breakpoint (struct gdbarch *gdbarch, } return current_target.to_remove_breakpoint (¤t_target, - gdbarch, bp_tgt); + gdbarch, bp_tgt, reason); } static void diff --git a/gdb/target.h b/gdb/target.h index e1419a9..7c8513a 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -478,7 +478,8 @@ struct target_ops struct bp_target_info *) TARGET_DEFAULT_FUNC (memory_insert_breakpoint); int (*to_remove_breakpoint) (struct target_ops *, struct gdbarch *, - struct bp_target_info *) + struct bp_target_info *, + enum remove_bp_reason) TARGET_DEFAULT_FUNC (memory_remove_breakpoint); /* Returns true if the target stopped because it executed a @@ -1496,7 +1497,8 @@ extern int target_insert_breakpoint (struct gdbarch *gdbarch, machine. Result is 0 for success, non-zero for error. */ extern int target_remove_breakpoint (struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt); + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason); /* Returns true if the terminal settings of the inferior are in effect. */ @@ -2356,7 +2358,8 @@ extern struct target_section_table *target_get_section_table /* From mem-break.c */ extern int memory_remove_breakpoint (struct target_ops *, struct gdbarch *, - struct bp_target_info *); + struct bp_target_info *, + enum remove_bp_reason); extern int memory_insert_breakpoint (struct target_ops *, struct gdbarch *, struct bp_target_info *); -- 1.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint 2016-02-14 18:37 ` [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint Pedro Alves @ 2016-02-15 9:12 ` Yao Qi 2016-02-15 14:41 ` Pedro Alves 0 siblings, 1 reply; 8+ messages in thread From: Yao Qi @ 2016-02-15 9:12 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Pedro Alves <palves@redhat.com> writes: Hi Pedro, > So the target knows whether we're detaching breakpoints. > Nothing uses the parameter in this patch yet. > > gdb/ChangeLog: > 2016-02-14 Pedro Alves <palves@redhat.com> > > * break-catch-sig.c (signal_catchpoint_remove_location): Adjust > interface. > * break-catch-syscall.c (remove_catch_syscall): > * breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h. > (remove_breakpoint_1): Pass 'reason' down. > (remove_catch_fork, remove_catch_vfork, remove_catch_solib) > (remove_catch_exec, remove_watchpoint, remove_masked_watchpoint) > (base_breakpoint_remove_location, bkpt_remove_location) > (bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust > interface. > * breakpoint.h (enum remove_bp_reason): Moved here from > breakpoint.c. > (struct breakpoint_ops) <remove_location>: Add 'reason' parameter. > * corelow.c (core_remove_breakpoint): New function. > (init_core_ops): Install it as to_remove_breakpoint method. > * exec.c (exec_remove_breakpoint): New function. > (init_exec_ops): Install it as to_remove_breakpoint method. > * mem-break.c (memory_remove_breakpoint): Adjust interface. > * record-btrace.c (record_btrace_remove_breakpoint): Adjust > interface. > * record-full.c (record_full_remove_breakpoint) > (record_full_core_remove_breakpoint): Adjust interface. > * remote.c (remote_remove_breakpoint): Adjust interface. > * target-debug.h (target_debug_print_enum_remove_bp_reason): New > macro. > * target-delegates.c: Regenerate. > * target.c (target_remove_breakpoint): Add 'reason' parameter. > * target.h (struct target_ops) <to_remove_breakpoint>: Add > 'reason' parameter. > (target_remove_breakpoint, memory_remove_breakpoint): Add 'reason' > parameter. We also need to add 'reason' in m32r_remove_breakpoint, mips_remove_breakpoint and procfs_remove_breakpoint, otherwise the patch breaks the all-targets build. This patch series look good to me. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint 2016-02-15 9:12 ` Yao Qi @ 2016-02-15 14:41 ` Pedro Alves 0 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2016-02-15 14:41 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 02/15/2016 09:12 AM, Yao Qi wrote: > We also need to add 'reason' in m32r_remove_breakpoint, > mips_remove_breakpoint and procfs_remove_breakpoint, otherwise the patch > breaks the all-targets build. > > This patch series look good to me. Indeed. Thanks for noticing. I had a mental note to go over those, but then lost it before posting... :-) Here's the updated patch. I also added the missing comments to the new functions in exec.c and corelow.c. [PATCH] Plumb enum remove_bp_reason all the way to target_remove_breakpoint So the target knows whether we're detaching breakpoints. Nothing uses the parameter in this patch yet. gdb/ChangeLog: 2016-02-15 Pedro Alves <palves@redhat.com> * break-catch-sig.c (signal_catchpoint_remove_location): Adjust interface. * break-catch-syscall.c (remove_catch_syscall): * breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h. (remove_breakpoint_1): Pass 'reason' down. (remove_catch_fork, remove_catch_vfork, remove_catch_solib) (remove_catch_exec, remove_watchpoint, remove_masked_watchpoint) (base_breakpoint_remove_location, bkpt_remove_location) (bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust interface. * breakpoint.h (enum remove_bp_reason): Moved here from breakpoint.c. (struct breakpoint_ops) <remove_location>: Add 'reason' parameter. * corelow.c (core_remove_breakpoint): New function. (init_core_ops): Install it as to_remove_breakpoint method. * exec.c (exec_remove_breakpoint): New function. (init_exec_ops): Install it as to_remove_breakpoint method. * mem-break.c (memory_remove_breakpoint): Adjust interface. * record-btrace.c (record_btrace_remove_breakpoint): Adjust interface. * record-full.c (record_full_remove_breakpoint) (record_full_core_remove_breakpoint): Adjust interface. * remote-m32r.c (m32r_remove_breakpoint): Adjust interface. * remote-mips.c (mips_remove_breakpoint): Adjust interface. * remote.c (remote_remove_breakpoint): Adjust interface. * target-debug.h (target_debug_print_enum_remove_bp_reason): New macro. * target-delegates.c: Regenerate. * target.c (target_remove_breakpoint): Add 'reason' parameter. * target.h (struct target_ops) <to_remove_breakpoint>: Add 'reason' parameter. (target_remove_breakpoint, memory_remove_breakpoint): Add 'reason' parameter. --- gdb/break-catch-sig.c | 3 ++- gdb/break-catch-syscall.c | 2 +- gdb/breakpoint.c | 47 +++++++++++++++++++---------------------------- gdb/breakpoint.h | 14 +++++++++++++- gdb/corelow.c | 12 +++++++++++- gdb/exec.c | 12 +++++++++++- gdb/mem-break.c | 3 ++- gdb/nto-procfs.c | 3 ++- gdb/record-btrace.c | 6 ++++-- gdb/record-full.c | 8 +++++--- gdb/remote-m32r-sdi.c | 3 ++- gdb/remote-mips.c | 5 +++-- gdb/remote.c | 5 +++-- gdb/target-debug.h | 2 ++ gdb/target-delegates.c | 10 ++++++---- gdb/target.c | 5 +++-- gdb/target.h | 9 ++++++--- 17 files changed, 95 insertions(+), 54 deletions(-) diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c index dcfae42..06ee44c 100644 --- a/gdb/break-catch-sig.c +++ b/gdb/break-catch-sig.c @@ -137,7 +137,8 @@ signal_catchpoint_insert_location (struct bp_location *bl) catchpoints. */ static int -signal_catchpoint_remove_location (struct bp_location *bl) +signal_catchpoint_remove_location (struct bp_location *bl, + enum remove_bp_reason reason) { struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner; int i; diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c index dbebdda..ec73a93 100644 --- a/gdb/break-catch-syscall.c +++ b/gdb/break-catch-syscall.c @@ -158,7 +158,7 @@ insert_catch_syscall (struct bp_location *bl) catchpoints. */ static int -remove_catch_syscall (struct bp_location *bl) +remove_catch_syscall (struct bp_location *bl, enum remove_bp_reason reason) { struct syscall_catchpoint *c = (struct syscall_catchpoint *) bl->owner; struct inferior *inf = current_inferior (); diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 9b37508..311f06e 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -194,18 +194,6 @@ static void commands_command (char *, int); static void condition_command (char *, int); -/* Why are we removing the breakpoint from the target? */ - -enum remove_bp_reason -{ - /* A regular remove. Remove the breakpoint and forget everything - about it. */ - REMOVE_BREAKPOINT, - - /* Detach the breakpoints from a fork child. */ - DETACH_BREAKPOINT, -}; - static int remove_breakpoint (struct bp_location *); static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason); @@ -3989,7 +3977,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) && !memory_validate_breakpoint (bl->gdbarch, &bl->target_info)) val = 0; else - val = bl->owner->ops->remove_location (bl); + val = bl->owner->ops->remove_location (bl, reason); } else { @@ -4007,7 +3995,8 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) &bl->overlay_target_info); else target_remove_breakpoint (bl->gdbarch, - &bl->overlay_target_info); + &bl->overlay_target_info, + reason); } /* Did we set a breakpoint at the VMA? If so, we will have marked the breakpoint 'inserted'. */ @@ -4023,7 +4012,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) wrong code with the saved shadow contents. */ if (bl->loc_type == bp_loc_hardware_breakpoint || section_is_mapped (bl->section)) - val = bl->owner->ops->remove_location (bl); + val = bl->owner->ops->remove_location (bl, reason); else val = 0; } @@ -4061,7 +4050,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) && bl->owner->ops->remove_location != NULL); bl->inserted = (reason == DETACH_BREAKPOINT); - bl->owner->ops->remove_location (bl); + bl->owner->ops->remove_location (bl, reason); /* Failure to remove any of the hardware watchpoints comes here. */ if (reason == REMOVE_BREAKPOINT && bl->inserted) @@ -4075,7 +4064,7 @@ remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) gdb_assert (bl->owner->ops != NULL && bl->owner->ops->remove_location != NULL); - val = bl->owner->ops->remove_location (bl); + val = bl->owner->ops->remove_location (bl, reason); if (val) return val; @@ -8086,7 +8075,7 @@ insert_catch_fork (struct bp_location *bl) catchpoints. */ static int -remove_catch_fork (struct bp_location *bl) +remove_catch_fork (struct bp_location *bl, enum remove_bp_reason reason) { return target_remove_fork_catchpoint (ptid_get_pid (inferior_ptid)); } @@ -8204,7 +8193,7 @@ insert_catch_vfork (struct bp_location *bl) catchpoints. */ static int -remove_catch_vfork (struct bp_location *bl) +remove_catch_vfork (struct bp_location *bl, enum remove_bp_reason reason) { return target_remove_vfork_catchpoint (ptid_get_pid (inferior_ptid)); } @@ -8347,7 +8336,7 @@ insert_catch_solib (struct bp_location *ignore) } static int -remove_catch_solib (struct bp_location *ignore) +remove_catch_solib (struct bp_location *ignore, enum remove_bp_reason reason) { return 0; } @@ -8677,7 +8666,7 @@ insert_catch_exec (struct bp_location *bl) } static int -remove_catch_exec (struct bp_location *bl) +remove_catch_exec (struct bp_location *bl, enum remove_bp_reason reason) { return target_remove_exec_catchpoint (ptid_get_pid (inferior_ptid)); } @@ -10686,7 +10675,7 @@ insert_watchpoint (struct bp_location *bl) /* Implement the "remove" breakpoint_ops method for hardware watchpoints. */ static int -remove_watchpoint (struct bp_location *bl) +remove_watchpoint (struct bp_location *bl, enum remove_bp_reason reason) { struct watchpoint *w = (struct watchpoint *) bl->owner; int length = w->exact ? 1 : bl->length; @@ -10941,7 +10930,7 @@ insert_masked_watchpoint (struct bp_location *bl) masked hardware watchpoints. */ static int -remove_masked_watchpoint (struct bp_location *bl) +remove_masked_watchpoint (struct bp_location *bl, enum remove_bp_reason reason) { struct watchpoint *w = (struct watchpoint *) bl->owner; @@ -12934,7 +12923,8 @@ base_breakpoint_insert_location (struct bp_location *bl) } static int -base_breakpoint_remove_location (struct bp_location *bl) +base_breakpoint_remove_location (struct bp_location *bl, + enum remove_bp_reason reason) { internal_error_pure_virtual_called (); } @@ -13096,12 +13086,12 @@ bkpt_insert_location (struct bp_location *bl) } static int -bkpt_remove_location (struct bp_location *bl) +bkpt_remove_location (struct bp_location *bl, enum remove_bp_reason reason) { if (bl->loc_type == bp_loc_hardware_breakpoint) return target_remove_hw_breakpoint (bl->gdbarch, &bl->target_info); else - return target_remove_breakpoint (bl->gdbarch, &bl->target_info); + return target_remove_breakpoint (bl->gdbarch, &bl->target_info, reason); } static int @@ -13444,7 +13434,8 @@ bkpt_probe_insert_location (struct bp_location *bl) } static int -bkpt_probe_remove_location (struct bp_location *bl) +bkpt_probe_remove_location (struct bp_location *bl, + enum remove_bp_reason reason) { /* Let's clear the semaphore before removing the location. */ if (bl->probe.probe->pops->clear_semaphore != NULL) @@ -13452,7 +13443,7 @@ bkpt_probe_remove_location (struct bp_location *bl) bl->probe.objfile, bl->gdbarch); - return bkpt_remove_location (bl); + return bkpt_remove_location (bl, reason); } static void diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 054eab4d..c7d7462 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -39,6 +39,18 @@ struct linespec_result; struct linespec_sals; struct event_location; +/* Why are we removing the breakpoint from the target? */ + +enum remove_bp_reason +{ + /* A regular remove. Remove the breakpoint and forget everything + about it. */ + REMOVE_BREAKPOINT, + + /* Detach the breakpoints from a fork child. */ + DETACH_BREAKPOINT, +}; + /* This is the maximum number of bytes a breakpoint instruction can take. Feel free to increase it. It's just used in a few places to size arrays that should be independent of the target @@ -519,7 +531,7 @@ struct breakpoint_ops with the "insert" method above. Return 0 for success, 1 if the breakpoint, watchpoint or catchpoint type is not supported, -1 for failure. */ - int (*remove_location) (struct bp_location *); + int (*remove_location) (struct bp_location *, enum remove_bp_reason reason); /* Return true if it the target has stopped due to hitting breakpoint location BL. This function does not check if we diff --git a/gdb/corelow.c b/gdb/corelow.c index 224719e..376b7c9 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -920,6 +920,16 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch, return 0; } +/* Implement the to_remove_breakpoint method. */ + +static int +core_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) +{ + return 0; +} + /* Okay, let's be honest: threads gleaned from a core file aren't exactly lively, are they? On the other hand, if we don't claim @@ -1043,7 +1053,7 @@ init_core_ops (void) core_ops.to_xfer_partial = core_xfer_partial; core_ops.to_files_info = core_files_info; core_ops.to_insert_breakpoint = ignore; - core_ops.to_remove_breakpoint = ignore; + core_ops.to_remove_breakpoint = core_remove_breakpoint; core_ops.to_thread_alive = core_thread_alive; core_ops.to_read_description = core_read_description; core_ops.to_pid_to_str = core_pid_to_str; diff --git a/gdb/exec.c b/gdb/exec.c index 23c89c2..d7f42a2 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -1005,6 +1005,16 @@ ignore (struct target_ops *ops, struct gdbarch *gdbarch, return 0; } +/* Implement the to_remove_breakpoint method. */ + +static int +exec_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) +{ + return 0; +} + static int exec_has_memory (struct target_ops *ops) { @@ -1036,7 +1046,7 @@ Specify the filename of the executable file."; exec_ops.to_get_section_table = exec_get_section_table; exec_ops.to_files_info = exec_files_info; exec_ops.to_insert_breakpoint = ignore; - exec_ops.to_remove_breakpoint = ignore; + exec_ops.to_remove_breakpoint = exec_remove_breakpoint; exec_ops.to_stratum = file_stratum; exec_ops.to_has_memory = exec_has_memory; exec_ops.to_make_corefile_notes = exec_make_note_section; diff --git a/gdb/mem-break.c b/gdb/mem-break.c index 9bb4c45..803f62b 100644 --- a/gdb/mem-break.c +++ b/gdb/mem-break.c @@ -93,7 +93,8 @@ memory_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, int memory_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { return gdbarch_memory_remove_breakpoint (gdbarch, bp_tgt); } diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c index 992de29..cd715d2 100644 --- a/gdb/nto-procfs.c +++ b/gdb/nto-procfs.c @@ -1015,7 +1015,8 @@ procfs_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, static int procfs_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { return procfs_breakpoint (bp_tgt->placed_address, _DEBUG_BREAK_EXEC, -1); } diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index 77b5180..6a5ffa4 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -1406,7 +1406,8 @@ record_btrace_insert_breakpoint (struct target_ops *ops, static int record_btrace_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { const char *old; int ret; @@ -1419,7 +1420,8 @@ record_btrace_remove_breakpoint (struct target_ops *ops, ret = 0; TRY { - ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt); + ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, bp_tgt, + reason); } CATCH (except, RETURN_MASK_ALL) { diff --git a/gdb/record-full.c b/gdb/record-full.c index f6023bf..418a346 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -1684,7 +1684,8 @@ record_full_insert_breakpoint (struct target_ops *ops, static int record_full_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { struct record_full_breakpoint *bp; int ix; @@ -1704,7 +1705,7 @@ record_full_remove_breakpoint (struct target_ops *ops, old_cleanups = record_full_gdb_operation_disable_set (); ret = ops->beneath->to_remove_breakpoint (ops->beneath, gdbarch, - bp_tgt); + bp_tgt, reason); do_cleanups (old_cleanups); if (ret != 0) @@ -2171,7 +2172,8 @@ record_full_core_insert_breakpoint (struct target_ops *ops, static int record_full_core_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { return 0; } diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c index c1bd858..25e51f0 100644 --- a/gdb/remote-m32r-sdi.c +++ b/gdb/remote-m32r-sdi.c @@ -1216,7 +1216,8 @@ m32r_insert_breakpoint (struct target_ops *ops, static int m32r_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { CORE_ADDR addr = bp_tgt->placed_address; int i; diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c index 2f6663b..31aae9b 100644 --- a/gdb/remote-mips.c +++ b/gdb/remote-mips.c @@ -2387,13 +2387,14 @@ mips_insert_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, static int mips_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { if (monitor_supports_breakpoints) return mips_clear_breakpoint (bp_tgt->placed_address, MIPS_INSN32_SIZE, BREAK_FETCH); else - return memory_remove_breakpoint (ops, gdbarch, bp_tgt); + return memory_remove_breakpoint (ops, gdbarch, bp_tgt, reason); } /* Tell whether this target can support a hardware breakpoint. CNT diff --git a/gdb/remote.c b/gdb/remote.c index 6d56f19..404a5c6 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -9339,7 +9339,8 @@ Target doesn't support breakpoints that have target side commands.")); static int remote_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { CORE_ADDR addr = bp_tgt->placed_address; struct remote_state *rs = get_remote_state (); @@ -9368,7 +9369,7 @@ remote_remove_breakpoint (struct target_ops *ops, return (rs->buf[0] == 'E'); } - return memory_remove_breakpoint (ops, gdbarch, bp_tgt); + return memory_remove_breakpoint (ops, gdbarch, bp_tgt, reason); } static enum Z_packet_type diff --git a/gdb/target-debug.h b/gdb/target-debug.h index 5f44d11..ef7e14d 100644 --- a/gdb/target-debug.h +++ b/gdb/target-debug.h @@ -158,6 +158,8 @@ target_debug_do_print (plongest (X)) #define target_debug_print_struct_inferior_p(X) \ target_debug_do_print (host_address_to_string (X)) +#define target_debug_print_enum_remove_bp_reason(X) \ + target_debug_do_print (plongest (X)) static void target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status) diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index d23bc75..e21746f 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -262,24 +262,26 @@ debug_insert_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct b } static int -delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2) +delegate_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3) { self = self->beneath; - return self->to_remove_breakpoint (self, arg1, arg2); + return self->to_remove_breakpoint (self, arg1, arg2, arg3); } static int -debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2) +debug_remove_breakpoint (struct target_ops *self, struct gdbarch *arg1, struct bp_target_info *arg2, enum remove_bp_reason arg3) { int result; fprintf_unfiltered (gdb_stdlog, "-> %s->to_remove_breakpoint (...)\n", debug_target.to_shortname); - result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2); + result = debug_target.to_remove_breakpoint (&debug_target, arg1, arg2, arg3); fprintf_unfiltered (gdb_stdlog, "<- %s->to_remove_breakpoint (", debug_target.to_shortname); target_debug_print_struct_target_ops_p (&debug_target); fputs_unfiltered (", ", gdb_stdlog); target_debug_print_struct_gdbarch_p (arg1); fputs_unfiltered (", ", gdb_stdlog); target_debug_print_struct_bp_target_info_p (arg2); + fputs_unfiltered (", ", gdb_stdlog); + target_debug_print_enum_remove_bp_reason (arg3); fputs_unfiltered (") = ", gdb_stdlog); target_debug_print_int (result); fputs_unfiltered ("\n", gdb_stdlog); diff --git a/gdb/target.c b/gdb/target.c index ac66a3a..a0d5937 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2081,7 +2081,8 @@ target_insert_breakpoint (struct gdbarch *gdbarch, int target_remove_breakpoint (struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason) { /* This is kind of a weird case to handle, but the permission might have been changed after breakpoints were inserted - in which case @@ -2094,7 +2095,7 @@ target_remove_breakpoint (struct gdbarch *gdbarch, } return current_target.to_remove_breakpoint (¤t_target, - gdbarch, bp_tgt); + gdbarch, bp_tgt, reason); } static void diff --git a/gdb/target.h b/gdb/target.h index e1419a9..7c8513a 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -478,7 +478,8 @@ struct target_ops struct bp_target_info *) TARGET_DEFAULT_FUNC (memory_insert_breakpoint); int (*to_remove_breakpoint) (struct target_ops *, struct gdbarch *, - struct bp_target_info *) + struct bp_target_info *, + enum remove_bp_reason) TARGET_DEFAULT_FUNC (memory_remove_breakpoint); /* Returns true if the target stopped because it executed a @@ -1496,7 +1497,8 @@ extern int target_insert_breakpoint (struct gdbarch *gdbarch, machine. Result is 0 for success, non-zero for error. */ extern int target_remove_breakpoint (struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt); + struct bp_target_info *bp_tgt, + enum remove_bp_reason reason); /* Returns true if the terminal settings of the inferior are in effect. */ @@ -2356,7 +2358,8 @@ extern struct target_section_table *target_get_section_table /* From mem-break.c */ extern int memory_remove_breakpoint (struct target_ops *, struct gdbarch *, - struct bp_target_info *); + struct bp_target_info *, + enum remove_bp_reason); extern int memory_insert_breakpoint (struct target_ops *, struct gdbarch *, struct bp_target_info *); -- 1.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] Introduce 'enum remove_bp_reason' 2016-02-14 18:37 [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves 2016-02-14 18:37 ` [PATCH 1/4] Simplify remove_breakpoint interface Pedro Alves 2016-02-14 18:37 ` [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint Pedro Alves @ 2016-02-14 18:37 ` Pedro Alves 2016-02-14 18:44 ` [PATCH 4/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves 2016-08-10 22:11 ` [PATCH 0/4] " Pedro Alves 4 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2016-02-14 18:37 UTC (permalink / raw) To: gdb-patches Makes the code more obvious. gdb/ChangeLog: 2016-02-14 Pedro Alves <palves@redhat.com> * breakpoint.c (insertion_state_t): Delete. (enum remove_bp_reason): New. (detach_breakpoints, remove_breakpoint_1, remove_breakpoint): Adjust to use enum remove_bp_reason instead of insertion_state_t. --- gdb/breakpoint.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 05da8ed..9b37508 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -194,15 +194,20 @@ static void commands_command (char *, int); static void condition_command (char *, int); -typedef enum - { - mark_inserted, - mark_uninserted - } -insertion_state_t; +/* Why are we removing the breakpoint from the target? */ + +enum remove_bp_reason +{ + /* A regular remove. Remove the breakpoint and forget everything + about it. */ + REMOVE_BREAKPOINT, + + /* Detach the breakpoints from a fork child. */ + DETACH_BREAKPOINT, +}; static int remove_breakpoint (struct bp_location *); -static int remove_breakpoint_1 (struct bp_location *, insertion_state_t); +static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason); static enum print_stop_action print_bp_stop_message (bpstat bs); @@ -3922,7 +3927,7 @@ detach_breakpoints (ptid_t ptid) continue; if (bl->inserted) - val |= remove_breakpoint_1 (bl, mark_inserted); + val |= remove_breakpoint_1 (bl, DETACH_BREAKPOINT); } do_cleanups (old_chain); @@ -3936,7 +3941,7 @@ detach_breakpoints (ptid_t ptid) *not* look at bl->pspace->aspace here. */ static int -remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) +remove_breakpoint_1 (struct bp_location *bl, enum remove_bp_reason reason) { int val; @@ -4048,18 +4053,18 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) if (val) return val; - bl->inserted = (is == mark_inserted); + bl->inserted = (reason == DETACH_BREAKPOINT); } else if (bl->loc_type == bp_loc_hardware_watchpoint) { gdb_assert (bl->owner->ops != NULL && bl->owner->ops->remove_location != NULL); - bl->inserted = (is == mark_inserted); + bl->inserted = (reason == DETACH_BREAKPOINT); bl->owner->ops->remove_location (bl); /* Failure to remove any of the hardware watchpoints comes here. */ - if ((is == mark_uninserted) && (bl->inserted)) + if (reason == REMOVE_BREAKPOINT && bl->inserted) warning (_("Could not remove hardware watchpoint %d."), bl->owner->number); } @@ -4074,7 +4079,7 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) if (val) return val; - bl->inserted = (is == mark_inserted); + bl->inserted = (reason == DETACH_BREAKPOINT); } return 0; @@ -4097,7 +4102,7 @@ remove_breakpoint (struct bp_location *bl) switch_to_program_space_and_thread (bl->pspace); - ret = remove_breakpoint_1 (bl, mark_uninserted); + ret = remove_breakpoint_1 (bl, REMOVE_BREAKPOINT); do_cleanups (old_chain); return ret; -- 1.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/4] Fix PR gdb/19187 (process record over a fork causes internal error) 2016-02-14 18:37 [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves ` (2 preceding siblings ...) 2016-02-14 18:37 ` [PATCH 2/4] Introduce 'enum remove_bp_reason' Pedro Alves @ 2016-02-14 18:44 ` Pedro Alves 2016-08-10 22:11 ` [PATCH 0/4] " Pedro Alves 4 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2016-02-14 18:44 UTC (permalink / raw) To: gdb-patches Right after a fork is detected, we detach breakpoints from the child (detach_breakpoints), which calls into target_remove_breakpoint with inferior_ptid pointing at the child process, but leaves the breakpoint marked inserted (in the parent). The problem is that record-full.c always deletes all knowledge of the breakpoint. Then when we later really delete the breakpoint from the parent, we fail the assertion, since the breakpoint is unexpectedly not found in the record-full.c breakpoint table. The fix is simply to not forget about the breakpoint if we're detaching it from a fork child. gdb/ChangeLog: 2016-02-14 Pedro Alves <palves@redhat.com> PR gdb/19187 * record-full.c (record_full_remove_breakpoint): Don't remove the breakpoint from the record_full_breakpoints VEC if we're detaching the breakpoint from a fork child. gdb/testsuite/ChangeLog: 2016-02-14 Pedro Alves <palves@redhat.com> PR gdb/19187 * gdb.reverse/waitpid-reverse.exp: Add comment and remove setup_kfails. --- gdb/record-full.c | 7 +++++-- gdb/testsuite/gdb.reverse/waitpid-reverse.exp | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/gdb/record-full.c b/gdb/record-full.c index 418a346..e325869 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -1712,8 +1712,11 @@ record_full_remove_breakpoint (struct target_ops *ops, return ret; } - VEC_unordered_remove (record_full_breakpoint_p, - record_full_breakpoints, ix); + if (reason == REMOVE_BREAKPOINT) + { + VEC_unordered_remove (record_full_breakpoint_p, + record_full_breakpoints, ix); + } return 0; } } diff --git a/gdb/testsuite/gdb.reverse/waitpid-reverse.exp b/gdb/testsuite/gdb.reverse/waitpid-reverse.exp index d583953..e8a1690 100644 --- a/gdb/testsuite/gdb.reverse/waitpid-reverse.exp +++ b/gdb/testsuite/gdb.reverse/waitpid-reverse.exp @@ -18,6 +18,8 @@ # # This test tests waitpid syscall for reverse execution. # +# Also serves as regression test for gdb/19187 (recording across a +# fork). if ![supports_reverse] { return @@ -40,14 +42,12 @@ gdb_test "break marker2" \ "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \ "set breakpoint at marker2" -setup_kfail "gdb/19187" *-*-* gdb_continue_to_breakpoint "marker2" ".*$srcfile:.*" gdb_test "break marker1" \ "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \ "set breakpoint at marker1" -setup_kfail "gdb/19187" *-*-* gdb_test "reverse-continue" ".*$srcfile:$decimal.*" "reverse to marker1" # If the variable was recorded properly on syscall, the old contents (-1) -- 1.9.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error) 2016-02-14 18:37 [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves ` (3 preceding siblings ...) 2016-02-14 18:44 ` [PATCH 4/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves @ 2016-08-10 22:11 ` Pedro Alves 4 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2016-08-10 22:11 UTC (permalink / raw) To: gdb-patches Yao reminded me about this series, and asked about putting it in 7.12. I've done so now: pushed to master and 7.12 branch. Thanks, Pedro Alves On 02/14/2016 06:37 PM, Pedro Alves wrote: > This series fixes: > > (gdb) record > break marker2 > Breakpoint 2 at 0x40067a: file /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.reverse/waitpid-reverse.c, line 30. > (gdb) PASS: gdb.reverse/waitpid-reverse.exp: set breakpoint at marker2 > continue > Continuing. > /home/pedro/gdb/mygit/src/gdb/record-full.c:1720: internal-error: record_full_remove_breakpoint: removing unknown breakpoint > A problem internal to GDB has been detected, > further debugging may prove unreliable. > Quit this debugging session? (y or n) > KFAIL: gdb.reverse/waitpid-reverse.exp: continue to breakpoint: marker2 (GDB internal error) (PRMS: gdb/19187) > > The problem is that the target side can't distinguish between deleting > a breakpoint and detaching a breakpoint. > > Right after a fork is detected, we detach breakpoints from the child > (detach_breakpoints), which calls into target_remove_breakpoint with > inferior_ptid pointing at the child process, but leaves the breakpoint > marked inserted (in the parent). > > Since record-full.c's target_remove_breakpoint implementation doesn't > know we're detaching a breakpoint, it removes the breakpoint from its > own breakpoint table. > > Then when we later remove the breakpoint from the parent, we fail that > assertion, since the breakpoint is unexpectedly not found in the > record-full.c breakpoint table. > > The first three patches pass down the reason for removing the > breakpoint all the way to the target. > > Patch 4 then simply makes record-full.c not delete the breakpoint from > its breakpoint table when we're detaching the breakpoint from a fork > child. > > Pedro Alves (4): > Simplify remove_breakpoint interface > Introduce 'enum remove_bp_reason' > Plumb enum remove_bp_reason all the way to target_remove_breakpoint > Fix PR gdb/19187 (process record over a fork causes internal error) > > gdb/break-catch-sig.c | 3 +- > gdb/break-catch-syscall.c | 2 +- > gdb/breakpoint.c | 70 +++++++++++++-------------- > gdb/breakpoint.h | 14 +++++- > gdb/corelow.c | 10 +++- > gdb/exec.c | 10 +++- > gdb/mem-break.c | 3 +- > gdb/record-btrace.c | 6 ++- > gdb/record-full.c | 15 ++++-- > gdb/remote.c | 5 +- > gdb/target-debug.h | 2 + > gdb/target-delegates.c | 10 ++-- > gdb/target.c | 5 +- > gdb/target.h | 9 ++-- > gdb/testsuite/gdb.reverse/waitpid-reverse.exp | 4 +- > 15 files changed, 105 insertions(+), 63 deletions(-) > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-10 22:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-02-14 18:37 [PATCH 0/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves 2016-02-14 18:37 ` [PATCH 1/4] Simplify remove_breakpoint interface Pedro Alves 2016-02-14 18:37 ` [PATCH 3/4] Plumb enum remove_bp_reason all the way to target_remove_breakpoint Pedro Alves 2016-02-15 9:12 ` Yao Qi 2016-02-15 14:41 ` Pedro Alves 2016-02-14 18:37 ` [PATCH 2/4] Introduce 'enum remove_bp_reason' Pedro Alves 2016-02-14 18:44 ` [PATCH 4/4] Fix PR gdb/19187 (process record over a fork causes internal error) Pedro Alves 2016-08-10 22:11 ` [PATCH 0/4] " Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).