From 2a7f3dffced7a20c992e1488d9f05fed8b8001fd Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 9 Feb 2016 12:12:17 +0000 Subject: [PATCH] Fix PR19548: Breakpoint re-set inserts breakpoints when it shouldn't PR19548 shows that we still have problems related to 13fd3ff34329: [PR17431: following execs with "breakpoint always-inserted on"] https://sourceware.org/ml/gdb-patches/2014-09/msg00733.html The problem this time is that we currently update the global location list and try to insert breakpoint locations after re-setting _each_ breakpoint in turn. Say: - We have _more_ than one breakpoint set. Let's assume 2. - There's a breakpoint with a pre-exec address that ends up being an unmapped address after the exec. - That breakpoint is NOT the first in the breakpoint list. Then when handling an exec, and we re-set the first breakpoint in the breakpoint list, we mistakently try to install the old pre-exec / un-re-set locations of the other breakpoint, which fails: (gdb) continue Continuing. process 28295 is executing new program: (...)/execl-update-breakpoints2 Error in re-setting breakpoint 1: Warning: Cannot insert breakpoint 2. Cannot access memory at address 0x1000764 Breakpoint 1, main (argc=1, argv=0x7fffffffd368) at /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/execl-update-breakpoints.c:34 34 len = strlen (argv[0]); (gdb) Fix this by deferring the global location list update till after all breakpoints are re-set. Tested on x86_64 Fedora 20, native and gdbserver. gdb/ChangeLog: 2016-02-09 Pedro Alves PR breakpoints/19548 * breakpoint.c (create_overlay_event_breakpoint): Don't update global location list here. (create_longjmp_master_breakpoint) (create_std_terminate_master_breakpoint) (create_exception_master_breakpoint, create_jit_event_breakpoint) (update_breakpoint_locations): (breakpoint_re_set): Update global location list after all breakpoints are re-set. gdb/testsuite/ChangeLog: 2016-02-09 Pedro Alves PR breakpoints/19548 * gdb.base/execl-update-breakpoints.c (some_function): New function. (main): Call it. * gdb.base/execl-update-breakpoints.exp: Add a second breakpoint. Tighten expected GDB output. --- gdb/ChangeLog | 12 +++++++ gdb/breakpoint.c | 25 ++++++--------- gdb/testsuite/ChangeLog | 9 ++++++ .../gdb.base/execl-update-breakpoints.c | 6 ++++ .../gdb.base/execl-update-breakpoints.exp | 31 +++++++++++++++++-- 5 files changed, 66 insertions(+), 17 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 6150f6c8aac..8e6e4ec2b25 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2016-02-09 Pedro Alves + + PR breakpoints/19548 + * breakpoint.c (create_overlay_event_breakpoint): Don't update + global location list here. + (create_longjmp_master_breakpoint) + (create_std_terminate_master_breakpoint) + (create_exception_master_breakpoint, create_jit_event_breakpoint) + (update_breakpoint_locations): + (breakpoint_re_set): Update global location list after all + breakpoints are re-set. + 2016-02-08 Simon Marchi * remote.c (remote_register_number_and_offset): Remove unused diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index afd90651932..c0598611382 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -3488,7 +3488,6 @@ create_overlay_event_breakpoint (void) overlay_events_enabled = 0; } } - update_global_location_list (UGLL_MAY_INSERT); } static void @@ -3602,7 +3601,6 @@ create_longjmp_master_breakpoint (void) } } } - update_global_location_list (UGLL_MAY_INSERT); do_cleanups (old_chain); } @@ -3661,8 +3659,6 @@ create_std_terminate_master_breakpoint (void) } } - update_global_location_list (UGLL_MAY_INSERT); - do_cleanups (old_chain); } @@ -3766,8 +3762,6 @@ create_exception_master_breakpoint (void) b->location = new_explicit_location (&explicit_loc); b->enable_state = bp_disabled; } - - update_global_location_list (UGLL_MAY_INSERT); } void @@ -7807,12 +7801,8 @@ struct lang_and_radix struct breakpoint * create_jit_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address) { - struct breakpoint *b; - - b = create_internal_breakpoint (gdbarch, address, bp_jit_event, - &internal_breakpoint_ops); - update_global_location_list_nothrow (UGLL_MAY_INSERT); - return b; + return create_internal_breakpoint (gdbarch, address, bp_jit_event, + &internal_breakpoint_ops); } /* Remove JIT code registration and unregistration breakpoint(s). */ @@ -14278,7 +14268,6 @@ update_breakpoint_locations (struct breakpoint *b, /* Ranged breakpoints have only one start location and one end location. */ b->enable_state = bp_disabled; - update_global_location_list (UGLL_MAY_INSERT); printf_unfiltered (_("Could not reset ranged breakpoint %d: " "multiple locations found\n"), b->number); @@ -14376,8 +14365,6 @@ update_breakpoint_locations (struct breakpoint *b, if (!locations_are_equal (existing_locations, b->loc)) observer_notify_breakpoint_modified (b); - - update_global_location_list (UGLL_MAY_INSERT); } /* Find the SaL locations corresponding to the given LOCATION. @@ -14617,6 +14604,11 @@ breakpoint_re_set (void) save_input_radix = input_radix; old_chain = save_current_space_and_thread (); + /* Note: we must not try to insert locations until after all + breakpoints have been re-set. Otherwise, e.g., when re-setting + breakpoint 1, we'd insert the locations of breakpoint 2, which + hadn't been re-set yet, and thus may have stale locations. */ + ALL_BREAKPOINTS_SAFE (b, b_tmp) { /* Format possible error msg. */ @@ -14637,6 +14629,9 @@ breakpoint_re_set (void) create_longjmp_master_breakpoint (); create_std_terminate_master_breakpoint (); create_exception_master_breakpoint (); + + /* Now we can insert. */ + update_global_location_list (UGLL_MAY_INSERT); } /* Reset the thread number of this breakpoint: diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 2a7193003a8..f8e08008806 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2016-02-09 Pedro Alves + + PR breakpoints/19548 + * gdb.base/execl-update-breakpoints.c (some_function): New + function. + (main): Call it. + * gdb.base/execl-update-breakpoints.exp: Add a second breakpoint. + Tighten expected GDB output. + 2016-02-08 Simon Marchi * Makefile.in (ALL_SUBDIRS): Remove. diff --git a/gdb/testsuite/gdb.base/execl-update-breakpoints.c b/gdb/testsuite/gdb.base/execl-update-breakpoints.c index 3978a70189c..38f1cea6ea0 100644 --- a/gdb/testsuite/gdb.base/execl-update-breakpoints.c +++ b/gdb/testsuite/gdb.base/execl-update-breakpoints.c @@ -20,6 +20,11 @@ #include #include +void +some_function (void) +{ +} + int main (int argc, char **argv) { @@ -34,5 +39,6 @@ main (int argc, char **argv) execl (bin, bin, (char *) NULL); perror ("execl failed"); + some_function (); exit (1); } diff --git a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp index 4f35999a256..47a848ea848 100644 --- a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp +++ b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp @@ -96,6 +96,7 @@ if {!$cannot_access} { proc test { always_inserted } { global exec1 + global gdb_prompt clean_restart ${exec1} @@ -106,13 +107,39 @@ proc test { always_inserted } { return -1 } - # On a buggy GDB, with always-inserted on, we'd see: + # Set a second breakpoint (whose original address also ends up + # unmmapped after the exec), for PR 19548. + gdb_test "break some_function" "Breakpoint .*" + + # PR17431: with always-inserted on, we'd see: # (gdb) continue # Continuing. # Warning: # Cannot insert breakpoint 1. # Cannot access memory at address 0x10000ff - gdb_test "continue" "Breakpoint 1, main.*" "continue across exec" + + # PR 19548: with more than one breakpoint, we'd see: + # (gdb) continue + # Continuing. + # process (...) is executing new program: (...)/execl-update-breakpoints2 + # Error in re-setting breakpoint 1: Warning: + # Cannot insert breakpoint 2. + # Cannot access memory at address 0x1000764 + set not_nl "\[^\r\n\]*" + set regex "" + append regex \ + "^continue\r\n" \ + "Continuing\\.\r\n" \ + "${not_nl} is executing new program: ${not_nl}\r\n" \ + "(Reading ${not_nl} from remote target\\.\\.\\.\r\n)*" \ + "\r\n" \ + "Breakpoint 1, main.*$gdb_prompt $" + set message "continue across exec" + gdb_test_multiple "continue" $message { + -re $regex { + pass $message + } + } } foreach always_inserted { "off" "on" } { -- 2.30.2