This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH 3/9] Put single-step breakpoints on the bp_location chain
- From: Pedro Alves <palves at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 26 Sep 2014 01:39:36 +0100
- Subject: [PATCH 3/9] Put single-step breakpoints on the bp_location chain
- Authentication-results: sourceware.org; auth=none
- References: <1411691982-10744-1-git-send-email-palves at redhat dot 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 <palves@redhat.com>
* 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) <bp_single_step>: 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);
--
1.9.3