* [PATCH 01/23] add_location_to_breakpoint -> breakpoint::add_location
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 02/23] Make structs breakpoint/base_breakpoint/catchpoint be abstract Pedro Alves
` (22 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Make add_location_to_breakpoint be a method of struct breakpoint.
A patch later in the series will move this to base_breakpoint, but for
now, it needs to be here.
Change-Id: I5bdc2ec1a7c2d66f26f51bf6f6adc8384a90b129
---
gdb/breakpoint.c | 70 +++++++++++++++++++++++-------------------------
gdb/breakpoint.h | 3 +++
2 files changed, 36 insertions(+), 37 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9abc1443d96..385c92a6e7b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -114,9 +114,6 @@ static void mention (const breakpoint *);
static struct breakpoint *set_raw_breakpoint_without_location (struct gdbarch *,
enum bptype);
-static struct bp_location *add_location_to_breakpoint (struct breakpoint *,
- const struct symtab_and_line *);
-
/* This function is used in gdbtk sources and thus can not be made
static. */
static struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
@@ -7308,7 +7305,7 @@ static void
init_raw_breakpoint (struct breakpoint *b, struct symtab_and_line sal,
enum bptype bptype)
{
- add_location_to_breakpoint (b, &sal);
+ b->add_location (sal);
if (bptype != bp_catchpoint)
gdb_assert (sal.pspace != NULL);
@@ -8109,16 +8106,15 @@ handle_automatic_hardware_breakpoints (bp_location *bl)
}
}
-static struct bp_location *
-add_location_to_breakpoint (struct breakpoint *b,
- const struct symtab_and_line *sal)
+bp_location *
+breakpoint::add_location (const symtab_and_line &sal)
{
- struct bp_location *loc, **tmp;
+ struct bp_location *new_loc, **tmp;
CORE_ADDR adjusted_address;
- struct gdbarch *loc_gdbarch = get_sal_arch (*sal);
+ struct gdbarch *loc_gdbarch = get_sal_arch (sal);
if (loc_gdbarch == NULL)
- loc_gdbarch = b->gdbarch;
+ loc_gdbarch = gdbarch;
/* Adjust the breakpoint's address prior to allocating a location.
Once we call allocate_location(), that mostly uninitialized
@@ -8127,31 +8123,31 @@ add_location_to_breakpoint (struct breakpoint *b,
not want its scan of the location chain to find a breakpoint and
location that's only been partially initialized. */
adjusted_address = adjust_breakpoint_address (loc_gdbarch,
- sal->pc, b->type);
+ sal.pc, type);
/* Sort the locations by their ADDRESS. */
- loc = b->allocate_location ();
- for (tmp = &(b->loc); *tmp != NULL && (*tmp)->address <= adjusted_address;
+ new_loc = allocate_location ();
+ for (tmp = &(loc); *tmp != NULL && (*tmp)->address <= adjusted_address;
tmp = &((*tmp)->next))
;
- loc->next = *tmp;
- *tmp = loc;
-
- loc->requested_address = sal->pc;
- loc->address = adjusted_address;
- loc->pspace = sal->pspace;
- loc->probe.prob = sal->prob;
- loc->probe.objfile = sal->objfile;
- gdb_assert (loc->pspace != NULL);
- loc->section = sal->section;
- loc->gdbarch = loc_gdbarch;
- loc->line_number = sal->line;
- loc->symtab = sal->symtab;
- loc->symbol = sal->symbol;
- loc->msymbol = sal->msymbol;
- loc->objfile = sal->objfile;
-
- set_breakpoint_location_function (loc);
+ new_loc->next = *tmp;
+ *tmp = new_loc;
+
+ new_loc->requested_address = sal.pc;
+ new_loc->address = adjusted_address;
+ new_loc->pspace = sal.pspace;
+ new_loc->probe.prob = sal.prob;
+ new_loc->probe.objfile = sal.objfile;
+ gdb_assert (new_loc->pspace != NULL);
+ new_loc->section = sal.section;
+ new_loc->gdbarch = loc_gdbarch;
+ new_loc->line_number = sal.line;
+ new_loc->symtab = sal.symtab;
+ new_loc->symbol = sal.symbol;
+ new_loc->msymbol = sal.msymbol;
+ new_loc->objfile = sal.objfile;
+
+ set_breakpoint_location_function (new_loc);
/* While by definition, permanent breakpoints are already present in the
code, we don't mark the location as inserted. Normally one would expect
@@ -8166,10 +8162,10 @@ add_location_to_breakpoint (struct breakpoint *b,
(If GDB later needs to continue execution past the permanent breakpoint,
it manually increments the PC, thus avoiding executing the breakpoint
instruction.) */
- if (bp_loc_is_permanent (loc))
- loc->permanent = 1;
+ if (bp_loc_is_permanent (new_loc))
+ new_loc->permanent = 1;
- return loc;
+ return new_loc;
}
\f
@@ -8375,7 +8371,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
}
else
{
- loc = add_location_to_breakpoint (b, &sal);
+ loc = b->add_location (sal);
if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
loc->inserted = 1;
}
@@ -12646,7 +12642,7 @@ update_breakpoint_locations (struct breakpoint *b,
switch_to_program_space_and_thread (sal.pspace);
- new_loc = add_location_to_breakpoint (b, &sal);
+ new_loc = b->add_location (sal);
/* Reparse conditions, they might contain references to the
old symtab. */
@@ -13537,7 +13533,7 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
sal.pc = pc;
sal.section = find_pc_overlay (pc);
sal.explicit_pc = 1;
- add_location_to_breakpoint (tp->control.single_step_breakpoints, &sal);
+ tp->control.single_step_breakpoints->add_location (sal);
update_global_location_list (UGLL_INSERT);
}
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index ac738fd7c2d..fb8651c55c3 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -739,6 +739,9 @@ struct breakpoint
/* Nothing to do. */
}
+ /* Add a location for SAL to this breakpoint. */
+ bp_location *add_location (const symtab_and_line &sal);
+
/* Return a range of this breakpoint's locations. */
bp_location_range locations () const;
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 02/23] Make structs breakpoint/base_breakpoint/catchpoint be abstract
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
2022-05-16 18:40 ` [PATCH 01/23] add_location_to_breakpoint -> breakpoint::add_location Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 03/23] ranged_breakpoint: don't use init_raw_breakpoint Pedro Alves
` (21 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
You should never instanciate these types directly.
Change-Id: I8086c74c415eadbd44924bb0ef20f34b5b97ee6f
---
gdb/breakpoint.c | 15 +++++++++++++++
gdb/breakpoint.h | 6 +++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 385c92a6e7b..8f6794511e7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -252,6 +252,21 @@ static const struct breakpoint_ops tracepoint_probe_breakpoint_ops =
create_breakpoints_sal,
};
+/* Implementation of abstract dtors. These must exist to satisfy the
+ linker. */
+
+breakpoint::~breakpoint ()
+{
+}
+
+base_breakpoint::~base_breakpoint ()
+{
+}
+
+catchpoint::~catchpoint ()
+{
+}
+
/* The structure to be used in regular breakpoints. */
struct ordinary_breakpoint : public base_breakpoint
{
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index fb8651c55c3..b7e3b4dc6a1 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -631,7 +631,7 @@ struct breakpoint
DISABLE_COPY_AND_ASSIGN (breakpoint);
- virtual ~breakpoint () = default;
+ virtual ~breakpoint () = 0;
/* Allocate a location for this breakpoint. */
virtual struct bp_location *allocate_location ();
@@ -862,6 +862,8 @@ struct base_breakpoint : public breakpoint
{
using breakpoint::breakpoint;
+ ~base_breakpoint () override = 0;
+
void re_set () override;
int insert_location (struct bp_location *) override;
int remove_location (struct bp_location *,
@@ -1023,6 +1025,8 @@ struct catchpoint : public base_breakpoint
COND_STRING is not NULL, then store it in the breakpoint. */
catchpoint (struct gdbarch *gdbarch, bool temp, const char *cond_string);
+ ~catchpoint () override = 0;
+
void re_set () override
{
/* For catchpoints, the default is to do nothing. */
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 03/23] ranged_breakpoint: don't use init_raw_breakpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
2022-05-16 18:40 ` [PATCH 01/23] add_location_to_breakpoint -> breakpoint::add_location Pedro Alves
2022-05-16 18:40 ` [PATCH 02/23] Make structs breakpoint/base_breakpoint/catchpoint be abstract Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 04/23] ranged_breakpoint, use install_breakpoint Pedro Alves
` (20 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
ranged_breakpoint's ctor already sets the breakpoint's type to
bp_hardware_breakpoint.
Since this is a "regular" breakpoint, b->pspace should remain NULL.
Thus, the only thing init_raw_breakpoint is needed for, is to add the
breakpoint's location. Do that directly.
Change-Id: I1505de94c3919881c2b300437e2c0da9b05f76bd
---
gdb/breakpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8f6794511e7..f9332e1f998 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9439,7 +9439,7 @@ break_range_command (const char *arg, int from_tty)
/* Now set up the breakpoint. */
std::unique_ptr<breakpoint> br (new ranged_breakpoint (get_current_arch ()));
- init_raw_breakpoint (br.get (), sal_start, bp_hardware_breakpoint);
+ br->add_location (sal_start);
b = add_to_breakpoint_chain (std::move (br));
set_breakpoint_count (breakpoint_count + 1);
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 04/23] ranged_breakpoint, use install_breakpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (2 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 03/23] ranged_breakpoint: don't use init_raw_breakpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 05/23] ranged_breakpoint - move initialization to ctor Pedro Alves
` (19 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This commit replaces a chunk of code in break_range_command by an
equivalent call to install_breakpoint.
Change-Id: I31c06cabd36f5be91740aab029265f678aa78e35
---
gdb/breakpoint.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f9332e1f998..ced976ca39d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9356,7 +9356,6 @@ break_range_command (const char *arg, int from_tty)
struct linespec_result canonical_start, canonical_end;
int bp_count, can_use_bp, length;
CORE_ADDR end;
- struct breakpoint *b;
/* We don't support software ranged breakpoints. */
if (target_ranged_break_num_registers () < 0)
@@ -9440,18 +9439,12 @@ break_range_command (const char *arg, int from_tty)
/* Now set up the breakpoint. */
std::unique_ptr<breakpoint> br (new ranged_breakpoint (get_current_arch ()));
br->add_location (sal_start);
- b = add_to_breakpoint_chain (std::move (br));
-
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
- b->disposition = disp_donttouch;
- b->location = std::move (start_location);
- b->location_range_end = std::move (end_location);
- b->loc->length = length;
+ br->disposition = disp_donttouch;
+ br->location = std::move (start_location);
+ br->location_range_end = std::move (end_location);
+ br->loc->length = length;
- mention (b);
- gdb::observers::breakpoint_created.notify (b);
- update_global_location_list (UGLL_MAY_INSERT);
+ install_breakpoint (false, std::move (br), true);
}
/* Return non-zero if EXP is verified as constant. Returned zero
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 05/23] ranged_breakpoint - move initialization to ctor
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (3 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 04/23] ranged_breakpoint, use install_breakpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 06/23] Make a few functions work with base_breakpoint instead of breakpoint Pedro Alves
` (18 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Move initialization of ranged_breakpoint's fields to its ctor.
Change-Id: If7b842861f3cc6a429ea329d45598b5852283ba3
---
gdb/breakpoint.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ced976ca39d..1c1dbfb3ad7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -317,9 +317,20 @@ struct dprintf_breakpoint : public ordinary_breakpoint
/* Ranged breakpoints. */
struct ranged_breakpoint : public ordinary_breakpoint
{
- explicit ranged_breakpoint (struct gdbarch *gdbarch)
+ explicit ranged_breakpoint (struct gdbarch *gdbarch,
+ const symtab_and_line &sal_start,
+ int length,
+ event_location_up start_location,
+ event_location_up end_location)
: ordinary_breakpoint (gdbarch, bp_hardware_breakpoint)
{
+ bp_location *bl = add_location (sal_start);
+ bl->length = length;
+
+ disposition = disp_donttouch;
+
+ location = std::move (start_location);
+ location_range_end = std::move (end_location);
}
int breakpoint_hit (const struct bp_location *bl,
@@ -9436,13 +9447,13 @@ break_range_command (const char *arg, int from_tty)
return;
}
- /* Now set up the breakpoint. */
- std::unique_ptr<breakpoint> br (new ranged_breakpoint (get_current_arch ()));
- br->add_location (sal_start);
- br->disposition = disp_donttouch;
- br->location = std::move (start_location);
- br->location_range_end = std::move (end_location);
- br->loc->length = length;
+ /* Now set up the breakpoint and install it. */
+
+ std::unique_ptr<breakpoint> br
+ (new ranged_breakpoint (get_current_arch (),
+ sal_start, length,
+ std::move (start_location),
+ std::move (end_location)));
install_breakpoint (false, std::move (br), true);
}
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/23] Make a few functions work with base_breakpoint instead of breakpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (4 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 05/23] ranged_breakpoint - move initialization to ctor Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 07/23] More breakpoint_ops parameter elimination Pedro Alves
` (17 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This makes tracepoints inherit from base_breakpoint, since their
locations are code locations. If we do that, then we can eliminate
tracepoint::re_set and tracepoint::decode_location, as they are doing
the same as the base_breakpoint implementations.
With this, all breakpoint types created by new_breakpoint_from_type
are code breakpoints, i.e., base_breakpoint subclasses, and thus we
can make it return a base_breakpoint pointer.
Finally, init_breakpoint_sal can take a base_breakpoint pointer as
"self" pointer too. This will let us convert this function to a
base_breakpoint ctor in a following patch.
Change-Id: I3a4073ff1a4c865f525588095c18dc42b744cb54
---
gdb/breakpoint.c | 30 ++++++------------------------
gdb/breakpoint.h | 9 ++-------
2 files changed, 8 insertions(+), 31 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1c1dbfb3ad7..3df3eb8979d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1258,10 +1258,10 @@ is_tracepoint (const struct breakpoint *b)
/* Factory function to create an appropriate instance of breakpoint given
TYPE. */
-static std::unique_ptr<breakpoint>
+static std::unique_ptr<base_breakpoint>
new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type)
{
- breakpoint *b;
+ base_breakpoint *b;
switch (type)
{
@@ -1317,7 +1317,7 @@ new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type)
gdb_assert_not_reached ("invalid type");
}
- return std::unique_ptr<breakpoint> (b);
+ return std::unique_ptr<base_breakpoint> (b);
}
/* A helper function that validates that COMMANDS are valid for a
@@ -8297,7 +8297,7 @@ update_dprintf_commands (const char *args, int from_tty,
"address location" from the address in the SAL. */
static void
-init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
+init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
gdb::array_view<const symtab_and_line> sals,
event_location_up &&location,
gdb::unique_xmalloc_ptr<char> filter,
@@ -8359,7 +8359,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
if (type == bp_static_tracepoint
|| type == bp_static_marker_tracepoint)
{
- struct tracepoint *t = (struct tracepoint *) b;
+ auto *t = static_cast <struct tracepoint *> (b);
struct static_tracepoint_marker marker;
if (strace_marker_p (b))
@@ -8453,7 +8453,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
int enabled, int internal, unsigned flags,
int display_canonical)
{
- std::unique_ptr<breakpoint> b = new_breakpoint_from_type (gdbarch, type);
+ std::unique_ptr<base_breakpoint> b = new_breakpoint_from_type (gdbarch, type);
init_breakpoint_sal (b.get (), gdbarch,
sals, std::move (location),
@@ -11952,14 +11952,6 @@ bkpt_probe_decode_location (struct breakpoint *b,
return sals;
}
-/* The breakpoint_ops structure to be used in tracepoints. */
-
-void
-tracepoint::re_set ()
-{
- breakpoint_re_set_default (this);
-}
-
int
tracepoint::breakpoint_hit (const struct bp_location *bl,
const address_space *aspace, CORE_ADDR bp_addr,
@@ -12034,16 +12026,6 @@ tracepoint::print_recreate (struct ui_file *fp) const
gdb_printf (fp, " passcount %d\n", pass_count);
}
-std::vector<symtab_and_line>
-tracepoint::decode_location (struct event_location *location,
- struct program_space *search_pspace)
-{
- if (event_location_type (location) == PROBE_LOCATION)
- return bkpt_probe_decode_location (this, location, search_pspace);
-
- return decode_location_default (this, location, search_pspace);
-}
-
/* Virtual table for tracepoints on static probes. */
static void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index b7e3b4dc6a1..48ceceabb3d 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -974,21 +974,16 @@ extern bool is_exception_catchpoint (breakpoint *bp);
/* An instance of this type is used to represent all kinds of
tracepoints. */
-struct tracepoint : public breakpoint
+struct tracepoint : public base_breakpoint
{
- using breakpoint::breakpoint;
+ using base_breakpoint::base_breakpoint;
- void re_set () override;
int breakpoint_hit (const struct bp_location *bl,
const address_space *aspace, CORE_ADDR bp_addr,
const target_waitstatus &ws) override;
void print_one_detail (struct ui_out *uiout) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
- std::vector<symtab_and_line> decode_location
- (struct event_location *location,
- struct program_space *search_pspace) override;
-
/* Number of times this tracepoint should single-step and collect
additional data. */
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 07/23] More breakpoint_ops parameter elimination
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (5 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 06/23] Make a few functions work with base_breakpoint instead of breakpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 08/23] Remove "internal" parameter from a couple functions Pedro Alves
` (16 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Remove breakpoint_ops parameters from a few functions that don't need
it.
Change-Id: Ifcf5e1cc688184acbf5e19b8ea60138ebe63cf28
---
gdb/breakpoint.c | 16 +++++++---------
gdb/breakpoint.h | 3 +--
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3df3eb8979d..c84614eeb02 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -100,7 +100,6 @@ static void create_breakpoints_sal (struct gdbarch *,
enum bptype,
enum bpdisp, int, int,
int,
- const struct breakpoint_ops *,
int, int, int, unsigned);
static std::vector<symtab_and_line> decode_location_default
@@ -8305,7 +8304,7 @@ init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- const struct breakpoint_ops *ops, int from_tty,
+ int from_tty,
int enabled, int internal, unsigned flags,
int display_canonical)
{
@@ -8449,7 +8448,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- const struct breakpoint_ops *ops, int from_tty,
+ int from_tty,
int enabled, int internal, unsigned flags,
int display_canonical)
{
@@ -8462,7 +8461,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
std::move (extra_string),
type, disposition,
thread, task, ignore_count,
- ops, from_tty,
+ from_tty,
enabled, internal, flags,
display_canonical);
@@ -8491,7 +8490,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- const struct breakpoint_ops *ops, int from_tty,
+ int from_tty,
int enabled, int internal, unsigned flags)
{
if (canonical->pre_expanded)
@@ -8513,7 +8512,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
std::move (cond_string),
std::move (extra_string),
type, disposition,
- thread, task, ignore_count, ops,
+ thread, task, ignore_count,
from_tty, enabled, internal, flags,
canonical->special_display);
}
@@ -8997,7 +8996,7 @@ create_breakpoint (struct gdbarch *gdbarch,
std::move (extra_string_copy),
type_wanted,
tempflag ? disp_del : disp_donttouch,
- thread, task, ignore_count, ops,
+ thread, task, ignore_count,
from_tty, enabled, internal, flags);
}
else
@@ -12128,7 +12127,6 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
enum bpdisp disposition,
int thread,
int task, int ignore_count,
- const struct breakpoint_ops *ops,
int from_tty, int enabled,
int internal, unsigned flags)
{
@@ -12153,7 +12151,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
std::move (cond_string),
std::move (extra_string),
type_wanted, disposition,
- thread, task, ignore_count, ops,
+ thread, task, ignore_count,
from_tty, enabled, internal, flags,
canonical->special_display);
/* Given that its possible to have multiple markers with
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 48ceceabb3d..fb2bbd729c0 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -582,8 +582,7 @@ struct breakpoint_ops
gdb::unique_xmalloc_ptr<char>,
gdb::unique_xmalloc_ptr<char>,
enum bptype, enum bpdisp, int, int,
- int, const struct breakpoint_ops *,
- int, int, int, unsigned);
+ int, int, int, int, unsigned);
};
enum watchpoint_triggered
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 08/23] Remove "internal" parameter from a couple functions
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (6 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 07/23] More breakpoint_ops parameter elimination Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 09/23] init_breakpoint_sal -> base_breakpoint::base_breakpoint Pedro Alves
` (15 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
None of init_breakpoint_sal, create_breakpoint_sal, and
strace_marker_create_breakpoints_sal make use of their "internal"
parameter, so remove it.
Change-Id: I943f3bb44717ade7a7b7547edf8f3ff3c37da435
---
gdb/breakpoint.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c84614eeb02..1e21b337802 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8305,7 +8305,7 @@ init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
int from_tty,
- int enabled, int internal, unsigned flags,
+ int enabled, unsigned flags,
int display_canonical)
{
int i;
@@ -8462,7 +8462,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
type, disposition,
thread, task, ignore_count,
from_tty,
- enabled, internal, flags,
+ enabled, flags,
display_canonical);
install_breakpoint (internal, std::move (b), 0);
@@ -12152,7 +12152,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
std::move (extra_string),
type_wanted, disposition,
thread, task, ignore_count,
- from_tty, enabled, internal, flags,
+ from_tty, enabled, flags,
canonical->special_display);
/* Given that its possible to have multiple markers with
the same string id, if the user is creating a static
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 09/23] init_breakpoint_sal -> base_breakpoint::base_breakpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (7 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 08/23] Remove "internal" parameter from a couple functions Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-20 13:40 ` Lancelot SIX
2022-05-16 18:40 ` [PATCH 10/23] Make ada_catchpoint_location's owner ctor parameter be ada_catchpoint Pedro Alves
` (14 subsequent siblings)
23 siblings, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This converts init_breakpoint_sal to a base_breakpoint constructor.
It removes a use of init_raw_breakpoint.
To avoid manually adding a bunch of parameters to
new_breakpoint_from_type, and manually passing them down to the
constructors of a number of different base_breakpoint subclasses, make
new_breakpoint_from_type a variable template function.
Change-Id: I4cc24133ac4c292f547289ec782fc78e5bbe2510
---
gdb/breakpoint.c | 239 +++++++++++++++++++++++------------------------
gdb/breakpoint.h | 16 ++++
2 files changed, 133 insertions(+), 122 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1e21b337802..ce742b97fd5 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1257,8 +1257,10 @@ is_tracepoint (const struct breakpoint *b)
/* Factory function to create an appropriate instance of breakpoint given
TYPE. */
+template<typename... Arg>
static std::unique_ptr<base_breakpoint>
-new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type)
+new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type,
+ Arg&&... args)
{
base_breakpoint *b;
@@ -1266,21 +1268,25 @@ new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type)
{
case bp_breakpoint:
case bp_hardware_breakpoint:
- b = new ordinary_breakpoint (gdbarch, type);
+ b = new ordinary_breakpoint (gdbarch, type,
+ std::forward<Arg> (args)...);
break;
case bp_fast_tracepoint:
case bp_static_tracepoint:
case bp_tracepoint:
- b = new tracepoint (gdbarch, type);
+ b = new tracepoint (gdbarch, type,
+ std::forward<Arg> (args)...);
break;
case bp_static_marker_tracepoint:
- b = new static_marker_tracepoint (gdbarch, type);
+ b = new static_marker_tracepoint (gdbarch, type,
+ std::forward<Arg> (args)...);
break;
case bp_dprintf:
- b = new dprintf_breakpoint (gdbarch, type);
+ b = new dprintf_breakpoint (gdbarch, type,
+ std::forward<Arg> (args)...);
break;
case bp_overlay_event:
@@ -1290,12 +1296,14 @@ new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type)
case bp_thread_event:
case bp_jit_event:
case bp_shlib_event:
- b = new internal_breakpoint (gdbarch, type);
+ b = new internal_breakpoint (gdbarch, type,
+ std::forward<Arg> (args)...);
break;
case bp_longjmp:
case bp_exception:
- b = new longjmp_breakpoint (gdbarch, type);
+ b = new longjmp_breakpoint (gdbarch, type,
+ std::forward<Arg> (args)...);
break;
case bp_watchpoint_scope:
@@ -1309,7 +1317,8 @@ new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type)
case bp_call_dummy:
case bp_until:
case bp_std_terminate:
- b = new momentary_breakpoint (gdbarch, type);
+ b = new momentary_breakpoint (gdbarch, type,
+ std::forward<Arg> (args)...);
break;
default:
@@ -8290,23 +8299,19 @@ update_dprintf_commands (const char *args, int from_tty,
update_dprintf_command_list (b);
}
-/* Create a breakpoint with SAL as location. Use LOCATION
- as a description of the location, and COND_STRING
- as condition expression. If LOCATION is NULL then create an
- "address location" from the address in the SAL. */
-
-static void
-init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
- gdb::array_view<const symtab_and_line> sals,
- event_location_up &&location,
- gdb::unique_xmalloc_ptr<char> filter,
- gdb::unique_xmalloc_ptr<char> cond_string,
- gdb::unique_xmalloc_ptr<char> extra_string,
- enum bptype type, enum bpdisp disposition,
- int thread, int task, int ignore_count,
- int from_tty,
- int enabled, unsigned flags,
- int display_canonical)
+base_breakpoint::base_breakpoint (struct gdbarch *gdbarch_,
+ enum bptype type_,
+ gdb::array_view<const symtab_and_line> sals,
+ event_location_up &&location_,
+ gdb::unique_xmalloc_ptr<char> filter_,
+ gdb::unique_xmalloc_ptr<char> cond_string_,
+ gdb::unique_xmalloc_ptr<char> extra_string_,
+ enum bpdisp disposition_,
+ int thread_, int task_, int ignore_count_,
+ int from_tty,
+ int enabled_, unsigned flags,
+ int display_canonical_)
+ : breakpoint (gdbarch_, type_)
{
int i;
@@ -8326,81 +8331,68 @@ init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
gdb_assert (!sals.empty ());
- for (const auto &sal : sals)
- {
- struct bp_location *loc;
+ thread = thread_;
+ task = task_;
- if (from_tty)
- {
- struct gdbarch *loc_gdbarch = get_sal_arch (sal);
- if (!loc_gdbarch)
- loc_gdbarch = gdbarch;
+ cond_string = std::move (cond_string_);
+ extra_string = std::move (extra_string_);
+ ignore_count = ignore_count_;
+ enable_state = enabled_ ? bp_enabled : bp_disabled;
+ disposition = disposition_;
- describe_other_breakpoints (loc_gdbarch,
- sal.pspace, sal.pc, sal.section, thread);
- }
+ if (type == bp_static_tracepoint
+ || type == bp_static_marker_tracepoint)
+ {
+ auto *t = static_cast <struct tracepoint *> (this);
+ struct static_tracepoint_marker marker;
- if (&sal == &sals[0])
+ if (strace_marker_p (this))
{
- init_raw_breakpoint (b, sal, type);
- b->thread = thread;
- b->task = task;
+ /* We already know the marker exists, otherwise, we
+ wouldn't see a sal for it. */
+ const char *p
+ = &event_location_to_string (location_.get ())[3];
+ const char *endp;
- b->cond_string = std::move (cond_string);
- b->extra_string = std::move (extra_string);
- b->ignore_count = ignore_count;
- b->enable_state = enabled ? bp_enabled : bp_disabled;
- b->disposition = disposition;
+ p = skip_spaces (p);
- if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
- b->loc->inserted = 1;
+ endp = skip_to_space (p);
- if (type == bp_static_tracepoint
- || type == bp_static_marker_tracepoint)
- {
- auto *t = static_cast <struct tracepoint *> (b);
- struct static_tracepoint_marker marker;
-
- if (strace_marker_p (b))
- {
- /* We already know the marker exists, otherwise, we
- wouldn't see a sal for it. */
- const char *p
- = &event_location_to_string (b->location.get ())[3];
- const char *endp;
-
- p = skip_spaces (p);
-
- endp = skip_to_space (p);
-
- t->static_trace_marker_id.assign (p, endp - p);
-
- gdb_printf (_("Probed static tracepoint "
- "marker \"%s\"\n"),
- t->static_trace_marker_id.c_str ());
- }
- else if (target_static_tracepoint_marker_at (sal.pc, &marker))
- {
- t->static_trace_marker_id = std::move (marker.str_id);
+ t->static_trace_marker_id.assign (p, endp - p);
- gdb_printf (_("Probed static tracepoint "
- "marker \"%s\"\n"),
- t->static_trace_marker_id.c_str ());
- }
- else
- warning (_("Couldn't determine the static "
- "tracepoint marker to probe"));
- }
+ gdb_printf (_("Probed static tracepoint "
+ "marker \"%s\"\n"),
+ t->static_trace_marker_id.c_str ());
+ }
+ else if (target_static_tracepoint_marker_at (sals[0].pc, &marker))
+ {
+ t->static_trace_marker_id = std::move (marker.str_id);
- loc = b->loc;
+ gdb_printf (_("Probed static tracepoint "
+ "marker \"%s\"\n"),
+ t->static_trace_marker_id.c_str ());
}
else
+ warning (_("Couldn't determine the static "
+ "tracepoint marker to probe"));
+ }
+
+ for (const auto &sal : sals)
+ {
+ if (from_tty)
{
- loc = b->add_location (sal);
- if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
- loc->inserted = 1;
+ struct gdbarch *loc_gdbarch = get_sal_arch (sal);
+ if (!loc_gdbarch)
+ loc_gdbarch = gdbarch;
+
+ describe_other_breakpoints (loc_gdbarch,
+ sal.pspace, sal.pc, sal.section, thread);
}
+ bp_location *new_loc = add_location (sal);
+ if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
+ new_loc->inserted = 1;
+
/* Do not set breakpoint locations conditions yet. As locations
are inserted, they get sorted based on their addresses. Let
the list stabilize to have reliable location numbers. */
@@ -8409,34 +8401,33 @@ init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
command line, otherwise it's an error. */
if (type == bp_dprintf)
{
- if (b->extra_string)
- update_dprintf_command_list (b);
+ if (extra_string)
+ update_dprintf_command_list (this);
else
error (_("Format string required"));
}
- else if (b->extra_string)
- error (_("Garbage '%s' at end of command"), b->extra_string.get ());
+ else if (extra_string)
+ error (_("Garbage '%s' at end of command"), extra_string.get ());
}
-
/* The order of the locations is now stable. Set the location
condition using the location's number. */
int loc_num = 1;
- for (bp_location *loc : b->locations ())
+ for (bp_location *bl : locations ())
{
- if (b->cond_string != nullptr)
- set_breakpoint_location_condition (b->cond_string.get (), loc,
- b->number, loc_num);
+ if (cond_string != nullptr)
+ set_breakpoint_location_condition (cond_string.get (), bl,
+ number, loc_num);
++loc_num;
}
- b->display_canonical = display_canonical;
- if (location != NULL)
- b->location = std::move (location);
+ display_canonical = display_canonical_;
+ if (location_ != NULL)
+ location = std::move (location_);
else
- b->location = new_address_location (b->loc->address, NULL, 0);
- b->filter = std::move (filter);
+ location = new_address_location (this->loc->address, NULL, 0);
+ filter = std::move (filter_);
}
static void
@@ -8452,18 +8443,19 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
int enabled, int internal, unsigned flags,
int display_canonical)
{
- std::unique_ptr<base_breakpoint> b = new_breakpoint_from_type (gdbarch, type);
-
- init_breakpoint_sal (b.get (), gdbarch,
- sals, std::move (location),
- std::move (filter),
- std::move (cond_string),
- std::move (extra_string),
- type, disposition,
- thread, task, ignore_count,
- from_tty,
- enabled, flags,
- display_canonical);
+ std::unique_ptr<base_breakpoint> b
+ = new_breakpoint_from_type (gdbarch,
+ type,
+ sals,
+ std::move (location),
+ std::move (filter),
+ std::move (cond_string),
+ std::move (extra_string),
+ disposition,
+ thread, task, ignore_count,
+ from_tty,
+ enabled, flags,
+ display_canonical);
install_breakpoint (internal, std::move (b), 0);
}
@@ -12144,16 +12136,19 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
event_location_up location
= copy_event_location (canonical->location.get ());
- std::unique_ptr<tracepoint> tp (new tracepoint (gdbarch,
- type_wanted));
- init_breakpoint_sal (tp.get (), gdbarch, lsal.sals[i],
- std::move (location), NULL,
- std::move (cond_string),
- std::move (extra_string),
- type_wanted, disposition,
- thread, task, ignore_count,
- from_tty, enabled, flags,
- canonical->special_display);
+ std::unique_ptr<tracepoint> tp
+ (new tracepoint (gdbarch,
+ type_wanted,
+ lsal.sals[i],
+ std::move (location),
+ NULL,
+ std::move (cond_string),
+ std::move (extra_string),
+ disposition,
+ thread, task, ignore_count,
+ from_tty, enabled, flags,
+ canonical->special_display));
+
/* Given that its possible to have multiple markers with
the same string id, if the user is creating a static
tracepoint by marker id ("strace -m MARKER_ID"), then
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index fb2bbd729c0..40ba98b7496 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -861,6 +861,22 @@ struct base_breakpoint : public breakpoint
{
using breakpoint::breakpoint;
+ /* Create a breakpoint with SALS as locations. Use LOCATION as a
+ description of the location, and COND_STRING as condition
+ expression. If LOCATION is NULL then create an "address
+ location" from the address in the SAL. */
+ base_breakpoint (struct gdbarch *gdbarch, bptype type,
+ gdb::array_view<const symtab_and_line> sals,
+ event_location_up &&location,
+ gdb::unique_xmalloc_ptr<char> filter,
+ gdb::unique_xmalloc_ptr<char> cond_string,
+ gdb::unique_xmalloc_ptr<char> extra_string,
+ enum bpdisp disposition,
+ int thread, int task, int ignore_count,
+ int from_tty,
+ int enabled, unsigned flags,
+ int display_canonical);
+
~base_breakpoint () override = 0;
void re_set () override;
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 09/23] init_breakpoint_sal -> base_breakpoint::base_breakpoint
2022-05-16 18:40 ` [PATCH 09/23] init_breakpoint_sal -> base_breakpoint::base_breakpoint Pedro Alves
@ 2022-05-20 13:40 ` Lancelot SIX
2022-05-20 19:20 ` Pedro Alves
0 siblings, 1 reply; 31+ messages in thread
From: Lancelot SIX @ 2022-05-20 13:40 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Hi,
Some styling remarks below.
On Mon, May 16, 2022 at 07:40:16PM +0100, Pedro Alves wrote:
> This converts init_breakpoint_sal to a base_breakpoint constructor.
>
> It removes a use of init_raw_breakpoint.
>
> To avoid manually adding a bunch of parameters to
> new_breakpoint_from_type, and manually passing them down to the
> constructors of a number of different base_breakpoint subclasses, make
> new_breakpoint_from_type a variable template function.
>
> Change-Id: I4cc24133ac4c292f547289ec782fc78e5bbe2510
> ---
> gdb/breakpoint.c | 239 +++++++++++++++++++++++------------------------
> gdb/breakpoint.h | 16 ++++
> 2 files changed, 133 insertions(+), 122 deletions(-)
> @@ -8326,81 +8331,68 @@ init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
>
> gdb_assert (!sals.empty ());
>
> - for (const auto &sal : sals)
> - {
> - struct bp_location *loc;
> + thread = thread_;
> + task = task_;
>
> - if (from_tty)
> - {
> - struct gdbarch *loc_gdbarch = get_sal_arch (sal);
> - if (!loc_gdbarch)
> - loc_gdbarch = gdbarch;
> + cond_string = std::move (cond_string_);
> + extra_string = std::move (extra_string_);
> + ignore_count = ignore_count_;
> + enable_state = enabled_ ? bp_enabled : bp_disabled;
> + disposition = disposition_;
>
> - describe_other_breakpoints (loc_gdbarch,
> - sal.pspace, sal.pc, sal.section, thread);
> - }
> + if (type == bp_static_tracepoint
> + || type == bp_static_marker_tracepoint)
> + {
> + auto *t = static_cast <struct tracepoint *> (this);
I think the space before "<" should be removed (I think I saw this in
one of the previous patches too).
> + struct static_tracepoint_marker marker;
>
> - if (&sal == &sals[0])
> + if (strace_marker_p (this))
> {
> - init_raw_breakpoint (b, sal, type);
> - b->thread = thread;
> - b->task = task;
> + /* We already know the marker exists, otherwise, we
> + wouldn't see a sal for it. */
> + const char *p
> + = &event_location_to_string (location_.get ())[3];
Looks like that the statement can fit on one line now.
> + const char *endp;
>
> - b->cond_string = std::move (cond_string);
> - b->extra_string = std::move (extra_string);
> - b->ignore_count = ignore_count;
> - b->enable_state = enabled ? bp_enabled : bp_disabled;
> - b->disposition = disposition;
> + p = skip_spaces (p);
>
> - if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
> - b->loc->inserted = 1;
> + endp = skip_to_space (p);
>
> - if (type == bp_static_tracepoint
> - || type == bp_static_marker_tracepoint)
> - {
> - auto *t = static_cast <struct tracepoint *> (b);
> - struct static_tracepoint_marker marker;
> -
> - if (strace_marker_p (b))
> - {
> - /* We already know the marker exists, otherwise, we
> - wouldn't see a sal for it. */
> - const char *p
> - = &event_location_to_string (b->location.get ())[3];
> - const char *endp;
> -
> - p = skip_spaces (p);
> -
> - endp = skip_to_space (p);
> -
> - t->static_trace_marker_id.assign (p, endp - p);
> -
> - gdb_printf (_("Probed static tracepoint "
> - "marker \"%s\"\n"),
> - t->static_trace_marker_id.c_str ());
> - }
> - else if (target_static_tracepoint_marker_at (sal.pc, &marker))
> - {
> - t->static_trace_marker_id = std::move (marker.str_id);
> + t->static_trace_marker_id.assign (p, endp - p);
>
> - gdb_printf (_("Probed static tracepoint "
> - "marker \"%s\"\n"),
> - t->static_trace_marker_id.c_str ());
> - }
> - else
> - warning (_("Couldn't determine the static "
> - "tracepoint marker to probe"));
> - }
> + gdb_printf (_("Probed static tracepoint "
> + "marker \"%s\"\n"),
This literal string can also fit into one line, no need to split it.
> + t->static_trace_marker_id.c_str ());
> + }
> + else if (target_static_tracepoint_marker_at (sals[0].pc, &marker))
> + {
> + t->static_trace_marker_id = std::move (marker.str_id);
>
> - loc = b->loc;
> + gdb_printf (_("Probed static tracepoint "
> + "marker \"%s\"\n"),
Same here.
> + t->static_trace_marker_id.c_str ());
> }
> else
> + warning (_("Couldn't determine the static "
> + "tracepoint marker to probe"));
And probably here too.
> + }
> +
> + for (const auto &sal : sals)
> + {
> + if (from_tty)
> {
> - loc = b->add_location (sal);
> - if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
> - loc->inserted = 1;
> + struct gdbarch *loc_gdbarch = get_sal_arch (sal);
> + if (!loc_gdbarch)
Should be "loc_gdbarch == nullptr"
> + loc_gdbarch = gdbarch;
> +
> + describe_other_breakpoints (loc_gdbarch,
> + sal.pspace, sal.pc, sal.section, thread);
> }
>
> + bp_location *new_loc = add_location (sal);
> + if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
> + new_loc->inserted = 1;
> +
> /* Do not set breakpoint locations conditions yet. As locations
> are inserted, they get sorted based on their addresses. Let
> the list stabilize to have reliable location numbers. */
Also, down the line, there are some leftover NULL which could become
nullptr. This is from code which just god re-indented, but while at
updating it, it could be nice to fix this too.
Best,
Lancelot.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 09/23] init_breakpoint_sal -> base_breakpoint::base_breakpoint
2022-05-20 13:40 ` Lancelot SIX
@ 2022-05-20 19:20 ` Pedro Alves
0 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-20 19:20 UTC (permalink / raw)
To: Lancelot SIX; +Cc: gdb-patches
On 2022-05-20 14:40, Lancelot SIX wrote:
> Hi,
>
> Some styling remarks below.
>
> On Mon, May 16, 2022 at 07:40:16PM +0100, Pedro Alves wrote:
>> This converts init_breakpoint_sal to a base_breakpoint constructor.
>>
>> It removes a use of init_raw_breakpoint.
>>
>> To avoid manually adding a bunch of parameters to
>> new_breakpoint_from_type, and manually passing them down to the
>> constructors of a number of different base_breakpoint subclasses, make
>> new_breakpoint_from_type a variable template function.
>>
>> Change-Id: I4cc24133ac4c292f547289ec782fc78e5bbe2510
>> ---
>> gdb/breakpoint.c | 239 +++++++++++++++++++++++------------------------
>> gdb/breakpoint.h | 16 ++++
>> 2 files changed, 133 insertions(+), 122 deletions(-)
>> @@ -8326,81 +8331,68 @@ init_breakpoint_sal (base_breakpoint *b, struct gdbarch *gdbarch,
>>
>> gdb_assert (!sals.empty ());
>>
>> - for (const auto &sal : sals)
>> - {
>> - struct bp_location *loc;
>> + thread = thread_;
>> + task = task_;
>>
>> - if (from_tty)
>> - {
>> - struct gdbarch *loc_gdbarch = get_sal_arch (sal);
>> - if (!loc_gdbarch)
>> - loc_gdbarch = gdbarch;
>> + cond_string = std::move (cond_string_);
>> + extra_string = std::move (extra_string_);
>> + ignore_count = ignore_count_;
>> + enable_state = enabled_ ? bp_enabled : bp_disabled;
>> + disposition = disposition_;
>>
>> - describe_other_breakpoints (loc_gdbarch,
>> - sal.pspace, sal.pc, sal.section, thread);
>> - }
>> + if (type == bp_static_tracepoint
>> + || type == bp_static_marker_tracepoint)
>> + {
>> + auto *t = static_cast <struct tracepoint *> (this);
>
> I think the space before "<" should be removed (I think I saw this in
> one of the previous patches too).
Thanks, I fixed it in the patch that introduced it (patch #6).
I then did:
$ git diff upstream/master... | grep static_cast
+ auto *t = static_cast<struct tracepoint *> (this);
+ = static_cast<momentary_breakpoint *> (tp->control.single_step_breakpoints);
with the whole series applied, so it looks like this was the only case.
>
>> + struct static_tracepoint_marker marker;
>>
>> - if (&sal == &sals[0])
>> + if (strace_marker_p (this))
>> {
>> - init_raw_breakpoint (b, sal, type);
>> - b->thread = thread;
>> - b->task = task;
>> + /* We already know the marker exists, otherwise, we
>> + wouldn't see a sal for it. */
>> + const char *p
>> + = &event_location_to_string (location_.get ())[3];
>
> Looks like that the statement can fit on one line now.
Done.
>
>> + const char *endp;
>>
>> - b->cond_string = std::move (cond_string);
>> - b->extra_string = std::move (extra_string);
>> - b->ignore_count = ignore_count;
>> - b->enable_state = enabled ? bp_enabled : bp_disabled;
>> - b->disposition = disposition;
>> + p = skip_spaces (p);
>>
>> - if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
>> - b->loc->inserted = 1;
>> + endp = skip_to_space (p);
>>
>> - if (type == bp_static_tracepoint
>> - || type == bp_static_marker_tracepoint)
>> - {
>> - auto *t = static_cast <struct tracepoint *> (b);
>> - struct static_tracepoint_marker marker;
>> -
>> - if (strace_marker_p (b))
>> - {
>> - /* We already know the marker exists, otherwise, we
>> - wouldn't see a sal for it. */
>> - const char *p
>> - = &event_location_to_string (b->location.get ())[3];
>> - const char *endp;
>> -
>> - p = skip_spaces (p);
>> -
>> - endp = skip_to_space (p);
>> -
>> - t->static_trace_marker_id.assign (p, endp - p);
>> -
>> - gdb_printf (_("Probed static tracepoint "
>> - "marker \"%s\"\n"),
>> - t->static_trace_marker_id.c_str ());
>> - }
>> - else if (target_static_tracepoint_marker_at (sal.pc, &marker))
>> - {
>> - t->static_trace_marker_id = std::move (marker.str_id);
>> + t->static_trace_marker_id.assign (p, endp - p);
>>
>> - gdb_printf (_("Probed static tracepoint "
>> - "marker \"%s\"\n"),
>> - t->static_trace_marker_id.c_str ());
>> - }
>> - else
>> - warning (_("Couldn't determine the static "
>> - "tracepoint marker to probe"));
>> - }
>> + gdb_printf (_("Probed static tracepoint "
>> + "marker \"%s\"\n"),
>
> This literal string can also fit into one line, no need to split it.
Done.
>
>> + t->static_trace_marker_id.c_str ());
>> + }
>> + else if (target_static_tracepoint_marker_at (sals[0].pc, &marker))
>> + {
>> + t->static_trace_marker_id = std::move (marker.str_id);
>>
>> - loc = b->loc;
>> + gdb_printf (_("Probed static tracepoint "
>> + "marker \"%s\"\n"),
>
> Same here.
Done.
>
>> + t->static_trace_marker_id.c_str ());
>> }
>> else
>> + warning (_("Couldn't determine the static "
>> + "tracepoint marker to probe"));
>
> And probably here too.
Done.
>
>> + }
>> +
>> + for (const auto &sal : sals)
>> + {
>> + if (from_tty)
>> {
>> - loc = b->add_location (sal);
>> - if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
>> - loc->inserted = 1;
>> + struct gdbarch *loc_gdbarch = get_sal_arch (sal);
>> + if (!loc_gdbarch)
>
> Should be "loc_gdbarch == nullptr"
>
>> + loc_gdbarch = gdbarch;
Done.
>> +
>> + describe_other_breakpoints (loc_gdbarch,
>> + sal.pspace, sal.pc, sal.section, thread);
>> }
>>
>> + bp_location *new_loc = add_location (sal);
>> + if ((flags & CREATE_BREAKPOINT_FLAGS_INSERTED) != 0)
>> + new_loc->inserted = 1;
>> +
>> /* Do not set breakpoint locations conditions yet. As locations
>> are inserted, they get sorted based on their addresses. Let
>> the list stabilize to have reliable location numbers. */
>
> Also, down the line, there are some leftover NULL which could become
> nullptr. This is from code which just god re-indented, but while at
> updating it, it could be nice to fix this too.
All done locally.
Thanks!
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 10/23] Make ada_catchpoint_location's owner ctor parameter be ada_catchpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (8 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 09/23] init_breakpoint_sal -> base_breakpoint::base_breakpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 11/23] Convert init_ada_exception_catchpoint to a ctor Pedro Alves
` (13 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This commit changes ada_catchpoint_location's ctor from:
ada_catchpoint_location (breakpoint *owner)
to:
ada_catchpoint_location (ada_catchpoint *owner)
just to make the code better document intention.
To do this, we need to move the ada_catchpoint_location type's
definition to after ada_catchpoint is defined, otherwise the compiler
doesn't know that ada_catchpoint is convertible to struct breakpoint.
Change-Id: Id908b2e38bde30b262381e00c5637adb9bf0129d
---
gdb/ada-lang.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 248b847cfbc..1c70f4178d0 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12102,22 +12102,6 @@ static std::string ada_exception_catchpoint_cond_string
exception, in order to be able to re-set the condition expression
when symbols change. */
-/* An instance of this type is used to represent an Ada catchpoint
- breakpoint location. */
-
-class ada_catchpoint_location : public bp_location
-{
-public:
- ada_catchpoint_location (breakpoint *owner)
- : bp_location (owner, bp_loc_software_breakpoint)
- {}
-
- /* The condition that checks whether the exception that was raised
- is the specific exception the user specified on catchpoint
- creation. */
- expression_up excep_cond_expr;
-};
-
/* An instance of this type is used to represent an Ada catchpoint. */
struct ada_catchpoint : public base_breakpoint
@@ -12144,6 +12128,22 @@ struct ada_catchpoint : public base_breakpoint
enum ada_exception_catchpoint_kind m_kind;
};
+/* An instance of this type is used to represent an Ada catchpoint
+ breakpoint location. */
+
+class ada_catchpoint_location : public bp_location
+{
+public:
+ explicit ada_catchpoint_location (ada_catchpoint *owner)
+ : bp_location (owner, bp_loc_software_breakpoint)
+ {}
+
+ /* The condition that checks whether the exception that was raised
+ is the specific exception the user specified on catchpoint
+ creation. */
+ expression_up excep_cond_expr;
+};
+
/* Parse the exception condition string in the context of each of the
catchpoint's locations, and store them for later evaluation. */
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 11/23] Convert init_ada_exception_catchpoint to a ctor
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (9 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 10/23] Make ada_catchpoint_location's owner ctor parameter be ada_catchpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 12/23] Refactor set_internal_breakpoint / internal_breakpoint ctor Pedro Alves
` (12 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Currently, init_ada_exception_catchpoint is defined in breakpoint.c, I
presume so it can call the static describe_other_breakpoints function.
I think this is a dependency inversion.
init_ada_exception_catchpoint, being code specific to Ada catchpoints,
should be in ada-lang.c, and describe_other_breakpoints, a core
function, should be exported.
And then, we can convert init_ada_exception_catchpoint to an
ada_catchpoint ctor.
Change-Id: I07695572dabc5a75d3d3740fd9b95db1529406a1
---
gdb/ada-lang.c | 43 +++++++++++++++++++++++++++++++++++++++----
gdb/breakpoint.c | 46 ++--------------------------------------------
gdb/breakpoint.h | 19 ++++++++-----------
3 files changed, 49 insertions(+), 59 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 1c70f4178d0..5ddca104dc3 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12107,10 +12107,45 @@ static std::string ada_exception_catchpoint_cond_string
struct ada_catchpoint : public base_breakpoint
{
ada_catchpoint (struct gdbarch *gdbarch_,
- enum ada_exception_catchpoint_kind kind)
+ enum ada_exception_catchpoint_kind kind,
+ struct symtab_and_line sal,
+ const char *addr_string_,
+ bool tempflag,
+ bool enabled,
+ bool from_tty)
: base_breakpoint (gdbarch_, bp_catchpoint),
m_kind (kind)
{
+ add_location (sal);
+
+ /* Unlike most base_breakpoint types, Ada catchpoints are
+ pspace-specific. */
+ gdb_assert (sal.pspace != nullptr);
+ this->pspace = sal.pspace;
+
+ if (from_tty)
+ {
+ struct gdbarch *loc_gdbarch = get_sal_arch (sal);
+ if (!loc_gdbarch)
+ loc_gdbarch = gdbarch;
+
+ describe_other_breakpoints (loc_gdbarch,
+ sal.pspace, sal.pc, sal.section, -1);
+ /* FIXME: brobecker/2006-12-28: Actually, re-implement a special
+ version for exception catchpoints, because two catchpoints
+ used for different exception names will use the same address.
+ In this case, a "breakpoint ... also set at..." warning is
+ unproductive. Besides, the warning phrasing is also a bit
+ inappropriate, we should use the word catchpoint, and tell
+ the user what type of catchpoint it is. The above is good
+ enough for now, though. */
+ }
+
+ enable_state = enabled ? bp_enabled : bp_disabled;
+ disposition = tempflag ? disp_del : disp_donttouch;
+ location = string_to_event_location (&addr_string_,
+ language_def (language_ada));
+ language = language_ada;
}
struct bp_location *allocate_location () override;
@@ -12759,9 +12794,9 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
std::string addr_string;
struct symtab_and_line sal = ada_exception_sal (ex_kind, &addr_string);
- std::unique_ptr<ada_catchpoint> c (new ada_catchpoint (gdbarch, ex_kind));
- init_ada_exception_breakpoint (c.get (), gdbarch, sal, addr_string.c_str (),
- tempflag, disabled, from_tty);
+ std::unique_ptr<ada_catchpoint> c
+ (new ada_catchpoint (gdbarch, ex_kind, sal, addr_string.c_str (),
+ tempflag, disabled, from_tty));
c->excep_string = excep_string;
create_excep_cond_exprs (c.get (), ex_kind);
if (!cond_string.empty ())
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ce742b97fd5..3773f317bfe 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -130,10 +130,6 @@ static CORE_ADDR adjust_breakpoint_address (struct gdbarch *gdbarch,
CORE_ADDR bpaddr,
enum bptype bptype);
-static void describe_other_breakpoints (struct gdbarch *,
- struct program_space *, CORE_ADDR,
- struct obj_section *, int);
-
static int watchpoint_locations_match (struct bp_location *loc1,
struct bp_location *loc2);
@@ -6862,11 +6858,9 @@ breakpoint_has_pc (struct breakpoint *b,
return 0;
}
-/* Print a message describing any user-breakpoints set at PC. This
- concerns with logical breakpoints, so we match program spaces, not
- address spaces. */
+/* See breakpoint.h. */
-static void
+void
describe_other_breakpoints (struct gdbarch *gdbarch,
struct program_space *pspace, CORE_ADDR pc,
struct obj_section *section, int thread)
@@ -10594,42 +10588,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
proceed (-1, GDB_SIGNAL_DEFAULT);
}
-void
-init_ada_exception_breakpoint (struct breakpoint *b,
- struct gdbarch *gdbarch,
- struct symtab_and_line sal,
- const char *addr_string,
- int tempflag,
- int enabled,
- int from_tty)
-{
- if (from_tty)
- {
- struct gdbarch *loc_gdbarch = get_sal_arch (sal);
- if (!loc_gdbarch)
- loc_gdbarch = gdbarch;
-
- describe_other_breakpoints (loc_gdbarch,
- sal.pspace, sal.pc, sal.section, -1);
- /* FIXME: brobecker/2006-12-28: Actually, re-implement a special
- version for exception catchpoints, because two catchpoints
- used for different exception names will use the same address.
- In this case, a "breakpoint ... also set at..." warning is
- unproductive. Besides, the warning phrasing is also a bit
- inappropriate, we should use the word catchpoint, and tell
- the user what type of catchpoint it is. The above is good
- enough for now, though. */
- }
-
- init_raw_breakpoint (b, sal, bp_catchpoint);
-
- b->enable_state = enabled ? bp_enabled : bp_disabled;
- b->disposition = tempflag ? disp_del : disp_donttouch;
- b->location = string_to_event_location (&addr_string,
- language_def (language_ada));
- b->language = language_ada;
-}
-
\f
/* Compare two breakpoints and return a strcmp-like result. */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 40ba98b7496..a4ead8b4d4e 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1460,17 +1460,6 @@ extern void
void *user_data_catch,
void *user_data_tcatch);
-/* Initialize a breakpoint struct for Ada exception catchpoints. */
-
-extern void
- init_ada_exception_breakpoint (struct breakpoint *b,
- struct gdbarch *gdbarch,
- struct symtab_and_line sal,
- const char *addr_string,
- int tempflag,
- int enabled,
- int from_tty);
-
/* Add breakpoint B on the breakpoint list, and notify the user, the
target and breakpoint_created observers of its existence. If
INTERNAL is non-zero, the breakpoint number will be allocated from
@@ -1905,4 +1894,12 @@ extern void catch_exception_event (enum exception_event_kind ex_event,
extern void print_solib_event (bool is_catchpoint);
+/* Print a message describing any user-breakpoints set at PC. This
+ concerns with logical breakpoints, so we match program spaces, not
+ address spaces. */
+
+extern void describe_other_breakpoints (struct gdbarch *,
+ struct program_space *, CORE_ADDR,
+ struct obj_section *, int);
+
#endif /* !defined (BREAKPOINT_H) */
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 12/23] Refactor set_internal_breakpoint / internal_breakpoint ctor
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (10 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 11/23] Convert init_ada_exception_catchpoint to a ctor Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 13/23] Refactor momentary breakpoints, eliminate set_raw_breakpoint{, _without_location} Pedro Alves
` (11 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This moves initialization of internal_breakpoint's breakpoint fields
to internal_breakpoint's ctor, and stops using
new_breakpoint_from_type for internal_breakpoint breakpoints.
Change-Id: I898ed0565f47cb00e4429f1c6446e6f9a385a78d
---
gdb/breakpoint.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3773f317bfe..1659d2b9737 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -119,6 +119,8 @@ static struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
struct symtab_and_line,
enum bptype);
+static breakpoint *add_to_breakpoint_chain (std::unique_ptr<breakpoint> &&b);
+
static struct breakpoint *
momentary_breakpoint_from_master (struct breakpoint *orig,
enum bptype type,
@@ -276,7 +278,19 @@ struct ordinary_breakpoint : public base_breakpoint
/* Internal breakpoints. */
struct internal_breakpoint : public base_breakpoint
{
- using base_breakpoint::base_breakpoint;
+ internal_breakpoint (struct gdbarch *gdbarch,
+ enum bptype type, CORE_ADDR address)
+ : base_breakpoint (gdbarch, type)
+ {
+ symtab_and_line sal;
+ sal.pc = address;
+ sal.section = find_pc_overlay (sal.pc);
+ sal.pspace = current_program_space;
+ add_location (sal);
+
+ pspace = current_program_space;
+ disposition = disp_donttouch;
+ }
void re_set () override;
void check_status (struct bpstat *bs) override;
@@ -1285,17 +1299,6 @@ new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type,
std::forward<Arg> (args)...);
break;
- case bp_overlay_event:
- case bp_longjmp_master:
- case bp_std_terminate_master:
- case bp_exception_master:
- case bp_thread_event:
- case bp_jit_event:
- case bp_shlib_event:
- b = new internal_breakpoint (gdbarch, type,
- std::forward<Arg> (args)...);
- break;
-
case bp_longjmp:
case bp_exception:
b = new longjmp_breakpoint (gdbarch, type,
@@ -3294,16 +3297,12 @@ static struct breakpoint *
create_internal_breakpoint (struct gdbarch *gdbarch,
CORE_ADDR address, enum bptype type)
{
- symtab_and_line sal;
- sal.pc = address;
- sal.section = find_pc_overlay (sal.pc);
- sal.pspace = current_program_space;
+ std::unique_ptr<internal_breakpoint> b
+ (new internal_breakpoint (gdbarch, type, address));
- breakpoint *b = set_raw_breakpoint (gdbarch, sal, type);
b->number = internal_breakpoint_number--;
- b->disposition = disp_donttouch;
- return b;
+ return add_to_breakpoint_chain (std::move (b));
}
static const char *const longjmp_names[] =
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 13/23] Refactor momentary breakpoints, eliminate set_raw_breakpoint{, _without_location}
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (11 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 12/23] Refactor set_internal_breakpoint / internal_breakpoint ctor Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 14/23] Make exception_catchpoint inherit base_breakpoint instead of catchpoint Pedro Alves
` (10 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This commit makes set_momentary_breakpoint allocate the breakpoint
type without relying on set_raw_breakpoint, and similarly,
momentary_breakpoint_from_master not rely on
set_raw_breakpoint_without_location. This will let us convert
init_raw_breakpoint to a ctor in a following patch.
The comment about set_raw_breakpoint being used in gdbtk sources is
stale. gdbtk no longer uses it.
Change-Id: Ibbf77731e4b22e18ccebc1b5799bbec0aff28c8a
---
gdb/breakpoint.c | 97 ++++++++++++------------------------------------
1 file changed, 24 insertions(+), 73 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1659d2b9737..6e24137ec4b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -111,13 +111,7 @@ static int can_use_hardware_watchpoint
static void mention (const breakpoint *);
-static struct breakpoint *set_raw_breakpoint_without_location (struct gdbarch *,
- enum bptype);
-/* This function is used in gdbtk sources and thus can not be made
- static. */
-static struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
- struct symtab_and_line,
- enum bptype);
+static breakpoint *add_to_breakpoint_chain (std::unique_ptr<breakpoint> &&b);
static breakpoint *add_to_breakpoint_chain (std::unique_ptr<breakpoint> &&b);
@@ -1299,27 +1293,6 @@ new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type,
std::forward<Arg> (args)...);
break;
- case bp_longjmp:
- case bp_exception:
- b = new longjmp_breakpoint (gdbarch, type,
- std::forward<Arg> (args)...);
- break;
-
- case bp_watchpoint_scope:
- case bp_finish:
- case bp_gnu_ifunc_resolver_return:
- case bp_step_resume:
- case bp_hp_step_resume:
- case bp_longjmp_resume:
- case bp_longjmp_call_dummy:
- case bp_exception_resume:
- case bp_call_dummy:
- case bp_until:
- case bp_std_terminate:
- b = new momentary_breakpoint (gdbarch, type,
- std::forward<Arg> (args)...);
- break;
-
default:
gdb_assert_not_reached ("invalid type");
}
@@ -7256,18 +7229,6 @@ add_to_breakpoint_chain (std::unique_ptr<breakpoint> &&b)
return result;
}
-/* Helper to set_raw_breakpoint below. Creates a breakpoint
- that has type BPTYPE and has no locations as yet. */
-
-static struct breakpoint *
-set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
- enum bptype bptype)
-{
- std::unique_ptr<breakpoint> b = new_breakpoint_from_type (gdbarch, bptype);
-
- return add_to_breakpoint_chain (std::move (b));
-}
-
/* Initialize loc->function_name. */
static void
@@ -7344,31 +7305,6 @@ init_raw_breakpoint (struct breakpoint *b, struct symtab_and_line sal,
b->pspace = sal.pspace;
}
-/* set_raw_breakpoint is a low level routine for allocating and
- partially initializing a breakpoint of type BPTYPE. The newly
- created breakpoint's address, section, source file name, and line
- number are provided by SAL. The newly created and partially
- initialized breakpoint is added to the breakpoint chain and
- is also returned as the value of this function.
-
- It is expected that the caller will complete the initialization of
- the newly created breakpoint struct as well as output any status
- information regarding the creation of a new breakpoint. In
- particular, set_raw_breakpoint does NOT set the breakpoint
- number! Care should be taken to not allow an error to occur
- prior to completing the initialization of the breakpoint. If this
- should happen, a bogus breakpoint will be left on the chain. */
-
-static struct breakpoint *
-set_raw_breakpoint (struct gdbarch *gdbarch,
- struct symtab_and_line sal, enum bptype bptype)
-{
- std::unique_ptr<breakpoint> b = new_breakpoint_from_type (gdbarch, bptype);
-
- init_raw_breakpoint (b.get (), sal, bptype);
- return add_to_breakpoint_chain (std::move (b));
-}
-
/* Call this routine when stepping and nexting to enable a breakpoint
if we do a longjmp() or 'throw' in TP. FRAME is the frame which
initiated the operation. */
@@ -7977,6 +7913,17 @@ new_single_step_breakpoint (int thread, struct gdbarch *gdbarch)
return add_to_breakpoint_chain (std::move (b));
}
+/* Allocate a new momentary breakpoint. */
+
+static momentary_breakpoint *
+new_momentary_breakpoint (struct gdbarch *gdbarch, enum bptype type)
+{
+ if (type == bp_longjmp || type == bp_exception)
+ return new longjmp_breakpoint (gdbarch, type);
+ else
+ return new momentary_breakpoint (gdbarch, type);
+}
+
/* Set a momentary breakpoint of type TYPE at address specified by
SAL. If FRAME_ID is valid, the breakpoint is restricted to that
frame. */
@@ -7985,22 +7932,26 @@ breakpoint_up
set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
struct frame_id frame_id, enum bptype type)
{
- struct breakpoint *b;
-
/* If FRAME_ID is valid, it should be a real frame, not an inlined or
tail-called one. */
gdb_assert (!frame_id_artificial_p (frame_id));
- b = set_raw_breakpoint (gdbarch, sal, type);
+ std::unique_ptr<momentary_breakpoint> b
+ (new_momentary_breakpoint (gdbarch, type));
+
+ b->add_location (sal);
+ b->pspace = sal.pspace;
b->enable_state = bp_enabled;
b->disposition = disp_donttouch;
b->frame_id = frame_id;
b->thread = inferior_thread ()->global_num;
+ breakpoint_up bp (add_to_breakpoint_chain (std::move (b)));
+
update_global_location_list_nothrow (UGLL_MAY_INSERT);
- return breakpoint_up (b);
+ return bp;
}
/* Make a momentary breakpoint based on the master breakpoint ORIG.
@@ -8012,9 +7963,8 @@ momentary_breakpoint_from_master (struct breakpoint *orig,
enum bptype type,
int loc_enabled)
{
- struct breakpoint *copy;
-
- copy = set_raw_breakpoint_without_location (orig->gdbarch, type);
+ std::unique_ptr<breakpoint> copy
+ (new_momentary_breakpoint (orig->gdbarch, type));
copy->loc = copy->allocate_location ();
set_breakpoint_location_function (copy->loc);
@@ -8035,8 +7985,9 @@ momentary_breakpoint_from_master (struct breakpoint *orig,
copy->disposition = disp_donttouch;
copy->number = internal_breakpoint_number--;
+ breakpoint *b = add_to_breakpoint_chain (std::move (copy));
update_global_location_list_nothrow (UGLL_DONT_INSERT);
- return copy;
+ return b;
}
/* Make a deep copy of momentary breakpoint ORIG. Returns NULL if
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 14/23] Make exception_catchpoint inherit base_breakpoint instead of catchpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (12 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 13/23] Refactor momentary breakpoints, eliminate set_raw_breakpoint{, _without_location} Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 15/23] Make breakpoint_address_bits look at the location kind Pedro Alves
` (9 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
exception_catchpoint is really a code breakpoint, with locations set
by sals, re-set like other code breakpoints, etc., so make it inherit
base_breakpoint.
This adds a bit of duplicated code to exception_catchpoint's ctor
(copied from struct catchpoint's ctor), but it will be eliminated in a
following patch.
Change-Id: I9fbb2927491120e9744a4f5e5cb5e6870ca07009
---
gdb/break-catch-throw.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index f15fa531519..90fc3e6d325 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -63,15 +63,17 @@ static const struct exception_names exception_functions[] =
{ "-probe-stap libstdcxx:catch", "__cxa_begin_catch" }
};
-/* The type of an exception catchpoint. */
+/* The type of an exception catchpoint. Unlike most catchpoints, this
+ one is implemented with code breakpoints, so it inherits struct
+ base_breakpoint, not struct catchpoint. */
-struct exception_catchpoint : public catchpoint
+struct exception_catchpoint : public base_breakpoint
{
exception_catchpoint (struct gdbarch *gdbarch,
- bool temp, const char *cond_string,
+ bool temp, const char *cond_string_,
enum exception_event_kind kind_,
std::string &&except_rx)
- : catchpoint (gdbarch, temp, cond_string),
+ : base_breakpoint (gdbarch, bp_catchpoint),
kind (kind_),
exception_rx (std::move (except_rx)),
pattern (exception_rx.empty ()
@@ -79,6 +81,12 @@ struct exception_catchpoint : public catchpoint
: new compiled_regex (exception_rx.c_str (), REG_NOSUB,
_("invalid type-matching regexp")))
{
+ if (cond_string_ != nullptr)
+ cond_string = make_unique_xstrdup (cond_string_);
+ disposition = temp ? disp_del : disp_donttouch;
+
+ pspace = current_program_space;
+ re_set ();
}
void re_set () override;
@@ -375,8 +383,6 @@ handle_gnu_v3_exceptions (int tempflag, std::string &&except_rx,
(new exception_catchpoint (gdbarch, tempflag, cond_string,
ex_event, std::move (except_rx)));
- cp->re_set ();
-
install_breakpoint (0, std::move (cp), 1);
}
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 15/23] Make breakpoint_address_bits look at the location kind
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (13 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 14/23] Make exception_catchpoint inherit base_breakpoint instead of catchpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 16/23] Make catchpoint inherit breakpoint, eliminate init_raw_breakpoint Pedro Alves
` (8 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Software watchpoints allocate a special dummy location using
software_watchpoint_add_no_memory_location, and then
breakpoint_address_bits checks whether the location is that special
location to decide whether the location has a meaninful address to
print.
Introduce a new bp_loc_software_watchpoint location kind, and make
breakpoint_address_bits use bl_address_is_meaningful instead, which
returns false for bp_loc_other, which is in accordance with we
document for bp_location::address:
/* (... snip ...) Valid for all types except
bp_loc_other. */
CORE_ADDR address = 0;
Rename software_watchpoint_add_no_memory_location to
add_dummy_location, and simplify it. This will be used by catchpoints
too in a following patch.
Note that neither "info breakpoints" nor "maint info breakpoints"
actually prints the addresses of watchpoints, but I think it would be
useful to do so in "maint info breakpoints". This approach let's us
implement that in the future.
Change-Id: I50e398f66ef618c31ffa662da755eaba6295aed7
---
gdb/breakpoint.c | 59 ++++++++++++++++++------------------------------
gdb/breakpoint.h | 1 +
2 files changed, 23 insertions(+), 37 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6e24137ec4b..6dfb89f51bc 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -164,6 +164,8 @@ static std::vector<symtab_and_line> bkpt_probe_decode_location
struct event_location *location,
struct program_space *search_pspace);
+static bool bl_address_is_meaningful (bp_location *loc);
+
/* update_global_location_list's modes of operation wrt to whether to
insert locations now. */
enum ugll_insert_mode
@@ -1830,34 +1832,18 @@ extract_bitfield_from_watchpoint_value (struct watchpoint *w, struct value *val)
return bit_val;
}
-/* Allocate a dummy location and add it to B, which must be a software
- watchpoint. This is required because even if a software watchpoint
- is not watching any memory, bpstat_stop_status requires a location
- to be able to report stops. */
+/* Allocate a dummy location and add it to B. This is required
+ because bpstat_stop_status requires a location to be able to report
+ stops. */
static void
-software_watchpoint_add_no_memory_location (struct breakpoint *b,
- struct program_space *pspace)
+add_dummy_location (struct breakpoint *b,
+ struct program_space *pspace)
{
- gdb_assert (b->type == bp_watchpoint && b->loc == NULL);
+ gdb_assert (b->loc == NULL);
- b->loc = b->allocate_location ();
+ b->loc = new bp_location (b, bp_loc_other);
b->loc->pspace = pspace;
- b->loc->address = -1;
- b->loc->length = -1;
-}
-
-/* Returns true if B is a software watchpoint that is not watching any
- memory (e.g., "watch $pc"). */
-
-static bool
-is_no_memory_software_watchpoint (struct breakpoint *b)
-{
- return (b->type == bp_watchpoint
- && b->loc != NULL
- && b->loc->next == NULL
- && b->loc->address == -1
- && b->loc->length == -1);
}
/* Assuming that B is a watchpoint:
@@ -2197,7 +2183,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
else
b->type = bp_watchpoint;
- loc_type = (b->type == bp_watchpoint? bp_loc_other
+ loc_type = (b->type == bp_watchpoint? bp_loc_software_watchpoint
: bp_loc_hardware_watchpoint);
for (bp_location *bl : b->locations ())
bl->loc_type = loc_type;
@@ -2208,7 +2194,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
bpstat_stop_status requires a location to be able to report
stops, so make sure there's at least a dummy one. */
if (b->type == bp_watchpoint && b->loc == NULL)
- software_watchpoint_add_no_memory_location (b, frame_pspace);
+ add_dummy_location (b, frame_pspace);
}
else if (!within_current_scope)
{
@@ -3840,11 +3826,12 @@ detach_breakpoints (ptid_t ptid)
/* This function must physically remove breakpoints locations
from the specified ptid, without modifying the breakpoint
- package's state. Locations of type bp_loc_other are only
- maintained at GDB side. So, there is no need to remove
- these bp_loc_other locations. Moreover, removing these
+ package's state. Locations of type bp_loc_other and
+ bp_loc_software_watchpoint are only maintained at GDB side,
+ so there is no need to remove them. Moreover, removing these
would modify the breakpoint package's state. */
- if (bl->loc_type == bp_loc_other)
+ if (bl->loc_type == bp_loc_other
+ || bl->loc_type == bp_loc_software_watchpoint)
continue;
if (bl->inserted)
@@ -5798,7 +5785,8 @@ bpstat_what (bpstat *bs_head)
{
/* Some catchpoints are implemented with breakpoints.
For those, we need to step over the breakpoint. */
- if (bs->bp_location_at->loc_type != bp_loc_other)
+ if (bs->bp_location_at->loc_type == bp_loc_software_breakpoint
+ || bs->bp_location_at->loc_type == bp_loc_hardware_breakpoint)
this_action = BPSTAT_WHAT_SINGLE;
}
break;
@@ -6557,16 +6545,12 @@ breakpoint_address_bits (struct breakpoint *b)
{
int print_address_bits = 0;
- /* Software watchpoints that aren't watching memory don't have an
- address to print. */
- if (is_no_memory_software_watchpoint (b))
- return 0;
-
for (bp_location *loc : b->locations ())
{
- int addr_bit;
+ if (!bl_address_is_meaningful (loc))
+ continue;
- addr_bit = gdbarch_addr_bit (loc->gdbarch);
+ int addr_bit = gdbarch_addr_bit (loc->gdbarch);
if (addr_bit > print_address_bits)
print_address_bits = addr_bit;
}
@@ -7161,6 +7145,7 @@ bp_location_from_bp_type (bptype type)
case bp_access_watchpoint:
return bp_loc_hardware_watchpoint;
case bp_watchpoint:
+ return bp_loc_software_watchpoint;
case bp_catchpoint:
case bp_tracepoint:
case bp_fast_tracepoint:
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index a4ead8b4d4e..7375e976dc6 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -314,6 +314,7 @@ enum bp_loc_type
{
bp_loc_software_breakpoint,
bp_loc_hardware_breakpoint,
+ bp_loc_software_watchpoint,
bp_loc_hardware_watchpoint,
bp_loc_other /* Miscellaneous... */
};
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 16/23] Make catchpoint inherit breakpoint, eliminate init_raw_breakpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (14 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 15/23] Make breakpoint_address_bits look at the location kind Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 17/23] Move common bits of catchpoint/exception_catchpoint to breakpoint's ctor Pedro Alves
` (7 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
struct catchpoint's ctor currently calls init_raw_breakpoint, which is
a bit weird, as that ctor-like function takes a sal argument, but
catchpoints don't have code locations.
Instead, make struct catchpoint's ctor add the catchpoint's dummy
location using add_dummy_location.
init_raw_breakpoint uses add_location under the hood, and with a dummy
sal it would ultimately use the breakpoint's gdbarch for the
location's gdbarch, so replace the references to loc->gdbarch (which
is now NULL) in syscall_catchpoint to references to the catchpoint's
gdbarch.
struct catchpoint's ctor was the last user of init_raw_breakpoint, so
this commit eliminates the latter.
Since catchpoint locations aren't code locations, make struct
catchpoint inherit struct breakpoint instead of base_breakpoint. This
let's us delete the tracepoint::re_set override too.
Change-Id: Ib428bf71efb09fdaf399c56e4372b0f41d9c5869
---
gdb/break-catch-syscall.c | 6 +++---
gdb/breakpoint.c | 31 +++----------------------------
gdb/breakpoint.h | 7 +------
3 files changed, 7 insertions(+), 37 deletions(-)
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index af55ecb1b5c..06d48466de7 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -197,7 +197,7 @@ syscall_catchpoint::print_it (const bpstat *bs) const
must print "called syscall" or "returned from syscall". */
struct target_waitstatus last;
struct syscall s;
- struct gdbarch *gdbarch = bs->bp_location_at->gdbarch;
+ struct gdbarch *gdbarch = b->gdbarch;
get_last_target_status (nullptr, nullptr, &last);
@@ -242,7 +242,7 @@ syscall_catchpoint::print_one (bp_location **last_loc) const
{
struct value_print_options opts;
struct ui_out *uiout = current_uiout;
- struct gdbarch *gdbarch = loc->gdbarch;
+ struct gdbarch *gdbarch = loc->owner->gdbarch;
get_user_print_options (&opts);
/* Field 4, the address, is omitted (which makes the columns not
@@ -293,7 +293,7 @@ syscall_catchpoint::print_one (bp_location **last_loc) const
void
syscall_catchpoint::print_mention () const
{
- struct gdbarch *gdbarch = loc->gdbarch;
+ struct gdbarch *gdbarch = loc->owner->gdbarch;
if (!syscalls_to_be_caught.empty ())
{
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6dfb89f51bc..bb433463c55 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7266,30 +7266,6 @@ get_sal_arch (struct symtab_and_line sal)
return NULL;
}
-/* Low level routine for partially initializing a breakpoint of type
- BPTYPE. The newly created breakpoint's address, section, source
- file name, and line number are provided by SAL.
-
- It is expected that the caller will complete the initialization of
- the newly created breakpoint struct as well as output any status
- information regarding the creation of a new breakpoint. */
-
-static void
-init_raw_breakpoint (struct breakpoint *b, struct symtab_and_line sal,
- enum bptype bptype)
-{
- b->add_location (sal);
-
- if (bptype != bp_catchpoint)
- gdb_assert (sal.pspace != NULL);
-
- /* Store the program space that was used to set the breakpoint,
- except for ordinary breakpoints, which are independent of the
- program space. */
- if (bptype != bp_breakpoint && bptype != bp_hardware_breakpoint)
- b->pspace = sal.pspace;
-}
-
/* Call this routine when stepping and nexting to enable a breakpoint
if we do a longjmp() or 'throw' in TP. FRAME is the frame which
initiated the operation. */
@@ -7751,12 +7727,11 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
catchpoint::catchpoint (struct gdbarch *gdbarch, bool temp,
const char *cond_string_)
- : base_breakpoint (gdbarch, bp_catchpoint)
+ : breakpoint (gdbarch, bp_catchpoint)
{
- symtab_and_line sal;
- sal.pspace = current_program_space;
+ add_dummy_location (this, current_program_space);
- init_raw_breakpoint (this, sal, bp_catchpoint);
+ pspace = current_program_space;
if (cond_string_ != nullptr)
cond_string = make_unique_xstrdup (cond_string_);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 7375e976dc6..21dd8d53b2d 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1030,18 +1030,13 @@ struct tracepoint : public base_breakpoint
/* The base class for catchpoints. */
-struct catchpoint : public base_breakpoint
+struct catchpoint : public breakpoint
{
/* If TEMP is true, then make the breakpoint temporary. If
COND_STRING is not NULL, then store it in the breakpoint. */
catchpoint (struct gdbarch *gdbarch, bool temp, const char *cond_string);
~catchpoint () override = 0;
-
- void re_set () override
- {
- /* For catchpoints, the default is to do nothing. */
- }
};
\f
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 17/23] Move common bits of catchpoint/exception_catchpoint to breakpoint's ctor
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (15 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 16/23] Make catchpoint inherit breakpoint, eliminate init_raw_breakpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 18/23] Move add_location(sal) to base_breakpoint Pedro Alves
` (6 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Move common bits of catchpoint and exception_catchpoint to
breakpoint's ctor, to avoid duplicating code.
Change-Id: I3a115180f4d496426522f1d89a3875026aea3cf2
---
gdb/break-catch-throw.c | 6 +-----
gdb/breakpoint.c | 24 ++++++++++++++++++------
gdb/breakpoint.h | 10 ++--------
3 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 90fc3e6d325..a6f477b712a 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -73,7 +73,7 @@ struct exception_catchpoint : public base_breakpoint
bool temp, const char *cond_string_,
enum exception_event_kind kind_,
std::string &&except_rx)
- : base_breakpoint (gdbarch, bp_catchpoint),
+ : base_breakpoint (gdbarch, bp_catchpoint, temp, cond_string_),
kind (kind_),
exception_rx (std::move (except_rx)),
pattern (exception_rx.empty ()
@@ -81,10 +81,6 @@ struct exception_catchpoint : public base_breakpoint
: new compiled_regex (exception_rx.c_str (), REG_NOSUB,
_("invalid type-matching regexp")))
{
- if (cond_string_ != nullptr)
- cond_string = make_unique_xstrdup (cond_string_);
- disposition = temp ? disp_del : disp_donttouch;
-
pspace = current_program_space;
re_set ();
}
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bb433463c55..4dce43b1e6b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7725,17 +7725,29 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
/* See breakpoint.h. */
+breakpoint::breakpoint (struct gdbarch *gdbarch_, enum bptype bptype,
+ bool temp, const char *cond_string_)
+ : type (bptype),
+ disposition (temp ? disp_del : disp_donttouch),
+ gdbarch (gdbarch_),
+ language (current_language->la_language),
+ input_radix (::input_radix),
+ cond_string (cond_string_ != nullptr
+ ? make_unique_xstrdup (cond_string_)
+ : nullptr),
+ related_breakpoint (this)
+{
+}
+
+/* See breakpoint.h. */
+
catchpoint::catchpoint (struct gdbarch *gdbarch, bool temp,
- const char *cond_string_)
- : breakpoint (gdbarch, bp_catchpoint)
+ const char *cond_string)
+ : breakpoint (gdbarch, bp_catchpoint, temp, cond_string)
{
add_dummy_location (this, current_program_space);
pspace = current_program_space;
-
- if (cond_string_ != nullptr)
- cond_string = make_unique_xstrdup (cond_string_);
- disposition = temp ? disp_del : disp_donttouch;
}
void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 21dd8d53b2d..af69af6863b 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -620,14 +620,8 @@ using bp_location_range = next_range<bp_location>;
struct breakpoint
{
- breakpoint (struct gdbarch *gdbarch_, enum bptype bptype)
- : type (bptype),
- gdbarch (gdbarch_),
- language (current_language->la_language),
- input_radix (::input_radix),
- related_breakpoint (this)
- {
- }
+ breakpoint (struct gdbarch *gdbarch_, enum bptype bptype,
+ bool temp = true, const char *cond_string = nullptr);
DISABLE_COPY_AND_ASSIGN (breakpoint);
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 18/23] Move add_location(sal) to base_breakpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (16 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 17/23] Move common bits of catchpoint/exception_catchpoint to breakpoint's ctor Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 19/23] Add/tweak intro comments of struct breakpoint and several subclasses Pedro Alves
` (5 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
After the previous patches, only base_breakpoint subclasses use
add_location(sal), so we can move it to base_breakpoint (a.k.a. base
class for code breakpoints).
This requires a few casts here and there, but always at spots where
you can see from context what the breakpoint's type actually is.
I inlined new_single_step_breakpoint into its only caller exactly for
this reason.
I did try to propagate more use of base_breakpoint to avoid casts, but
that turned out unwieldy for this patch.
Change-Id: I49d959322b0fdce5a88a216bb44730fc5dd7c6f8
---
gdb/breakpoint.c | 45 +++++++++++++++++++--------------------------
gdb/breakpoint.h | 8 ++++----
gdb/elfread.c | 6 +++---
gdb/minsyms.c | 4 ++--
gdb/symtab.h | 5 +++--
5 files changed, 31 insertions(+), 37 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4dce43b1e6b..613f6a875e4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -87,7 +87,7 @@
static void map_breakpoint_numbers (const char *,
gdb::function_view<void (breakpoint *)>);
-static void breakpoint_re_set_default (struct breakpoint *);
+static void breakpoint_re_set_default (base_breakpoint *);
static void
create_sals_from_location_default (struct event_location *location,
@@ -5860,10 +5860,10 @@ bpstat_run_callbacks (bpstat *bs_head)
handle_jit_event (bs->bp_location_at->address);
break;
case bp_gnu_ifunc_resolver:
- gnu_ifunc_resolver_stop (b);
+ gnu_ifunc_resolver_stop ((base_breakpoint *) b);
break;
case bp_gnu_ifunc_resolver_return:
- gnu_ifunc_resolver_return_stop (b);
+ gnu_ifunc_resolver_return_stop ((base_breakpoint *) b);
break;
}
}
@@ -7867,24 +7867,6 @@ 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)
-{
- std::unique_ptr<breakpoint> b (new momentary_breakpoint (gdbarch,
- bp_single_step));
-
- b->disposition = disp_donttouch;
- b->frame_id = null_frame_id;
-
- b->thread = thread;
- gdb_assert (b->thread != 0);
-
- return add_to_breakpoint_chain (std::move (b));
-}
-
/* Allocate a new momentary breakpoint. */
static momentary_breakpoint *
@@ -8057,7 +8039,7 @@ handle_automatic_hardware_breakpoints (bp_location *bl)
}
bp_location *
-breakpoint::add_location (const symtab_and_line &sal)
+base_breakpoint::add_location (const symtab_and_line &sal)
{
struct bp_location *new_loc, **tmp;
CORE_ADDR adjusted_address;
@@ -12480,7 +12462,7 @@ hoist_existing_locations (struct breakpoint *b, struct program_space *pspace)
untouched. */
void
-update_breakpoint_locations (struct breakpoint *b,
+update_breakpoint_locations (base_breakpoint *b,
struct program_space *filter_pspace,
gdb::array_view<const symtab_and_line> sals,
gdb::array_view<const symtab_and_line> sals_end)
@@ -12688,7 +12670,7 @@ location_to_sals (struct breakpoint *b, struct event_location *location,
locations. */
static void
-breakpoint_re_set_default (struct breakpoint *b)
+breakpoint_re_set_default (base_breakpoint *b)
{
struct program_space *filter_pspace = current_program_space;
std::vector<symtab_and_line> expanded, expanded_end;
@@ -13399,15 +13381,26 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
if (tp->control.single_step_breakpoints == NULL)
{
+ std::unique_ptr<breakpoint> b
+ (new momentary_breakpoint (gdbarch, bp_single_step));
+
+ b->disposition = disp_donttouch;
+
+ b->thread = tp->global_num;
+ gdb_assert (b->thread != 0);
+
tp->control.single_step_breakpoints
- = new_single_step_breakpoint (tp->global_num, gdbarch);
+ = add_to_breakpoint_chain (std::move (b));
}
sal = find_pc_line (pc, 0);
sal.pc = pc;
sal.section = find_pc_overlay (pc);
sal.explicit_pc = 1;
- tp->control.single_step_breakpoints->add_location (sal);
+
+ auto *ss_bp
+ = static_cast<momentary_breakpoint *> (tp->control.single_step_breakpoints);
+ ss_bp->add_location (sal);
update_global_location_list (UGLL_INSERT);
}
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index af69af6863b..807c97a5bed 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -733,9 +733,6 @@ struct breakpoint
/* Nothing to do. */
}
- /* Add a location for SAL to this breakpoint. */
- bp_location *add_location (const symtab_and_line &sal);
-
/* Return a range of this breakpoint's locations. */
bp_location_range locations () const;
@@ -874,6 +871,9 @@ struct base_breakpoint : public breakpoint
~base_breakpoint () override = 0;
+ /* Add a location for SAL to this breakpoint. */
+ bp_location *add_location (const symtab_and_line &sal);
+
void re_set () override;
int insert_location (struct bp_location *) override;
int remove_location (struct bp_location *,
@@ -1383,7 +1383,7 @@ extern void until_break_command (const char *, int, int);
/* Initialize a struct bp_location. */
extern void update_breakpoint_locations
- (struct breakpoint *b,
+ (base_breakpoint *b,
struct program_space *filter_pspace,
gdb::array_view<const symtab_and_line> sals,
gdb::array_view<const symtab_and_line> sals_end);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index b136c605b1a..27203722802 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -927,7 +927,7 @@ elf_gnu_ifunc_resolve_addr (struct gdbarch *gdbarch, CORE_ADDR pc)
/* Handle inferior hit of bp_gnu_ifunc_resolver, see its definition. */
static void
-elf_gnu_ifunc_resolver_stop (struct breakpoint *b)
+elf_gnu_ifunc_resolver_stop (base_breakpoint *b)
{
struct breakpoint *b_return;
struct frame_info *prev_frame = get_prev_frame (get_current_frame ());
@@ -978,7 +978,7 @@ elf_gnu_ifunc_resolver_stop (struct breakpoint *b)
/* Handle inferior hit of bp_gnu_ifunc_resolver_return, see its definition. */
static void
-elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
+elf_gnu_ifunc_resolver_return_stop (base_breakpoint *b)
{
thread_info *thread = inferior_thread ();
struct gdbarch *gdbarch = get_frame_arch (get_current_frame ());
@@ -1008,7 +1008,7 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
"gnu-indirect-function breakpoint type %d"),
(int) b->type);
}
- b = b_next;
+ b = (base_breakpoint *) b_next;
}
gdb_assert (b->type == bp_gnu_ifunc_resolver);
gdb_assert (b->loc->next == NULL);
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index cbd0ad22392..217ee047446 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1019,7 +1019,7 @@ stub_gnu_ifunc_resolve_name (const char *function_name,
/* See elf_gnu_ifunc_resolver_stop for its real implementation. */
static void
-stub_gnu_ifunc_resolver_stop (struct breakpoint *b)
+stub_gnu_ifunc_resolver_stop (base_breakpoint *b)
{
internal_error (__FILE__, __LINE__,
_("elf_gnu_ifunc_resolver_stop cannot be reached."));
@@ -1028,7 +1028,7 @@ stub_gnu_ifunc_resolver_stop (struct breakpoint *b)
/* See elf_gnu_ifunc_resolver_return_stop for its real implementation. */
static void
-stub_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
+stub_gnu_ifunc_resolver_return_stop (base_breakpoint *b)
{
internal_error (__FILE__, __LINE__,
_("elf_gnu_ifunc_resolver_return_stop cannot be reached."));
diff --git a/gdb/symtab.h b/gdb/symtab.h
index b1cf84f756f..5218be587de 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -55,6 +55,7 @@ struct obj_section;
struct cmd_list_element;
class probe;
struct lookup_name_info;
+struct base_breakpoint;
/* How to match a lookup name against a symbol search name. */
enum class symbol_name_match_type
@@ -2227,10 +2228,10 @@ struct gnu_ifunc_fns
CORE_ADDR *function_address_p);
/* See elf_gnu_ifunc_resolver_stop for its real implementation. */
- void (*gnu_ifunc_resolver_stop) (struct breakpoint *b);
+ void (*gnu_ifunc_resolver_stop) (base_breakpoint *b);
/* See elf_gnu_ifunc_resolver_return_stop for its real implementation. */
- void (*gnu_ifunc_resolver_return_stop) (struct breakpoint *b);
+ void (*gnu_ifunc_resolver_return_stop) (base_breakpoint *b);
};
#define gnu_ifunc_resolve_addr gnu_ifunc_fns_p->gnu_ifunc_resolve_addr
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 19/23] Add/tweak intro comments of struct breakpoint and several subclasses
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (17 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 18/23] Move add_location(sal) to base_breakpoint Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 20/23] Momentary breakpoints should have no breakpoint number Pedro Alves
` (4 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This tweaks the intro comments of the following classes:
internal_breakpoint
momentary_breakpoint
breakpoint
base_breakpoint
watchpoint
catchpoint
Change-Id: If6b31f51ebbb81705fbe5b8435f60ab2c88a98c8
---
gdb/breakpoint.c | 12 ++++++++++--
gdb/breakpoint.h | 13 +++++++------
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 613f6a875e4..0d239f50fcc 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -271,7 +271,10 @@ struct ordinary_breakpoint : public base_breakpoint
void print_recreate (struct ui_file *fp) const override;
};
-/* Internal breakpoints. */
+/* Internal breakpoints. These typically have a lifetime the same as
+ the program, and they end up installed on the breakpoint chain with
+ a negative breakpoint number. They're visible in "maint info
+ breakpoints", but not "info breakpoints". */
struct internal_breakpoint : public base_breakpoint
{
internal_breakpoint (struct gdbarch *gdbarch,
@@ -294,7 +297,12 @@ struct internal_breakpoint : public base_breakpoint
void print_mention () const override;
};
-/* Momentary breakpoints. */
+/* Momentary breakpoints. These typically have a lifetime of some run
+ control command only, are always thread-specific, and have 0 for
+ breakpoint number. I.e., there can be many momentary breakpoints
+ on the breakpoint chain and they all same the same number (zero).
+ They're visible in "maint info breakpoints", but not "info
+ breakpoints". */
struct momentary_breakpoint : public base_breakpoint
{
using base_breakpoint::base_breakpoint;
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 807c97a5bed..8735396e8d4 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -616,7 +616,7 @@ using bp_location_range = next_range<bp_location>;
useful for a hack I had to put in; I'm going to leave it in because
I can see how there might be times when it would indeed be useful */
-/* This is for all kinds of breakpoints. */
+/* Abstract base class representing all kinds of breakpoints. */
struct breakpoint
{
@@ -846,9 +846,9 @@ struct breakpoint
void print_recreate_thread (struct ui_file *fp) const;
};
-/* The structure to be inherit by all kinds of breakpoints (real
- breakpoints, i.e., user "break" breakpoints, internal and momentary
- breakpoints, etc.). */
+/* Abstract base class representing code breakpoints. User "break"
+ breakpoints, internal and momentary breakpoints, etc. IOW, any
+ kind of breakpoint whose locations are created from SALs. */
struct base_breakpoint : public breakpoint
{
using breakpoint::breakpoint;
@@ -887,7 +887,8 @@ struct base_breakpoint : public breakpoint
struct program_space *search_pspace) override;
};
-/* An instance of this type is used to represent a watchpoint. */
+/* An instance of this type is used to represent a watchpoint,
+ a.k.a. a data breakpoint. */
struct watchpoint : public breakpoint
{
@@ -1022,7 +1023,7 @@ struct tracepoint : public base_breakpoint
int static_trace_marker_id_idx = 0;
};
-/* The base class for catchpoints. */
+/* The abstract base class for catchpoints. */
struct catchpoint : public breakpoint
{
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 20/23] Momentary breakpoints should have no breakpoint number
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (18 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 19/23] Add/tweak intro comments of struct breakpoint and several subclasses Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-20 16:00 ` Tom Tromey
2022-05-16 18:40 ` [PATCH 21/23] Make sure momentary breakpoints are always thread-specific Pedro Alves
` (3 subsequent siblings)
23 siblings, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Momentary breakpoints have no breakpoint number, their breakpoint
number should be always 0, to avoid constantly incrementing (or
decrementing) the internal breakpoint count.
Indeed, set_momentary_breakpoint installs the created breakpoint
without a number.
However, momentary_breakpoint_from_master incorrectly gives an
internal breakpoint number to the new breakpoint. This commit fixes
that.
Change-Id: Iedcae5432cdf232db9e9a6e1a646d358abd34f95
---
gdb/breakpoint.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0d239f50fcc..4f664cbd7aa 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7945,7 +7945,6 @@ momentary_breakpoint_from_master (struct breakpoint *orig,
copy->enable_state = bp_enabled;
copy->disposition = disp_donttouch;
- copy->number = internal_breakpoint_number--;
breakpoint *b = add_to_breakpoint_chain (std::move (copy));
update_global_location_list_nothrow (UGLL_DONT_INSERT);
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 20/23] Momentary breakpoints should have no breakpoint number
2022-05-16 18:40 ` [PATCH 20/23] Momentary breakpoints should have no breakpoint number Pedro Alves
@ 2022-05-20 16:00 ` Tom Tromey
2022-05-20 19:25 ` Pedro Alves
0 siblings, 1 reply; 31+ messages in thread
From: Tom Tromey @ 2022-05-20 16:00 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> Momentary breakpoints have no breakpoint number, their breakpoint
Pedro> number should be always 0, to avoid constantly incrementing (or
Pedro> decrementing) the internal breakpoint count.
Pedro> Indeed, set_momentary_breakpoint installs the created breakpoint
Pedro> without a number.
Pedro> However, momentary_breakpoint_from_master incorrectly gives an
Pedro> internal breakpoint number to the new breakpoint. This commit fixes
Pedro> that.
I think it would probably be good to document this somewhere.
Like, maybe the deleted line could be replaced with a comment.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 20/23] Momentary breakpoints should have no breakpoint number
2022-05-20 16:00 ` Tom Tromey
@ 2022-05-20 19:25 ` Pedro Alves
0 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-20 19:25 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2022-05-20 17:00, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>
> Pedro> Momentary breakpoints have no breakpoint number, their breakpoint
> Pedro> number should be always 0, to avoid constantly incrementing (or
> Pedro> decrementing) the internal breakpoint count.
>
> Pedro> Indeed, set_momentary_breakpoint installs the created breakpoint
> Pedro> without a number.
>
> Pedro> However, momentary_breakpoint_from_master incorrectly gives an
> Pedro> internal breakpoint number to the new breakpoint. This commit fixes
> Pedro> that.
>
> I think it would probably be good to document this somewhere.
> Like, maybe the deleted line could be replaced with a comment.
I think if we do it here, we'd need to do it in set_momentary_breakpoint
too. I had already added such a comment on top of struct momentary_breakpoint,
though, in patch #19:
-/* Momentary breakpoints. */
+/* Momentary breakpoints. These typically have a lifetime of some run
+ control command only, are always thread-specific, and have 0 for
+ breakpoint number. I.e., there can be many momentary breakpoints
+ on the breakpoint chain and they all same the same number (zero).
+ They're visible in "maint info breakpoints", but not "info
+ breakpoints". */
struct momentary_breakpoint : public base_breakpoint
{
Maybe we could find a spot to put an assert at. Seems difficult without
relying on checking breakpoint types, though...
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 21/23] Make sure momentary breakpoints are always thread-specific
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (19 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 20/23] Momentary breakpoints should have no breakpoint number Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 22/23] Test "set multiple-symbols on" creating multiple breakpoints Pedro Alves
` (2 subsequent siblings)
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
This adds a new ctor to momentary_breakpoints with a few parameters
that are always necessary for momentary breakpoints.
In particular, I noticed that set_std_terminate_breakpoint doesn't
make the breakpoint be thread specific, which looks like a bug to me.
The point of that breakpoint is to intercept std::terminate calls that
happen as result of the called thread throwing an exception that won't
be caught by the dummy frame. If some other thread calls
std::terminate, IMO, it's no different from some other thread calling
exit/_exit, for example.
Change-Id: Ifc5ff4a6d6e58b8c4854d00b86725382d38a1a02
---
gdb/breakpoint.c | 83 +++++++++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 37 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4f664cbd7aa..f4642c2d8e7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -118,7 +118,7 @@ static breakpoint *add_to_breakpoint_chain (std::unique_ptr<breakpoint> &&b);
static struct breakpoint *
momentary_breakpoint_from_master (struct breakpoint *orig,
enum bptype type,
- int loc_enabled);
+ int loc_enabled, int thread);
static void breakpoint_adjustment_warning (CORE_ADDR, CORE_ADDR, int, int);
@@ -305,7 +305,25 @@ struct internal_breakpoint : public base_breakpoint
breakpoints". */
struct momentary_breakpoint : public base_breakpoint
{
- using base_breakpoint::base_breakpoint;
+ momentary_breakpoint (struct gdbarch *gdbarch_, enum bptype bptype,
+ program_space *pspace_,
+ const struct frame_id &frame_id_,
+ int thread_)
+ : base_breakpoint (gdbarch_, bptype)
+ {
+ /* If FRAME_ID is valid, it should be a real frame, not an inlined
+ or tail-called one. */
+ gdb_assert (!frame_id_artificial_p (frame_id));
+
+ /* Momentary breakpoints are always thread-specific. */
+ gdb_assert (thread_ > 0);
+
+ pspace = pspace_;
+ enable_state = bp_enabled;
+ disposition = disp_donttouch;
+ frame_id = frame_id_;
+ thread = thread_;
+ }
void re_set () override;
void check_status (struct bpstat *bs) override;
@@ -7293,12 +7311,9 @@ set_longjmp_breakpoint (struct thread_info *tp, struct frame_id frame)
|| b->type == bp_exception_master))
{
enum bptype type = b->type == bp_longjmp_master ? bp_longjmp : bp_exception;
- struct breakpoint *clone;
-
/* longjmp_breakpoint_ops ensures INITIATING_FRAME is cleared again
after their removal. */
- clone = momentary_breakpoint_from_master (b, type, 1);
- clone->thread = thread;
+ momentary_breakpoint_from_master (b, type, 1, thread);
}
tp->initiating_frame = frame;
@@ -7340,11 +7355,10 @@ set_longjmp_breakpoint_for_call_dummy (void)
for (breakpoint *b : all_breakpoints ())
if (b->pspace == current_program_space && b->type == bp_longjmp_master)
{
- struct breakpoint *new_b;
-
- new_b = momentary_breakpoint_from_master (b, bp_longjmp_call_dummy,
- 1);
- new_b->thread = inferior_thread ()->global_num;
+ int thread = inferior_thread ()->global_num;
+ breakpoint *new_b
+ = momentary_breakpoint_from_master (b, bp_longjmp_call_dummy,
+ 1, thread);
/* Link NEW_B into the chain of RETVAL breakpoints. */
@@ -7473,7 +7487,8 @@ set_std_terminate_breakpoint (void)
if (b->pspace == current_program_space
&& b->type == bp_std_terminate_master)
{
- momentary_breakpoint_from_master (b, bp_std_terminate, 1);
+ momentary_breakpoint_from_master (b, bp_std_terminate, 1,
+ inferior_thread ()->global_num);
}
}
@@ -7877,13 +7892,17 @@ enable_breakpoints_after_startup (void)
/* Allocate a new momentary breakpoint. */
+template<typename... Arg>
static momentary_breakpoint *
-new_momentary_breakpoint (struct gdbarch *gdbarch, enum bptype type)
+new_momentary_breakpoint (struct gdbarch *gdbarch, enum bptype type,
+ Arg&&... args)
{
if (type == bp_longjmp || type == bp_exception)
- return new longjmp_breakpoint (gdbarch, type);
+ return new longjmp_breakpoint (gdbarch, type,
+ std::forward<Arg> (args)...);
else
- return new momentary_breakpoint (gdbarch, type);
+ return new momentary_breakpoint (gdbarch, type,
+ std::forward<Arg> (args)...);
}
/* Set a momentary breakpoint of type TYPE at address specified by
@@ -7899,15 +7918,10 @@ set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
gdb_assert (!frame_id_artificial_p (frame_id));
std::unique_ptr<momentary_breakpoint> b
- (new_momentary_breakpoint (gdbarch, type));
+ (new_momentary_breakpoint (gdbarch, type, sal.pspace, frame_id,
+ inferior_thread ()->global_num));
b->add_location (sal);
- b->pspace = sal.pspace;
- b->enable_state = bp_enabled;
- b->disposition = disp_donttouch;
- b->frame_id = frame_id;
-
- b->thread = inferior_thread ()->global_num;
breakpoint_up bp (add_to_breakpoint_chain (std::move (b)));
@@ -7923,10 +7937,12 @@ set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
static struct breakpoint *
momentary_breakpoint_from_master (struct breakpoint *orig,
enum bptype type,
- int loc_enabled)
+ int loc_enabled,
+ int thread)
{
std::unique_ptr<breakpoint> copy
- (new_momentary_breakpoint (orig->gdbarch, type));
+ (new_momentary_breakpoint (orig->gdbarch, type, orig->pspace,
+ orig->frame_id, thread));
copy->loc = copy->allocate_location ();
set_breakpoint_location_function (copy->loc);
@@ -7939,12 +7955,6 @@ momentary_breakpoint_from_master (struct breakpoint *orig,
copy->loc->line_number = orig->loc->line_number;
copy->loc->symtab = orig->loc->symtab;
copy->loc->enabled = loc_enabled;
- copy->frame_id = orig->frame_id;
- copy->thread = orig->thread;
- copy->pspace = orig->pspace;
-
- copy->enable_state = bp_enabled;
- copy->disposition = disp_donttouch;
breakpoint *b = add_to_breakpoint_chain (std::move (copy));
update_global_location_list_nothrow (UGLL_DONT_INSERT);
@@ -7961,7 +7971,8 @@ clone_momentary_breakpoint (struct breakpoint *orig)
if (orig == NULL)
return NULL;
- return momentary_breakpoint_from_master (orig, orig->type, 0);
+ return momentary_breakpoint_from_master (orig, orig->type, 0,
+ orig->thread);
}
breakpoint_up
@@ -13389,12 +13400,10 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
if (tp->control.single_step_breakpoints == NULL)
{
std::unique_ptr<breakpoint> b
- (new momentary_breakpoint (gdbarch, bp_single_step));
-
- b->disposition = disp_donttouch;
-
- b->thread = tp->global_num;
- gdb_assert (b->thread != 0);
+ (new momentary_breakpoint (gdbarch, bp_single_step,
+ current_program_space,
+ null_frame_id,
+ tp->global_num));
tp->control.single_step_breakpoints
= add_to_breakpoint_chain (std::move (b));
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 22/23] Test "set multiple-symbols on" creating multiple breakpoints
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (20 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 21/23] Make sure momentary breakpoints are always thread-specific Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-16 18:40 ` [PATCH 23/23] Rename base_breakpoint -> code_breakpoint Pedro Alves
2022-05-20 16:06 ` [PATCH 00/23] More breakpoints cleanups Tom Tromey
23 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
To look for code paths that lead to create_breakpoints_sal creating
multiple breakpoints, I ran the whole testsuite with this hack:
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8377,8 +8377,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
int from_tty,
int enabled, int internal, unsigned flags)
{
- if (canonical->pre_expanded)
- gdb_assert (canonical->lsals.size () == 1);
+ gdb_assert (canonical->lsals.size () == 1);
surprisingly, the assert never failed...
The way to get to create_breakpoints_sal with multiple lsals is to use
"set multiple-symbols ask" and then select multiple options from the
menu, like so:
(gdb) set multiple-symbols ask
(gdb) b overload1arg
[0] cancel
[1] all
[2] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg()
[3] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(char)
[4] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(double)
[5] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(float)
[6] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(int)
[7] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(long)
[8] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(short)
[9] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(signed char)
[10] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned char)
[11] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned int)
[12] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned long)
[13] /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc:foo::overload1arg(unsigned short)
> 2-3
Breakpoint 2 at 0x1532: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc, line 107.
Breakpoint 3 at 0x154b: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.cp/ovldbreak.cc, line 110.
warning: Multiple breakpoints were set.
Use the "delete" command to delete unwanted breakpoints.
... which would trigger the assert.
This commit makes gdb.cp/ovldbreak.exp test this scenario. It does
that by making set_bp_overloaded take a list of expected created
breakpoints rather than just one breakpoint. It converts the
procedure to use gdb_test_multiple instead of send_gdb/gdb_expect
along the way.
Change-Id: Id87d1e08feb6670440d926f5344e5081f5e37c8e
---
gdb/testsuite/gdb.cp/ovldbreak.exp | 100 +++++++++++++++++++++--------
1 file changed, 75 insertions(+), 25 deletions(-)
diff --git a/gdb/testsuite/gdb.cp/ovldbreak.exp b/gdb/testsuite/gdb.cp/ovldbreak.exp
index 681edf204d3..06adf82ecbb 100644
--- a/gdb/testsuite/gdb.cp/ovldbreak.exp
+++ b/gdb/testsuite/gdb.cp/ovldbreak.exp
@@ -67,43 +67,75 @@ proc take_gdb_out_of_choice_menu {} {
-# This procedure sets an overloaded breakpoint.
-# When I ask for such a breakpoint, gdb gives me a menu of 'cancel' 'all'
-# and a bunch of choices. I then choose from that menu by number.
+# This procedure sets an overloaded breakpoint. When users ask for
+# such a breakpoint, gdb gives a menu of 'cancel' 'all' and one choice
+# per overload. Users can then choose from that menu by number.
+#
+# NAME is the spec to use to create the breakpoint. EXPECTEDMENU is
+# the expected menu. MYCHOICE is the choice selected. Can be more
+# than one overload, e.g. "2-3". BPNUMBER is the expected next
+# breakpoint created. LINENUMBERS is a list of line numbers, one
+# element per expected breakpoint created.
-proc set_bp_overloaded {name expectedmenu mychoice bpnumber linenumber} {
- global gdb_prompt hex srcfile
+proc set_bp_overloaded {name expectedmenu mychoice bpnumber linenumbers} {
+ global gdb_prompt hex decimal srcfile
# Get into the overload menu.
- send_gdb "break $name\n"
- gdb_expect {
- -re "$expectedmenu" {
- pass "bp menu for $name choice $mychoice"
-
- # Choose my choice.
- send_gdb "$mychoice\n"
- gdb_expect {
- -re "Breakpoint $bpnumber at $hex: file.*$srcfile, line $linenumber.\r\n$gdb_prompt $" {
- pass "set bp $bpnumber on $name $mychoice line $linenumber"
- }
- -re ".*$gdb_prompt $" {
- fail "set bp $bpnumber on $name $mychoice line $linenumber (bad bp)"
+ gdb_test_multiple "break $name" "bp menu for $name choice $mychoice" {
+ -re "$expectedmenu" {
+ pass $gdb_test_name
+
+ set any "\[^\r\n\]*"
+
+ # True if we've seen a bad breakpoint.
+ set bad_bp 0
+
+ # How many breakpoints we expect to see.
+ set expected_bps [llength $linenumbers]
+
+ # The count of seen breakpoints.
+ set seen_bps 0
+
+ # Choose my choice.
+ gdb_test_multiple "$mychoice" "set bp $bpnumber on $name $mychoice line $linenumbers" {
+ -re "Breakpoint ($decimal) at $hex: file$any$srcfile, line ($decimal).\r\n" {
+
+ set got_num $expect_out(1,string)
+ set got_line $expect_out(2,string)
+
+ if {$seen_bps >= $expected_bps} {
+ set bad_bp 1
+ } else {
+ set linenumber [lindex $linenumbers $seen_bps]
+
+ if {$got_num != $bpnumber || $got_line != $linenumber} {
+ set bad_bp 1
+ }
+
+ incr bpnumber
+ incr seen_bps
+ }
+ exp_continue
+ }
+ -re "$gdb_prompt $" {
+ gdb_assert {!$bad_bp && $seen_bps == $expected_bps} \
+ $gdb_test_name
}
timeout {
- fail "set bp $bpnumber on $name $mychoice line $linenumber (timeout)"
+ fail "$gdb_test_name (timeout)"
take_gdb_out_of_choice_menu
}
}
}
-re ".*\r\n> " {
- fail "bp menu for $name choice $mychoice (bad menu)"
+ fail "$gdb_test_name (bad menu)"
take_gdb_out_of_choice_menu
}
-re ".*$gdb_prompt $" {
- fail "bp menu for $name choice $mychoice (no menu)"
+ fail "$gdb_test_name (no menu)"
}
timeout {
- fail "bp menu for $name choice $mychoice (timeout)"
+ fail "$gdb_test_name (timeout)"
take_gdb_out_of_choice_menu
}
}
@@ -198,8 +230,10 @@ append menu_overload1arg {[\r\n]*> $}
# of the multiple-choice menu when breaking on an overloaded method.
gdb_test_no_output "set multiple-symbols ask"
-# Set breakpoints on foo::overload1arg, one by one.
+# The last breakpoint created.
set bpnum 1
+
+# Set breakpoints on foo::overload1arg, one by one.
set method "foo::overload1arg"
for {set idx 0} {$idx < [llength $overloads]} {incr idx} {
set type [lindex $overloads $idx]
@@ -257,6 +291,21 @@ gdb_expect {
gdb_test "info break" $bptable "breakpoint info (after cancel)"
+# Test that if the user selects multiple entries from the option list,
+# GDB creates one breakpoint per entry.
+with_test_prefix "multiple breakpoints" {
+ set method "foo::overload1arg"
+
+ set expected_lines {}
+ for {set i 0} {$i < 2} {incr i} {
+ set type [lindex $overloads $i]
+ lappend expected_lines $line($type_map("$type"))
+ }
+ set_bp_overloaded $method $menu_overload1arg \
+ "2-3" [incr bpnum] $expected_lines
+ incr bpnum
+}
+
# Delete these breakpoints.
send_gdb "delete breakpoints\n"
@@ -284,6 +333,7 @@ gdb_test "info breakpoints" "No breakpoints or watchpoints." "breakpoint info (a
# Test choice "all".
# This is copy-and-paste from set_bp_overloaded.
+incr bpnum
send_gdb "break foo::overload1arg\n"
gdb_expect {
-re "$menu_overload1arg" {
@@ -291,7 +341,7 @@ gdb_expect {
# Choose all.
send_gdb "1\n"
gdb_expect {
- -re "Breakpoint $decimal at $hex: foo::overload1arg. .12 locations.\r\n.*$gdb_prompt $" {
+ -re "Breakpoint $bpnum at $hex: foo::overload1arg. .12 locations.\r\n.*$gdb_prompt $" {
pass "set bp on overload1arg all"
}
-re ".*$gdb_prompt $" {
@@ -380,7 +430,7 @@ array set might_fail {
}
foreach type $all_types {
- continue_to_bp_overloaded 14 $might_fail($type) $line($type) \
+ continue_to_bp_overloaded $bpnum $might_fail($type) $line($type) \
$type $arguments($type)
}
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 23/23] Rename base_breakpoint -> code_breakpoint
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (21 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 22/23] Test "set multiple-symbols on" creating multiple breakpoints Pedro Alves
@ 2022-05-16 18:40 ` Pedro Alves
2022-05-20 16:05 ` Tom Tromey
2022-05-20 16:06 ` [PATCH 00/23] More breakpoints cleanups Tom Tromey
23 siblings, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2022-05-16 18:40 UTC (permalink / raw)
To: gdb-patches
Even after the previous patches reworking the inheritance of several
breakpoint types, the present breakpoint hierarchy looks a bit
surprising, as we have "breakpoint" as the superclass, and then
"base_breakpoint" inherits from "breakpoint". Like so, simplified:
breakpoint
base_breakpoint
ordinary_breakpoint
internal_breakpoint
momentary_breakpoint
ada_catchpoint
exception_catchpoint
tracepoint
watchpoint
catchpoint
exec_catchpoint
...
The surprising part to me is having "base_breakpoint" being a subclass
of "breakpoint". I'm just refering to naming here -- I mean, you'd
expect that it would be the top level baseclass that would be called
"base".
Just flipping the names of breakpoint and base_breakpoint around
wouldn't be super great for us, IMO, given we think of every type of
*point as a breakpoint at the user visible level. E.g., "info
breakpoints" shows watchpoints, tracepoints, etc. So it makes to call
the top level class breakpoint.
Instead, I propose renaming base_breakpoint to code_breakpoint. The
previous patches made sure that all code breakpoints inherit from
base_breakpoint, so it's fitting. Also, "code breakpoint" contrasts
nicely with a watchpoint also being typically known as a "data
breakpoint".
After this commit, the resulting hierarchy looks like:
breakpoint
code_breakpoint
ordinary_breakpoint
internal_breakpoint
momentary_breakpoint
ada_catchpoint
exception_catchpoint
tracepoint
watchpoint
catchpoint
exec_catchpoint
...
... which makes a lot more sense to me.
I've left this patch as last in the series in case people want to
bikeshed on the naming.
"code" has a nice property that it's exactly as many letters as
"base", so this patch didn't require any reindentation. :-)
Change-Id: Id8dc06683a69fad80d88e674f65e826d6a4e3f66
---
gdb/ada-lang.c | 8 ++---
gdb/break-catch-throw.c | 6 ++--
gdb/breakpoint.c | 62 ++++++++++++++++----------------
gdb/breakpoint.h | 16 ++++-----
gdb/elfread.c | 6 ++--
gdb/mi/mi-cmd-break.c | 4 +--
gdb/minsyms.c | 4 +--
gdb/python/py-finishbreakpoint.c | 2 +-
gdb/symtab.h | 6 ++--
9 files changed, 57 insertions(+), 57 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 5ddca104dc3..7e4988be5d0 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12104,7 +12104,7 @@ static std::string ada_exception_catchpoint_cond_string
/* An instance of this type is used to represent an Ada catchpoint. */
-struct ada_catchpoint : public base_breakpoint
+struct ada_catchpoint : public code_breakpoint
{
ada_catchpoint (struct gdbarch *gdbarch_,
enum ada_exception_catchpoint_kind kind,
@@ -12113,12 +12113,12 @@ struct ada_catchpoint : public base_breakpoint
bool tempflag,
bool enabled,
bool from_tty)
- : base_breakpoint (gdbarch_, bp_catchpoint),
+ : code_breakpoint (gdbarch_, bp_catchpoint),
m_kind (kind)
{
add_location (sal);
- /* Unlike most base_breakpoint types, Ada catchpoints are
+ /* Unlike most code_breakpoint types, Ada catchpoints are
pspace-specific. */
gdb_assert (sal.pspace != nullptr);
this->pspace = sal.pspace;
@@ -12247,7 +12247,7 @@ ada_catchpoint::re_set ()
{
/* Call the base class's method. This updates the catchpoint's
locations. */
- this->base_breakpoint::re_set ();
+ this->code_breakpoint::re_set ();
/* Reparse the exception conditional expressions. One for each
location. */
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index a6f477b712a..66cf80be1c5 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -65,15 +65,15 @@ static const struct exception_names exception_functions[] =
/* The type of an exception catchpoint. Unlike most catchpoints, this
one is implemented with code breakpoints, so it inherits struct
- base_breakpoint, not struct catchpoint. */
+ code_breakpoint, not struct catchpoint. */
-struct exception_catchpoint : public base_breakpoint
+struct exception_catchpoint : public code_breakpoint
{
exception_catchpoint (struct gdbarch *gdbarch,
bool temp, const char *cond_string_,
enum exception_event_kind kind_,
std::string &&except_rx)
- : base_breakpoint (gdbarch, bp_catchpoint, temp, cond_string_),
+ : code_breakpoint (gdbarch, bp_catchpoint, temp, cond_string_),
kind (kind_),
exception_rx (std::move (except_rx)),
pattern (exception_rx.empty ()
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f4642c2d8e7..ce425568dd5 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -87,7 +87,7 @@
static void map_breakpoint_numbers (const char *,
gdb::function_view<void (breakpoint *)>);
-static void breakpoint_re_set_default (base_breakpoint *);
+static void breakpoint_re_set_default (code_breakpoint *);
static void
create_sals_from_location_default (struct event_location *location,
@@ -225,7 +225,7 @@ static void tracepoint_probe_create_sals_from_location
(struct event_location *location,
struct linespec_result *canonical);
-const struct breakpoint_ops base_breakpoint_ops =
+const struct breakpoint_ops code_breakpoint_ops =
{
create_sals_from_location_default,
create_breakpoints_sal,
@@ -252,7 +252,7 @@ breakpoint::~breakpoint ()
{
}
-base_breakpoint::~base_breakpoint ()
+code_breakpoint::~code_breakpoint ()
{
}
@@ -261,9 +261,9 @@ catchpoint::~catchpoint ()
}
/* The structure to be used in regular breakpoints. */
-struct ordinary_breakpoint : public base_breakpoint
+struct ordinary_breakpoint : public code_breakpoint
{
- using base_breakpoint::base_breakpoint;
+ using code_breakpoint::code_breakpoint;
int resources_needed (const struct bp_location *) override;
enum print_stop_action print_it (const bpstat *bs) const override;
@@ -275,11 +275,11 @@ struct ordinary_breakpoint : public base_breakpoint
the program, and they end up installed on the breakpoint chain with
a negative breakpoint number. They're visible in "maint info
breakpoints", but not "info breakpoints". */
-struct internal_breakpoint : public base_breakpoint
+struct internal_breakpoint : public code_breakpoint
{
internal_breakpoint (struct gdbarch *gdbarch,
enum bptype type, CORE_ADDR address)
- : base_breakpoint (gdbarch, type)
+ : code_breakpoint (gdbarch, type)
{
symtab_and_line sal;
sal.pc = address;
@@ -303,13 +303,13 @@ struct internal_breakpoint : public base_breakpoint
on the breakpoint chain and they all same the same number (zero).
They're visible in "maint info breakpoints", but not "info
breakpoints". */
-struct momentary_breakpoint : public base_breakpoint
+struct momentary_breakpoint : public code_breakpoint
{
momentary_breakpoint (struct gdbarch *gdbarch_, enum bptype bptype,
program_space *pspace_,
const struct frame_id &frame_id_,
int thread_)
- : base_breakpoint (gdbarch_, bptype)
+ : code_breakpoint (gdbarch_, bptype)
{
/* If FRAME_ID is valid, it should be a real frame, not an inlined
or tail-called one. */
@@ -1290,11 +1290,11 @@ is_tracepoint (const struct breakpoint *b)
TYPE. */
template<typename... Arg>
-static std::unique_ptr<base_breakpoint>
+static std::unique_ptr<code_breakpoint>
new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type,
Arg&&... args)
{
- base_breakpoint *b;
+ code_breakpoint *b;
switch (type)
{
@@ -1325,7 +1325,7 @@ new_breakpoint_from_type (struct gdbarch *gdbarch, bptype type,
gdb_assert_not_reached ("invalid type");
}
- return std::unique_ptr<base_breakpoint> (b);
+ return std::unique_ptr<code_breakpoint> (b);
}
/* A helper function that validates that COMMANDS are valid for a
@@ -5886,10 +5886,10 @@ bpstat_run_callbacks (bpstat *bs_head)
handle_jit_event (bs->bp_location_at->address);
break;
case bp_gnu_ifunc_resolver:
- gnu_ifunc_resolver_stop ((base_breakpoint *) b);
+ gnu_ifunc_resolver_stop ((code_breakpoint *) b);
break;
case bp_gnu_ifunc_resolver_return:
- gnu_ifunc_resolver_return_stop ((base_breakpoint *) b);
+ gnu_ifunc_resolver_return_stop ((code_breakpoint *) b);
break;
}
}
@@ -8057,7 +8057,7 @@ handle_automatic_hardware_breakpoints (bp_location *bl)
}
bp_location *
-base_breakpoint::add_location (const symtab_and_line &sal)
+code_breakpoint::add_location (const symtab_and_line &sal)
{
struct bp_location *new_loc, **tmp;
CORE_ADDR adjusted_address;
@@ -8215,7 +8215,7 @@ update_dprintf_commands (const char *args, int from_tty,
update_dprintf_command_list (b);
}
-base_breakpoint::base_breakpoint (struct gdbarch *gdbarch_,
+code_breakpoint::code_breakpoint (struct gdbarch *gdbarch_,
enum bptype type_,
gdb::array_view<const symtab_and_line> sals,
event_location_up &&location_,
@@ -8359,7 +8359,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
int enabled, int internal, unsigned flags,
int display_canonical)
{
- std::unique_ptr<base_breakpoint> b
+ std::unique_ptr<code_breakpoint> b
= new_breakpoint_from_type (gdbarch,
type,
sals,
@@ -8736,14 +8736,14 @@ breakpoint_ops_for_event_location_type (enum event_location_type location_type,
if (location_type == PROBE_LOCATION)
return &tracepoint_probe_breakpoint_ops;
else
- return &base_breakpoint_ops;
+ return &code_breakpoint_ops;
}
else
{
if (location_type == PROBE_LOCATION)
return &bkpt_probe_breakpoint_ops;
else
- return &base_breakpoint_ops;
+ return &code_breakpoint_ops;
}
}
@@ -8756,7 +8756,7 @@ breakpoint_ops_for_event_location (const struct event_location *location,
if (location != nullptr)
return breakpoint_ops_for_event_location_type
(event_location_type (location), is_tracepoint);
- return &base_breakpoint_ops;
+ return &code_breakpoint_ops;
}
/* See breakpoint.h. */
@@ -9091,7 +9091,7 @@ dprintf_command (const char *arg, int from_tty)
0, bp_dprintf,
0 /* Ignore count */,
pending_break_support,
- &base_breakpoint_ops,
+ &code_breakpoint_ops,
from_tty,
1 /* enabled */,
0 /* internal */,
@@ -11465,7 +11465,7 @@ breakpoint::decode_location (struct event_location *location,
/* Default breakpoint_ops methods. */
void
-base_breakpoint::re_set ()
+code_breakpoint::re_set ()
{
/* FIXME: is this still reachable? */
if (breakpoint_event_location_empty_p (this))
@@ -11479,7 +11479,7 @@ base_breakpoint::re_set ()
}
int
-base_breakpoint::insert_location (struct bp_location *bl)
+code_breakpoint::insert_location (struct bp_location *bl)
{
CORE_ADDR addr = bl->target_info.reqstd_address;
@@ -11503,7 +11503,7 @@ base_breakpoint::insert_location (struct bp_location *bl)
}
int
-base_breakpoint::remove_location (struct bp_location *bl,
+code_breakpoint::remove_location (struct bp_location *bl,
enum remove_bp_reason reason)
{
if (bl->probe.prob != nullptr)
@@ -11519,7 +11519,7 @@ base_breakpoint::remove_location (struct bp_location *bl,
}
int
-base_breakpoint::breakpoint_hit (const struct bp_location *bl,
+code_breakpoint::breakpoint_hit (const struct bp_location *bl,
const address_space *aspace,
CORE_ADDR bp_addr,
const target_waitstatus &ws)
@@ -11655,7 +11655,7 @@ ordinary_breakpoint::print_recreate (struct ui_file *fp) const
}
std::vector<symtab_and_line>
-base_breakpoint::decode_location (struct event_location *location,
+code_breakpoint::decode_location (struct event_location *location,
struct program_space *search_pspace)
{
if (event_location_type (location) == PROBE_LOCATION)
@@ -12480,7 +12480,7 @@ hoist_existing_locations (struct breakpoint *b, struct program_space *pspace)
untouched. */
void
-update_breakpoint_locations (base_breakpoint *b,
+update_breakpoint_locations (code_breakpoint *b,
struct program_space *filter_pspace,
gdb::array_view<const symtab_and_line> sals,
gdb::array_view<const symtab_and_line> sals_end)
@@ -12688,7 +12688,7 @@ location_to_sals (struct breakpoint *b, struct event_location *location,
locations. */
static void
-breakpoint_re_set_default (base_breakpoint *b)
+breakpoint_re_set_default (code_breakpoint *b)
{
struct program_space *filter_pspace = current_program_space;
std::vector<symtab_and_line> expanded, expanded_end;
@@ -13519,7 +13519,7 @@ ftrace_command (const char *arg, int from_tty)
bp_fast_tracepoint /* type_wanted */,
0 /* Ignore count */,
pending_break_support,
- &base_breakpoint_ops,
+ &code_breakpoint_ops,
from_tty,
1 /* enabled */,
0 /* internal */, 0);
@@ -13544,7 +13544,7 @@ strace_command (const char *arg, int from_tty)
}
else
{
- ops = &base_breakpoint_ops;
+ ops = &code_breakpoint_ops;
location = string_to_event_location (&arg, current_language);
type = bp_static_tracepoint;
}
@@ -13627,7 +13627,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
utp->type /* type_wanted */,
0 /* Ignore count */,
pending_break_support,
- &base_breakpoint_ops,
+ &code_breakpoint_ops,
0 /* from_tty */,
utp->enabled /* enabled */,
0 /* internal */,
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 8735396e8d4..566f1285e46 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -849,7 +849,7 @@ struct breakpoint
/* Abstract base class representing code breakpoints. User "break"
breakpoints, internal and momentary breakpoints, etc. IOW, any
kind of breakpoint whose locations are created from SALs. */
-struct base_breakpoint : public breakpoint
+struct code_breakpoint : public breakpoint
{
using breakpoint::breakpoint;
@@ -857,7 +857,7 @@ struct base_breakpoint : public breakpoint
description of the location, and COND_STRING as condition
expression. If LOCATION is NULL then create an "address
location" from the address in the SAL. */
- base_breakpoint (struct gdbarch *gdbarch, bptype type,
+ code_breakpoint (struct gdbarch *gdbarch, bptype type,
gdb::array_view<const symtab_and_line> sals,
event_location_up &&location,
gdb::unique_xmalloc_ptr<char> filter,
@@ -869,7 +869,7 @@ struct base_breakpoint : public breakpoint
int enabled, unsigned flags,
int display_canonical);
- ~base_breakpoint () override = 0;
+ ~code_breakpoint () override = 0;
/* Add a location for SAL to this breakpoint. */
bp_location *add_location (const symtab_and_line &sal);
@@ -985,9 +985,9 @@ extern bool is_exception_catchpoint (breakpoint *bp);
/* An instance of this type is used to represent all kinds of
tracepoints. */
-struct tracepoint : public base_breakpoint
+struct tracepoint : public code_breakpoint
{
- using base_breakpoint::base_breakpoint;
+ using code_breakpoint::code_breakpoint;
int breakpoint_hit (const struct bp_location *bl,
const address_space *aspace, CORE_ADDR bp_addr,
@@ -1384,7 +1384,7 @@ extern void until_break_command (const char *, int, int);
/* Initialize a struct bp_location. */
extern void update_breakpoint_locations
- (base_breakpoint *b,
+ (code_breakpoint *b,
struct program_space *filter_pspace,
gdb::array_view<const symtab_and_line> sals,
gdb::array_view<const symtab_and_line> sals_end);
@@ -1434,7 +1434,7 @@ extern void awatch_command_wrapper (const char *, int, bool);
extern void rwatch_command_wrapper (const char *, int, bool);
extern void tbreak_command (const char *, int);
-extern const struct breakpoint_ops base_breakpoint_ops;
+extern const struct breakpoint_ops code_breakpoint_ops;
/* Arguments to pass as context to some catch command handlers. */
#define CATCH_PERMANENT ((void *) (uintptr_t) 0)
@@ -1463,7 +1463,7 @@ extern void install_breakpoint (int internal, std::unique_ptr<breakpoint> &&b,
/* Returns the breakpoint ops appropriate for use with with LOCATION and
according to IS_TRACEPOINT. Use this to ensure, for example, that you pass
the correct ops to create_breakpoint for probe locations. If LOCATION is
- NULL, returns base_breakpoint_ops. */
+ NULL, returns code_breakpoint_ops. */
extern const struct breakpoint_ops *breakpoint_ops_for_event_location
(const struct event_location *location, bool is_tracepoint);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 27203722802..32cb27c8967 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -927,7 +927,7 @@ elf_gnu_ifunc_resolve_addr (struct gdbarch *gdbarch, CORE_ADDR pc)
/* Handle inferior hit of bp_gnu_ifunc_resolver, see its definition. */
static void
-elf_gnu_ifunc_resolver_stop (base_breakpoint *b)
+elf_gnu_ifunc_resolver_stop (code_breakpoint *b)
{
struct breakpoint *b_return;
struct frame_info *prev_frame = get_prev_frame (get_current_frame ());
@@ -978,7 +978,7 @@ elf_gnu_ifunc_resolver_stop (base_breakpoint *b)
/* Handle inferior hit of bp_gnu_ifunc_resolver_return, see its definition. */
static void
-elf_gnu_ifunc_resolver_return_stop (base_breakpoint *b)
+elf_gnu_ifunc_resolver_return_stop (code_breakpoint *b)
{
thread_info *thread = inferior_thread ();
struct gdbarch *gdbarch = get_frame_arch (get_current_frame ());
@@ -1008,7 +1008,7 @@ elf_gnu_ifunc_resolver_return_stop (base_breakpoint *b)
"gnu-indirect-function breakpoint type %d"),
(int) b->type);
}
- b = (base_breakpoint *) b_next;
+ b = (code_breakpoint *) b_next;
}
gdb_assert (b->type == bp_gnu_ifunc_resolver);
gdb_assert (b->loc->next == NULL);
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 1d9fc0de436..d4a7562b281 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -327,12 +327,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
else if (dprintf)
{
type_wanted = bp_dprintf;
- ops = &base_breakpoint_ops;
+ ops = &code_breakpoint_ops;
}
else
{
type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
- ops = &base_breakpoint_ops;
+ ops = &code_breakpoint_ops;
}
if (is_explicit)
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 217ee047446..4ec62558b69 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1019,7 +1019,7 @@ stub_gnu_ifunc_resolve_name (const char *function_name,
/* See elf_gnu_ifunc_resolver_stop for its real implementation. */
static void
-stub_gnu_ifunc_resolver_stop (base_breakpoint *b)
+stub_gnu_ifunc_resolver_stop (code_breakpoint *b)
{
internal_error (__FILE__, __LINE__,
_("elf_gnu_ifunc_resolver_stop cannot be reached."));
@@ -1028,7 +1028,7 @@ stub_gnu_ifunc_resolver_stop (base_breakpoint *b)
/* See elf_gnu_ifunc_resolver_return_stop for its real implementation. */
static void
-stub_gnu_ifunc_resolver_return_stop (base_breakpoint *b)
+stub_gnu_ifunc_resolver_return_stop (code_breakpoint *b)
{
internal_error (__FILE__, __LINE__,
_("elf_gnu_ifunc_resolver_return_stop cannot be reached."));
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index abd5b48a949..df54d3643ab 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -305,7 +305,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
bp_breakpoint,
0,
AUTO_BOOLEAN_TRUE,
- &base_breakpoint_ops,
+ &code_breakpoint_ops,
0, 1, internal_bp, 0);
}
catch (const gdb_exception &except)
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 5218be587de..ac902a4cbbe 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -55,7 +55,7 @@ struct obj_section;
struct cmd_list_element;
class probe;
struct lookup_name_info;
-struct base_breakpoint;
+struct code_breakpoint;
/* How to match a lookup name against a symbol search name. */
enum class symbol_name_match_type
@@ -2228,10 +2228,10 @@ struct gnu_ifunc_fns
CORE_ADDR *function_address_p);
/* See elf_gnu_ifunc_resolver_stop for its real implementation. */
- void (*gnu_ifunc_resolver_stop) (base_breakpoint *b);
+ void (*gnu_ifunc_resolver_stop) (code_breakpoint *b);
/* See elf_gnu_ifunc_resolver_return_stop for its real implementation. */
- void (*gnu_ifunc_resolver_return_stop) (base_breakpoint *b);
+ void (*gnu_ifunc_resolver_return_stop) (code_breakpoint *b);
};
#define gnu_ifunc_resolve_addr gnu_ifunc_fns_p->gnu_ifunc_resolve_addr
--
2.36.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 23/23] Rename base_breakpoint -> code_breakpoint
2022-05-16 18:40 ` [PATCH 23/23] Rename base_breakpoint -> code_breakpoint Pedro Alves
@ 2022-05-20 16:05 ` Tom Tromey
0 siblings, 0 replies; 31+ messages in thread
From: Tom Tromey @ 2022-05-20 16:05 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> Instead, I propose renaming base_breakpoint to code_breakpoint. The
Pedro> previous patches made sure that all code breakpoints inherit from
Pedro> base_breakpoint, so it's fitting. Also, "code breakpoint" contrasts
Pedro> nicely with a watchpoint also being typically known as a "data
Pedro> breakpoint".
Seems reasonable to me.
Pedro> I've left this patch as last in the series in case people want to
Pedro> bikeshed on the naming.
I think this name is fine.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/23] More breakpoints cleanups
2022-05-16 18:40 [PATCH 00/23] More breakpoints cleanups Pedro Alves
` (22 preceding siblings ...)
2022-05-16 18:40 ` [PATCH 23/23] Rename base_breakpoint -> code_breakpoint Pedro Alves
@ 2022-05-20 16:06 ` Tom Tromey
2022-05-20 19:43 ` Pedro Alves
23 siblings, 1 reply; 31+ messages in thread
From: Tom Tromey @ 2022-05-20 16:06 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
Pedro> The main goal of this series was to eliminate init_raw_breakpoint.
Pedro> It slowly tweaks a number of things to be able to do that.
I looked through these again today, and sent one minor comment.
This all looks good to me, thank you for doing it.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/23] More breakpoints cleanups
2022-05-20 16:06 ` [PATCH 00/23] More breakpoints cleanups Tom Tromey
@ 2022-05-20 19:43 ` Pedro Alves
0 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2022-05-20 19:43 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2022-05-20 17:06, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>
> Pedro> The main goal of this series was to eliminate init_raw_breakpoint.
> Pedro> It slowly tweaks a number of things to be able to do that.
>
> I looked through these again today, and sent one minor comment.
> This all looks good to me, thank you for doing it.
Thanks Tom. I've merged this after addressing comments and retesting.
^ permalink raw reply [flat|nested] 31+ messages in thread