From patchwork Fri Sep 26 00:39:36 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 2979 Received: (qmail 11453 invoked by alias); 26 Sep 2014 00:39:54 -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 11343 invoked by uid 89); 26 Sep 2014 00:39:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 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; Fri, 26 Sep 2014 00:39:50 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8Q0dmmS027652 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 25 Sep 2014 20:39:49 -0400 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8Q0dgFB019425 for ; Thu, 25 Sep 2014 20:39:47 -0400 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 3/9] Put single-step breakpoints on the bp_location chain Date: Fri, 26 Sep 2014 01:39:36 +0100 Message-Id: <1411691982-10744-4-git-send-email-palves@redhat.com> In-Reply-To: <1411691982-10744-1-git-send-email-palves@redhat.com> References: <1411691982-10744-1-git-send-email-palves@redhat.com> This patch makes single-step breakpoints "real" breakpoints on the global location list. There are several benefits to this: - It removes the currently limitation that only 2 single-step breakpoints can be inserted. See an example here of a discussion around a case that wants more than 2, possibly unbounded: https://sourceware.org/ml/gdb-patches/2014-03/msg00663.html - makes software single-step work on read-only code regions. The logic to convert a software breakpoint to a hardware breakpoint if the memory map says the breakpoint address is in read only memory is in insert_bp_location. Because software single-step breakpoints bypass all that go and straight to target_insert_breakpoint, we can't software single-step over read only memory. This patch should remove that limitation, though I haven't tested that (should be testable with a test that forces a code region to read-only with "mem LOW HIGH ro"). - Paves the way to have multiple threads software single-stepping at the same time, leaving update_global_location_list to worry about duplicate locations. - Makes the moribund location machinery aware of software single-step breakpoints, paving the way to enable software single-step on non-stop, instead of forcing displaced stepping for all single steps. - It's generaly cleaner. We no longer have to play games with single-step breakpoints inserted at the same address as regular breakpoints, like we recently had to do for 7.8. See this discussion: https://sourceware.org/ml/gdb-patches/2014-06/msg00052.html. Tested on x86_64 Fedora 20, on top of my 'single-step breakpoints on x86' series. gdb/ 2014-09-25 Pedro Alves * 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) (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) : New enum value. (insert_single_step_breakpoint): Update comment. --- gdb/breakpoint.c | 216 ++++++++++++++++++++----------------------------------- gdb/breakpoint.h | 13 ++-- 2 files changed, 81 insertions(+), 148 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 997514e..0064d88 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -226,11 +226,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); @@ -334,8 +329,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; @@ -1668,21 +1662,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); - } - } } @@ -3790,6 +3769,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 @@ -3887,9 +3873,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; } @@ -4159,6 +4142,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; @@ -5635,6 +5622,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)) @@ -5790,6 +5778,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: @@ -6148,6 +6137,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"}, @@ -6339,6 +6329,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: @@ -7217,6 +7208,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: @@ -9199,10 +9191,31 @@ enable_breakpoints_after_startup (void) breakpoint_re_set (); } +/* Create a new single-step breakpoint for thread THREAD, with no + locations. */ + +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 breakpoint that will evaporate an end of command - at address specified by SAL. - Restrict it to frame FRAME if FRAME is nonzero. */ +/* 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, @@ -13326,28 +13339,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->placed_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 @@ -13356,19 +13350,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->placed_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 +15436,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 +15458,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 +15466,9 @@ 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; - - 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; - } + delete_breakpoint (single_step_breakpoints); } /* Delete software single step breakpoints without removing them from @@ -15522,51 +15479,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; - } + /* 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; } -/* 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]); -} - -/* 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->placed_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 +15510,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 e191c10..12f1158 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 */ @@ -1458,8 +1453,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);