From patchwork Mon Feb 1 17:04:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 10688 Received: (qmail 6567 invoked by alias); 1 Feb 2016 17:04:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 6550 invoked by uid 89); 1 Feb 2016 17:04:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Format, stale, execs, sk:locati X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 01 Feb 2016 17:04:09 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id A4925C0A5279 for ; Mon, 1 Feb 2016 17:04:07 +0000 (UTC) Received: from brno.lan (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u11H46k8025928 for ; Mon, 1 Feb 2016 12:04:07 -0500 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH] Fix 19548: Breakpoint re-set inserts breakpoints when it shouldn't Date: Mon, 1 Feb 2016 17:04:06 +0000 Message-Id: <1454346246-28564-1-git-send-email-palves@redhat.com> 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-01 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-01 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/breakpoint.c | 25 +++++++---------- gdb/testsuite/gdb.base/execl-update-breakpoints.c | 6 +++++ .../gdb.base/execl-update-breakpoints.exp | 31 ++++++++++++++++++++-- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index afd9065..c059861 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/gdb.base/execl-update-breakpoints.c b/gdb/testsuite/gdb.base/execl-update-breakpoints.c index 3978a70..38f1cea 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 4f35999..e7b276c 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 17227 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" } {