diff options
| -rw-r--r-- | gdb/ChangeLog | 38 | ||||
| -rw-r--r-- | gdb/breakpoint.c | 226 | ||||
| -rw-r--r-- | gdb/breakpoint.h | 13 | ||||
| -rw-r--r-- | gdb/infrun.c | 16 | ||||
| -rw-r--r-- | gdb/testsuite/ChangeLog | 11 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/breakpoint-in-ro-region.c | 9 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp | 104 |
7 files changed, 270 insertions, 147 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ef803bb..1affa2c 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,43 @@ 2014-10-15 Pedro Alves <palves@redhat.com> + PR breakpoints/9649 + * breakpoint.c (single_step_breakpoints, single_step_gdbarch): + Delete array globals. + (single_step_breakpoints): New global. + (breakpoint_xfer_memory): Remove special handling for single-step + breakpoints. + (update_breakpoints_after_exec): Delete bp_single_step + breakpoints. + (detach_breakpoints): Remove special handling for single-step + breakpoints. + (breakpoint_init_inferior): Delete bp_single_step breakpoints. + (bpstat_stop_status): Add comment. + (bpstat_what, bptype_string, print_one_breakpoint_location) + (adjust_breakpoint_address, init_bp_location): Handle + bp_single_step. + (new_single_step_breakpoint): New function. + (set_momentary_breakpoint, bkpt_remove_location): Remove special + handling for single-step breakpoints. + (insert_single_step_breakpoint, single_step_breakpoints_inserted) + (remove_single_step_breakpoints, cancel_single_step_breakpoints): + Rewrite. + (detach_single_step_breakpoints, find_single_step_breakpoint): + Delete functions. + (breakpoint_has_location_inserted_here): New function. + (single_step_breakpoint_inserted_here_p): Rewrite. + * breakpoint.h: Remove FIXME. + (enum bptype) <bp_single_step>: New enum value. + (insert_single_step_breakpoint): Update comment. + * infrun.c (resume_cleanups) + (delete_step_thread_step_resume_breakpoint): Remove single-step + breakpoints. + (fetch_inferior_event): Install a cleanup that removes infrun + breakpoints. + (switch_back_to_stepped_thread) <expect thread advanced also>: + Clear step-over info. + +2014-10-15 Pedro Alves <palves@redhat.com> + * infrun.c (delete_step_resume_breakpoint_callback): Delete. (delete_thread_infrun_breakpoints): New function, with parts salvaged from delete_step_resume_breakpoint_callback. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 2a6e51d..27420f5 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -225,11 +225,6 @@ static void stopat_command (char *arg, int from_tty); static void tcatch_command (char *arg, int from_tty); -static void detach_single_step_breakpoints (void); - -static int find_single_step_breakpoint (struct address_space *aspace, - CORE_ADDR pc); - static void free_bp_location (struct bp_location *loc); static void incref_bp_location (struct bp_location *loc); static void decref_bp_location (struct bp_location **loc); @@ -333,8 +328,7 @@ struct breakpoint_ops dprintf_breakpoint_ops; /* One (or perhaps two) breakpoints used for software single stepping. */ -static void *single_step_breakpoints[2]; -static struct gdbarch *single_step_gdbarch[2]; +static struct breakpoint *single_step_breakpoints; /* The style in which to perform a dynamic printf. This is a user option because different output options have different tradeoffs; @@ -1664,21 +1658,6 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf, one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org, memaddr, len, &bl->target_info, bl->gdbarch); } - - /* Now process single-step breakpoints. These are not found in the - bp_location array. */ - for (i = 0; i < 2; i++) - { - struct bp_target_info *bp_tgt = single_step_breakpoints[i]; - - if (bp_tgt != NULL) - { - struct gdbarch *gdbarch = single_step_gdbarch[i]; - - one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org, - memaddr, len, bp_tgt, gdbarch); - } - } } @@ -3792,6 +3771,13 @@ update_breakpoints_after_exec (void) continue; } + /* Just like single-step breakpoints. */ + if (b->type == bp_single_step) + { + delete_breakpoint (b); + continue; + } + /* Longjmp and longjmp-resume breakpoints are also meaningless after an exec. */ if (b->type == bp_longjmp || b->type == bp_longjmp_resume @@ -3884,9 +3870,6 @@ detach_breakpoints (ptid_t ptid) val |= remove_breakpoint_1 (bl, mark_inserted); } - /* Detach single-step breakpoints as well. */ - detach_single_step_breakpoints (); - do_cleanups (old_chain); return val; } @@ -4156,6 +4139,10 @@ breakpoint_init_inferior (enum inf_context context) /* Also remove step-resume breakpoints. */ + case bp_single_step: + + /* Also remove single-step breakpoints. */ + delete_breakpoint (b); break; @@ -5632,6 +5619,7 @@ bpstat_stop_status (struct address_space *aspace, } } + /* Check if a moribund breakpoint explains the stop. */ for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix) { if (breakpoint_location_address_match (loc, aspace, bp_addr)) @@ -5787,6 +5775,7 @@ bpstat_what (bpstat bs_head) break; case bp_breakpoint: case bp_hardware_breakpoint: + case bp_single_step: case bp_until: case bp_finish: case bp_shlib_event: @@ -6145,6 +6134,7 @@ bptype_string (enum bptype type) {bp_none, "?deleted?"}, {bp_breakpoint, "breakpoint"}, {bp_hardware_breakpoint, "hw breakpoint"}, + {bp_single_step, "sw single-step"}, {bp_until, "until"}, {bp_finish, "finish"}, {bp_watchpoint, "watchpoint"}, @@ -6336,6 +6326,7 @@ print_one_breakpoint_location (struct breakpoint *b, case bp_breakpoint: case bp_hardware_breakpoint: + case bp_single_step: case bp_until: case bp_finish: case bp_longjmp: @@ -7178,6 +7169,16 @@ adjust_breakpoint_address (struct gdbarch *gdbarch, have their addresses modified. */ return bpaddr; } + else if (bptype == bp_single_step) + { + /* Single-step breakpoints should not have their addresses + modified. If there's any architectural constrain that + applies to this address, then it should have already been + taken into account when the breakpoint was created in the + first place. If we didn't do this, stepping through e.g., + Thumb-2 IT blocks would break. */ + return bpaddr; + } else { CORE_ADDR adjusted_bpaddr; @@ -7214,6 +7215,7 @@ init_bp_location (struct bp_location *loc, const struct bp_location_ops *ops, switch (owner->type) { case bp_breakpoint: + case bp_single_step: case bp_until: case bp_finish: case bp_longjmp: @@ -9196,10 +9198,31 @@ enable_breakpoints_after_startup (void) breakpoint_re_set (); } +/* Create a new single-step breakpoint for thread THREAD, with no + locations. */ -/* Set a breakpoint that will evaporate an end of command - at address specified by SAL. - Restrict it to frame FRAME if FRAME is nonzero. */ +static struct breakpoint * +new_single_step_breakpoint (int thread, struct gdbarch *gdbarch) +{ + struct breakpoint *b = XNEW (struct breakpoint); + + init_raw_breakpoint_without_location (b, gdbarch, bp_single_step, + &momentary_breakpoint_ops); + + b->disposition = disp_donttouch; + b->frame_id = null_frame_id; + + b->thread = thread; + gdb_assert (b->thread != 0); + + add_to_breakpoint_chain (b); + + return b; +} + +/* Set a momentary breakpoint of type TYPE at address specified by + SAL. If FRAME_ID is valid, the breakpoint is restricted to that + frame. */ struct breakpoint * set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal, @@ -13324,28 +13347,9 @@ static int bkpt_insert_location (struct bp_location *bl) { if (bl->loc_type == bp_loc_hardware_breakpoint) - return target_insert_hw_breakpoint (bl->gdbarch, - &bl->target_info); + return target_insert_hw_breakpoint (bl->gdbarch, &bl->target_info); else - { - struct bp_target_info *bp_tgt = &bl->target_info; - int ret; - int sss_slot; - - /* There is no need to insert a breakpoint if an unconditional - raw/sss breakpoint is already inserted at that location. */ - sss_slot = find_single_step_breakpoint (bp_tgt->placed_address_space, - bp_tgt->reqstd_address); - if (sss_slot >= 0) - { - struct bp_target_info *sss_bp_tgt = single_step_breakpoints[sss_slot]; - - bp_target_info_copy_insertion_state (bp_tgt, sss_bp_tgt); - return 0; - } - - return target_insert_breakpoint (bl->gdbarch, bp_tgt); - } + return target_insert_breakpoint (bl->gdbarch, &bl->target_info); } static int @@ -13354,19 +13358,7 @@ bkpt_remove_location (struct bp_location *bl) if (bl->loc_type == bp_loc_hardware_breakpoint) return target_remove_hw_breakpoint (bl->gdbarch, &bl->target_info); else - { - struct bp_target_info *bp_tgt = &bl->target_info; - struct address_space *aspace = bp_tgt->placed_address_space; - CORE_ADDR address = bp_tgt->reqstd_address; - - /* Only remove the breakpoint if there is no raw/sss breakpoint - still inserted at this location. Otherwise, we would be - effectively disabling the raw/sss breakpoint. */ - if (single_step_breakpoint_inserted_here_p (aspace, address)) - return 0; - - return target_remove_breakpoint (bl->gdbarch, bp_tgt); - } + return target_remove_breakpoint (bl->gdbarch, &bl->target_info); } static int @@ -15454,31 +15446,20 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch, struct address_space *aspace, CORE_ADDR next_pc) { - void **bpt_p; + struct thread_info *tp = inferior_thread (); + struct symtab_and_line sal; + CORE_ADDR pc = next_pc; - if (single_step_breakpoints[0] == NULL) - { - bpt_p = &single_step_breakpoints[0]; - single_step_gdbarch[0] = gdbarch; - } - else - { - gdb_assert (single_step_breakpoints[1] == NULL); - bpt_p = &single_step_breakpoints[1]; - single_step_gdbarch[1] = gdbarch; - } + if (single_step_breakpoints == NULL) + single_step_breakpoints = new_single_step_breakpoint (tp->num, gdbarch); - /* NOTE drow/2006-04-11: A future improvement to this function would - be to only create the breakpoints once, and actually put them on - the breakpoint chain. That would let us use set_raw_breakpoint. - We could adjust the addresses each time they were needed. Doing - this requires corresponding changes elsewhere where single step - breakpoints are handled, however. So, for now, we use this. */ + sal = find_pc_line (pc, 0); + sal.pc = pc; + sal.section = find_pc_overlay (pc); + sal.explicit_pc = 1; + add_location_to_breakpoint (single_step_breakpoints, &sal); - *bpt_p = deprecated_insert_raw_breakpoint (gdbarch, aspace, next_pc); - if (*bpt_p == NULL) - error (_("Could not insert single-step breakpoint at %s"), - paddress (gdbarch, next_pc)); + update_global_location_list (UGLL_INSERT); } /* Check if the breakpoints used for software single stepping @@ -15487,8 +15468,7 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch, int single_step_breakpoints_inserted (void) { - return (single_step_breakpoints[0] != NULL - || single_step_breakpoints[1] != NULL); + return (single_step_breakpoints != NULL); } /* Remove and delete any breakpoints used for software single step. */ @@ -15496,22 +15476,11 @@ single_step_breakpoints_inserted (void) void remove_single_step_breakpoints (void) { - gdb_assert (single_step_breakpoints[0] != NULL); + gdb_assert (single_step_breakpoints != NULL); - /* See insert_single_step_breakpoint for more about this deprecated - call. */ - deprecated_remove_raw_breakpoint (single_step_gdbarch[0], - single_step_breakpoints[0]); - single_step_gdbarch[0] = NULL; - single_step_breakpoints[0] = NULL; + delete_breakpoint (single_step_breakpoints); - if (single_step_breakpoints[1] != NULL) - { - deprecated_remove_raw_breakpoint (single_step_gdbarch[1], - single_step_breakpoints[1]); - single_step_gdbarch[1] = NULL; - single_step_breakpoints[1] = NULL; - } + single_step_breakpoints = NULL; } /* Delete software single step breakpoints without removing them from @@ -15522,51 +15491,28 @@ remove_single_step_breakpoints (void) void cancel_single_step_breakpoints (void) { - int i; - - for (i = 0; i < 2; i++) - if (single_step_breakpoints[i]) - { - xfree (single_step_breakpoints[i]); - single_step_breakpoints[i] = NULL; - single_step_gdbarch[i] = NULL; - } -} - -/* Detach software single-step breakpoints from INFERIOR_PTID without - removing them. */ - -static void -detach_single_step_breakpoints (void) -{ - int i; - - for (i = 0; i < 2; i++) - if (single_step_breakpoints[i]) - target_remove_breakpoint (single_step_gdbarch[i], - single_step_breakpoints[i]); + /* We don't really need to (or should) delete them here. After an + exit, breakpoint_init_inferior deletes it. After an exec, + update_breakpoints_after_exec does it. Just clear our + reference. */ + single_step_breakpoints = NULL; } -/* Find the software single-step breakpoint that inserted at PC. - Returns its slot if found, and -1 if not found. */ +/* Check whether any location of BP is inserted at PC. */ static int -find_single_step_breakpoint (struct address_space *aspace, - CORE_ADDR pc) +breakpoint_has_location_inserted_here (struct breakpoint *bp, + struct address_space *aspace, + CORE_ADDR pc) { - int i; + struct bp_location *loc; - for (i = 0; i < 2; i++) - { - struct bp_target_info *bp_tgt = single_step_breakpoints[i]; - if (bp_tgt - && breakpoint_address_match (bp_tgt->placed_address_space, - bp_tgt->reqstd_address, - aspace, pc)) - return i; - } + for (loc = bp->loc; loc != NULL; loc = loc->next) + if (loc->inserted + && breakpoint_location_address_match (loc, aspace, pc)) + return 1; - return -1; + return 0; } /* Check whether a software single-step breakpoint is inserted at @@ -15576,7 +15522,9 @@ int single_step_breakpoint_inserted_here_p (struct address_space *aspace, CORE_ADDR pc) { - return find_single_step_breakpoint (aspace, pc) >= 0; + return (single_step_breakpoints != NULL + && breakpoint_has_location_inserted_here (single_step_breakpoints, + aspace, pc)); } /* Returns 0 if 'bp' is NOT a syscall catchpoint, diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index b611057..7c563c1 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -47,18 +47,13 @@ struct linespec_sals; /* Type of breakpoint. */ -/* FIXME In the future, we should fold all other breakpoint-like - things into here. This includes: - - * single-step (for machines where we have to simulate single - stepping) (probably, though perhaps it is better for it to look as - much as possible like a single-step to wait_for_inferior). */ enum bptype { bp_none = 0, /* Eventpoint has been deleted */ bp_breakpoint, /* Normal breakpoint */ bp_hardware_breakpoint, /* Hardware assisted breakpoint */ + bp_single_step, /* Software single-step */ bp_until, /* used by until command */ bp_finish, /* used by finish command */ bp_watchpoint, /* Watchpoint */ @@ -1461,8 +1456,10 @@ extern void add_solib_catchpoint (char *arg, int is_load, int is_temp, deletes all breakpoints. */ extern void delete_command (char *arg, int from_tty); -/* Manage a software single step breakpoint (or two). Insert may be - called twice before remove is called. */ +/* Create and insert a new software single step breakpoint for the + current thread. May be called multiple times; each time will add a + new location to the set of potential addresses the next instruction + is at. */ extern void insert_single_step_breakpoint (struct gdbarch *, struct address_space *, CORE_ADDR); diff --git a/gdb/infrun.c b/gdb/infrun.c index aa36dbc..29c37e1 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1918,6 +1918,9 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid) static void resume_cleanups (void *ignore) { + if (single_step_breakpoints_inserted ()) + remove_single_step_breakpoints (); + normal_stop (); } @@ -2893,6 +2896,9 @@ static void delete_just_stopped_threads_infrun_breakpoints (void) { for_each_just_stopped_thread (delete_thread_infrun_breakpoints); + + if (single_step_breakpoints_inserted ()) + remove_single_step_breakpoints (); } /* A cleanup wrapper. */ @@ -3152,6 +3158,8 @@ fetch_inferior_event (void *client_data) still for the thread which has thrown the exception. */ make_bpstat_clear_actions_cleanup (); + make_cleanup (delete_just_stopped_threads_infrun_breakpoints_cleanup, NULL); + /* Now figure out what to do with the result of the result. */ handle_inferior_event (ecs); @@ -5525,6 +5533,14 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) fprintf_unfiltered (gdb_stdlog, "infrun: expected thread advanced also\n"); + /* Clear the info of the previous step-over, as it's no + longer valid. It's what keep_going would do too, if + we called it. Must do this before trying to insert + the sss breakpoint, otherwise if we were previously + trying to step over this exact address in another + thread, the breakpoint ends up not installed. */ + clear_step_over_info (); + insert_single_step_breakpoint (get_frame_arch (frame), get_frame_address_space (frame), stop_pc); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 800a483d..5264087 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,14 @@ +2014-10-15 Pedro Alves <palves@redhat.com> + + PR breakpoints/9649 + * gdb.base/breakpoint-in-ro-region.c (main): Add more instructions. + * gdb.base/breakpoint-in-ro-region.exp + (probe_target_hardware_step): New procedure. + (top level): Probe hardware stepping and hardware breakpoint + support. Test stepping through a read-only region, with both + "breakpoint auto-hw" on and off and both "always-inserted" on and + off. + 2014-10-15 Iain Buclaw <ibuclaw@gdcproject.org> * gdb.dlang/demangle.exp: Update for demangling changes. diff --git a/gdb/testsuite/gdb.base/breakpoint-in-ro-region.c b/gdb/testsuite/gdb.base/breakpoint-in-ro-region.c index 696a67d..2a999b1 100644 --- a/gdb/testsuite/gdb.base/breakpoint-in-ro-region.c +++ b/gdb/testsuite/gdb.base/breakpoint-in-ro-region.c @@ -23,6 +23,15 @@ main (void) i = 0; i = 0; i = 0; + i = 0; + i = 0; + i = 0; + i = 0; + i = 0; + i = 0; + i = 0; + i = 0; + i = 0; return 0; } diff --git a/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp b/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp index 8eacefa..1991aa5 100644 --- a/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp +++ b/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp @@ -27,6 +27,28 @@ if ![runto main] { delete_breakpoints +# Probe for hardware stepping. + +proc probe_target_hardware_step {} { + global gdb_prompt + + set hw_step 0 + + gdb_test_no_output "set debug target 1" + set test "probe target hardware step" + gdb_test_multiple "si" $test { + -re "to_resume \\(\[^\r\n\]+, step, .*$gdb_prompt $" { + set hw_step 1 + pass $test + } + -re "$gdb_prompt $" { + pass $test + } + } + gdb_test "set debug target 0" "->to_log_command.*\\).*" + return $hw_step +} + # Get the bounds of a function, and write them to FUNC_LO (inclusive), # FUNC_HI (exclusive). Return true on success and false on failure. proc get_function_bounds {function func_lo func_hi} { @@ -108,6 +130,7 @@ proc get_next_insn {} { return $next } +set hw_step [probe_target_hardware_step] if ![get_function_bounds "main" main_lo main_hi] { # Can't do the following tests if main's bounds are unknown. @@ -140,3 +163,84 @@ gdb_test "p /x *(char *) $main_lo = 1" \ gdb_test "break *$main_lo" \ "Cannot insert breakpoint .*Cannot set software breakpoint at read-only address $main_lo.*" \ "inserting software breakpoint in read-only memory fails" + +delete_breakpoints + +set supports_hbreak 0 +set test "probe hbreak support" +gdb_test_multiple "hbreak *$main_lo" $test { + -re "You may have requested too many.*$gdb_prompt $" { + pass "$test (no support)" + } + -re "No hardware breakpoint support.*$gdb_prompt $" { + pass "$test (no support)" + } + -re "$gdb_prompt $" { + pass "$test (support)" + set supports_hbreak 1 + } +} + +delete_breakpoints + +# Check that the "auto-hw on/off" setting affects single-step +# breakpoints as expected, by stepping through the read-only region. +# If the target does hardware stepping, we won't exercise that aspect, +# but we should be able to step through the region without seeing the +# hardware breakpoint or read-only address errors. +proc test_single_step { always_inserted auto_hw } { + global gdb_prompt + global decimal + global supports_hbreak + global hw_step + + gdb_test_no_output "set breakpoint always-inserted $always_inserted" + gdb_test_no_output "set breakpoint auto-hw $auto_hw" + + # Get the address of the current instruction so we know where SI is + # starting from. + set curr_insn [get_curr_insn] + + # Get the address of the next instruction so we know where SI should + # land. + set next_insn [get_next_insn] + + set test "step in ro region" + gdb_test_multiple "si" $test { + -re "Could not insert hardware breakpoints.*$gdb_prompt $" { + gdb_assert {!$hw_step && $auto_hw == "on" && !$supports_hbreak} \ + "$test (cannot insert hw break)" + } + -re "Cannot set software breakpoint at read-only address $next_insn.*$gdb_prompt $" { + gdb_assert {!$hw_step && $auto_hw == "off"} \ + "$test (cannot insert sw break)" + } + -re "^si\r\nNote: automatically using hardware breakpoints for read-only addresses\.\r\n${decimal}\[ \t\]+i = 0;\r\n$gdb_prompt $" { + gdb_assert {!$hw_step && $auto_hw == "on" && $supports_hbreak} \ + "$test (auto-hw)" + } + -re "^si\r\n${decimal}\[ \t\]+i = 0;\r\n$gdb_prompt $" { + gdb_assert {$hw_step || ($auto_hw == "on" && $supports_hbreak)} \ + "$test (no error)" + } + } + + gdb_test "maint info breakpoints 0" \ + "No breakpoint or watchpoint matching '0'\." \ + "single-step breakpoint is not left behind" + + # Confirm the thread really advanced. + if {$hw_step || ($auto_hw == "on" && $supports_hbreak)} { + gdb_test "p /x \$pc" " = $next_insn" "thread advanced" + } else { + gdb_test "p /x \$pc" " = $curr_insn" "thread did not advance" + } +} + +foreach always_inserted {"off" "on"} { + foreach auto_hw {"off" "on"} { + with_test_prefix "always-inserted $always_inserted: auto-hw $auto_hw" { + test_single_step $always_inserted $auto_hw + } + } +} |