* [PATCH 0/4] Const args, int->bool, and scoped_restore in breakpoint.c
@ 2022-10-05 9:32 Andrew Burgess
2022-10-05 9:32 ` [PATCH 1/4] gdb: make some bp_location arguments const " Andrew Burgess
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-10-05 9:32 UTC (permalink / raw)
To: gdb-patches
A series of patches that does some general code polishing in
breakpoint.c. I don't think there should be any user visible changes
after any of these patches.
---
Andrew Burgess (4):
gdb: make some bp_location arguments const in breakpoint.c
gdb: used scoped_restore_frame in update_watchpoint
gdb: make use of scoped_restore in unduplicated_should_be_inserted
gdb: some int to bool conversion in breakpoint.c
gdb/breakpoint.c | 321 ++++++++++++++++++++++-------------------------
1 file changed, 153 insertions(+), 168 deletions(-)
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/4] gdb: make some bp_location arguments const in breakpoint.c 2022-10-05 9:32 [PATCH 0/4] Const args, int->bool, and scoped_restore in breakpoint.c Andrew Burgess @ 2022-10-05 9:32 ` Andrew Burgess 2022-10-18 13:46 ` Bruno Larsen 2022-10-05 9:32 ` [PATCH 2/4] gdb: used scoped_restore_frame in update_watchpoint Andrew Burgess ` (3 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2022-10-05 9:32 UTC (permalink / raw) To: gdb-patches I spotted a few places where I could make some 'bp_location *' arguments constant in breakpoint.c. There should be no user visible changes after this commit. --- gdb/breakpoint.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 002f4a935b1..6e1a15e4f1b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -125,11 +125,11 @@ static CORE_ADDR adjust_breakpoint_address (struct gdbarch *gdbarch, enum bptype bptype, struct program_space *pspace); -static int watchpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2); +static int watchpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2); -static int breakpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2, +static int breakpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2, bool sw_hw_bps_match = false); static int breakpoint_location_address_match (struct bp_location *bl, @@ -2643,7 +2643,7 @@ build_target_command_list (struct bp_location *bl) registers state. */ static int -breakpoint_kind (struct bp_location *bl, CORE_ADDR *addr) +breakpoint_kind (const struct bp_location *bl, CORE_ADDR *addr) { if (bl->owner->type == bp_single_step) { @@ -4229,7 +4229,7 @@ moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) ASPACE. */ static int -bp_location_inserted_here_p (struct bp_location *bl, +bp_location_inserted_here_p (const struct bp_location *bl, const address_space *aspace, CORE_ADDR pc) { if (bl->inserted @@ -5440,7 +5440,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) on the current target. */ static int -need_moribund_for_location_type (struct bp_location *loc) +need_moribund_for_location_type (const struct bp_location *loc) { return ((loc->loc_type == bp_loc_software_breakpoint && !target_supports_stopped_by_sw_breakpoint ()) @@ -5974,7 +5974,7 @@ bp_condition_evaluator (const breakpoint *b) similar to bp_condition_evaluator, but for locations. */ static const char * -bp_location_condition_evaluator (struct bp_location *bl) +bp_location_condition_evaluator (const struct bp_location *bl) { if (bl && !is_breakpoint (bl->owner)) return NULL; @@ -6933,8 +6933,8 @@ bl_address_is_meaningful (bp_location *loc) true if LOC1 and LOC2 represent the same watchpoint location. */ static int -watchpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2) +watchpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2) { struct watchpoint *w1 = (struct watchpoint *) loc1->owner; struct watchpoint *w2 = (struct watchpoint *) loc2->owner; @@ -7045,8 +7045,8 @@ breakpoint_location_address_range_overlap (struct bp_location *bl, true, otherwise returns false. */ static int -tracepoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2) +tracepoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2) { if (is_tracepoint (loc1->owner) && is_tracepoint (loc2->owner)) /* Since tracepoint locations are never duplicated with others', tracepoint @@ -7064,8 +7064,8 @@ tracepoint_locations_match (struct bp_location *loc1, otherwise they don't. */ static int -breakpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2, +breakpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2, bool sw_hw_bps_match) { int hw_point1, hw_point2; @@ -13157,7 +13157,7 @@ enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable) owner. 1-based indexing. -1 signals NOT FOUND. */ static int -find_loc_num_by_location (bp_location *loc) +find_loc_num_by_location (const bp_location *loc) { if (loc != nullptr && loc->owner != nullptr) { -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] gdb: make some bp_location arguments const in breakpoint.c 2022-10-05 9:32 ` [PATCH 1/4] gdb: make some bp_location arguments const " Andrew Burgess @ 2022-10-18 13:46 ` Bruno Larsen 0 siblings, 0 replies; 13+ messages in thread From: Bruno Larsen @ 2022-10-18 13:46 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 05/10/2022 11:32, Andrew Burgess via Gdb-patches wrote: > I spotted a few places where I could make some 'bp_location *' > arguments constant in breakpoint.c. > > There should be no user visible changes after this commit. Hi Andrew, This patch no longer applies, because of commit d8a77e4c8043c645c3c7a07a042c112053715e95, which already made the change to find_loc_num_by_location. Other than this, this patch looks ok to me. Reviewed-By: Bruno Larsen <blarsen@redhat.com> Cheers, Bruno > --- > gdb/breakpoint.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 002f4a935b1..6e1a15e4f1b 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -125,11 +125,11 @@ static CORE_ADDR adjust_breakpoint_address (struct gdbarch *gdbarch, > enum bptype bptype, > struct program_space *pspace); > > -static int watchpoint_locations_match (struct bp_location *loc1, > - struct bp_location *loc2); > +static int watchpoint_locations_match (const struct bp_location *loc1, > + const struct bp_location *loc2); > > -static int breakpoint_locations_match (struct bp_location *loc1, > - struct bp_location *loc2, > +static int breakpoint_locations_match (const struct bp_location *loc1, > + const struct bp_location *loc2, > bool sw_hw_bps_match = false); > > static int breakpoint_location_address_match (struct bp_location *bl, > @@ -2643,7 +2643,7 @@ build_target_command_list (struct bp_location *bl) > registers state. */ > > static int > -breakpoint_kind (struct bp_location *bl, CORE_ADDR *addr) > +breakpoint_kind (const struct bp_location *bl, CORE_ADDR *addr) > { > if (bl->owner->type == bp_single_step) > { > @@ -4229,7 +4229,7 @@ moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) > ASPACE. */ > > static int > -bp_location_inserted_here_p (struct bp_location *bl, > +bp_location_inserted_here_p (const struct bp_location *bl, > const address_space *aspace, CORE_ADDR pc) > { > if (bl->inserted > @@ -5440,7 +5440,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) > on the current target. */ > > static int > -need_moribund_for_location_type (struct bp_location *loc) > +need_moribund_for_location_type (const struct bp_location *loc) > { > return ((loc->loc_type == bp_loc_software_breakpoint > && !target_supports_stopped_by_sw_breakpoint ()) > @@ -5974,7 +5974,7 @@ bp_condition_evaluator (const breakpoint *b) > similar to bp_condition_evaluator, but for locations. */ > > static const char * > -bp_location_condition_evaluator (struct bp_location *bl) > +bp_location_condition_evaluator (const struct bp_location *bl) > { > if (bl && !is_breakpoint (bl->owner)) > return NULL; > @@ -6933,8 +6933,8 @@ bl_address_is_meaningful (bp_location *loc) > true if LOC1 and LOC2 represent the same watchpoint location. */ > > static int > -watchpoint_locations_match (struct bp_location *loc1, > - struct bp_location *loc2) > +watchpoint_locations_match (const struct bp_location *loc1, > + const struct bp_location *loc2) > { > struct watchpoint *w1 = (struct watchpoint *) loc1->owner; > struct watchpoint *w2 = (struct watchpoint *) loc2->owner; > @@ -7045,8 +7045,8 @@ breakpoint_location_address_range_overlap (struct bp_location *bl, > true, otherwise returns false. */ > > static int > -tracepoint_locations_match (struct bp_location *loc1, > - struct bp_location *loc2) > +tracepoint_locations_match (const struct bp_location *loc1, > + const struct bp_location *loc2) > { > if (is_tracepoint (loc1->owner) && is_tracepoint (loc2->owner)) > /* Since tracepoint locations are never duplicated with others', tracepoint > @@ -7064,8 +7064,8 @@ tracepoint_locations_match (struct bp_location *loc1, > otherwise they don't. */ > > static int > -breakpoint_locations_match (struct bp_location *loc1, > - struct bp_location *loc2, > +breakpoint_locations_match (const struct bp_location *loc1, > + const struct bp_location *loc2, > bool sw_hw_bps_match) > { > int hw_point1, hw_point2; > @@ -13157,7 +13157,7 @@ enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable) > owner. 1-based indexing. -1 signals NOT FOUND. */ > > static int > -find_loc_num_by_location (bp_location *loc) > +find_loc_num_by_location (const bp_location *loc) > { > if (loc != nullptr && loc->owner != nullptr) > { ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] gdb: used scoped_restore_frame in update_watchpoint 2022-10-05 9:32 [PATCH 0/4] Const args, int->bool, and scoped_restore in breakpoint.c Andrew Burgess 2022-10-05 9:32 ` [PATCH 1/4] gdb: make some bp_location arguments const " Andrew Burgess @ 2022-10-05 9:32 ` Andrew Burgess 2022-10-05 9:32 ` [PATCH 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted Andrew Burgess ` (2 subsequent siblings) 4 siblings, 0 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-05 9:32 UTC (permalink / raw) To: gdb-patches I was doing some int to bool cleanup in update_watchpoint, and I noticed a manual version of scoped_restore_selected_frame. As always when these things are done manually, there is the chance that, in an error case, we might leave the wrong frame selected. This commit updates things to use scoped_restore_selected_frame, and also converts a local variable from int to bool. The only user visible change after this commit is in the case where update_watchpoint throws an error - we should now correctly restore the previously selected frame. Otherwise, this commit should be invisible to the user. --- gdb/breakpoint.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 6e1a15e4f1b..57281a199b0 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1926,9 +1926,7 @@ add_dummy_location (struct breakpoint *b, static void update_watchpoint (struct watchpoint *b, int reparse) { - int within_current_scope; - struct frame_id saved_frame_id; - int frame_saved; + bool within_current_scope; /* If this is a local watchpoint, we only want to check if the watchpoint frame is in scope if the current thread is the thread @@ -1938,12 +1936,12 @@ update_watchpoint (struct watchpoint *b, int reparse) if (b->disposition == disp_del_at_next_stop) return; - - frame_saved = 0; + + gdb::optional<scoped_restore_selected_frame> restore_frame; /* Determine if the watchpoint is within scope. */ if (b->exp_valid_block == NULL) - within_current_scope = 1; + within_current_scope = true; else { struct frame_info *fi = get_current_frame (); @@ -1962,8 +1960,7 @@ update_watchpoint (struct watchpoint *b, int reparse) /* FIXME drow/2003-09-09: It would be nice if evaluate_expression took a frame parameter, so that we didn't have to change the selected frame. */ - frame_saved = 1; - saved_frame_id = get_frame_id (get_selected_frame (NULL)); + restore_frame.emplace (); fi = frame_find_by_id (b->watchpoint_frame); within_current_scope = (fi != NULL); @@ -2229,10 +2226,6 @@ in which its expression is valid.\n"), b->number); watchpoint_del_at_next_stop (b); } - - /* Restore the selected frame. */ - if (frame_saved) - select_frame (frame_find_by_id (saved_frame_id)); } -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted 2022-10-05 9:32 [PATCH 0/4] Const args, int->bool, and scoped_restore in breakpoint.c Andrew Burgess 2022-10-05 9:32 ` [PATCH 1/4] gdb: make some bp_location arguments const " Andrew Burgess 2022-10-05 9:32 ` [PATCH 2/4] gdb: used scoped_restore_frame in update_watchpoint Andrew Burgess @ 2022-10-05 9:32 ` Andrew Burgess 2022-10-05 9:32 ` [PATCH 4/4] gdb: some int to bool conversion in breakpoint.c Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 0/4] Const args, int->bool, and scoped_restore " Andrew Burgess 4 siblings, 0 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-05 9:32 UTC (permalink / raw) To: gdb-patches I noticed that we could make use of a scoped_restore in the function unduplicated_should_be_inserted. I've also converted the function return type from int to bool. This change shouldn't make any difference, as I don't think anything within should_be_inserted could throw an exception, but the change doesn't hurt, and will help keep us safe if anything ever changes in the future. --- gdb/breakpoint.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 57281a199b0..e3f3ba3efa6 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2301,16 +2301,13 @@ should_be_inserted (struct bp_location *bl) /* Same as should_be_inserted but does the check assuming that the location is not duplicated. */ -static int +static bool unduplicated_should_be_inserted (struct bp_location *bl) { - int result; - const int save_duplicate = bl->duplicate; + scoped_restore restore_bl_duplicate + = make_scoped_restore (&bl->duplicate, 0); - bl->duplicate = 0; - result = should_be_inserted (bl); - bl->duplicate = save_duplicate; - return result; + return should_be_inserted (bl); } /* Parses a conditional described by an expression COND into an -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] gdb: some int to bool conversion in breakpoint.c 2022-10-05 9:32 [PATCH 0/4] Const args, int->bool, and scoped_restore in breakpoint.c Andrew Burgess ` (2 preceding siblings ...) 2022-10-05 9:32 ` [PATCH 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted Andrew Burgess @ 2022-10-05 9:32 ` Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 0/4] Const args, int->bool, and scoped_restore " Andrew Burgess 4 siblings, 0 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-05 9:32 UTC (permalink / raw) To: gdb-patches Some int to bool conversion in breakpoint.c. I've only updated the function signatures of static functions, but I've updated some function local variables throughout the file. The most interesting change is in unduplicated_should_be_inserted, where I've made use of make_scoped_restore to temporarily change the bl->duplicate flag. There should be no user visible changes after this commit. --- gdb/breakpoint.c | 271 +++++++++++++++++++++++------------------------ 1 file changed, 133 insertions(+), 138 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index e3f3ba3efa6..8e219221780 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -118,27 +118,27 @@ static struct breakpoint * enum bptype type, int loc_enabled, int thread); -static void breakpoint_adjustment_warning (CORE_ADDR, CORE_ADDR, int, int); +static void breakpoint_adjustment_warning (CORE_ADDR, CORE_ADDR, int, bool); static CORE_ADDR adjust_breakpoint_address (struct gdbarch *gdbarch, CORE_ADDR bpaddr, enum bptype bptype, struct program_space *pspace); -static int watchpoint_locations_match (const struct bp_location *loc1, - const struct bp_location *loc2); +static bool watchpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2); -static int breakpoint_locations_match (const struct bp_location *loc1, - const struct bp_location *loc2, - bool sw_hw_bps_match = false); +static bool breakpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2, + bool sw_hw_bps_match = false); -static int breakpoint_location_address_match (struct bp_location *bl, - const struct address_space *aspace, - CORE_ADDR addr); +static bool breakpoint_location_address_match (struct bp_location *bl, + const struct address_space *aspace, + CORE_ADDR addr); -static int breakpoint_location_address_range_overlap (struct bp_location *, - const address_space *, - CORE_ADDR, int); +static bool breakpoint_location_address_range_overlap (struct bp_location *, + const address_space *, + CORE_ADDR, int); static int remove_breakpoint (struct bp_location *); static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason); @@ -212,10 +212,10 @@ static void set_tracepoint_count (int num); static bool is_masked_watchpoint (const struct breakpoint *b); -/* Return 1 if B refers to a static tracepoint set by marker ("-m"), zero - otherwise. */ +/* Return true if B refers to a static tracepoint set by marker ("-m"), + zero otherwise. */ -static int strace_marker_p (struct breakpoint *b); +static bool strace_marker_p (struct breakpoint *b); static void bkpt_probe_create_sals_from_location_spec (location_spec *locspec, @@ -595,7 +595,7 @@ breakpoint_condition_evaluation_mode (void) /* Return true if GDB should evaluate breakpoint conditions or false otherwise. */ -static int +static bool gdb_evaluates_breakpoint_condition_p (void) { const char *mode = breakpoint_condition_evaluation_mode (); @@ -760,7 +760,7 @@ static struct cmd_list_element *breakpoint_show_cmdlist; struct cmd_list_element *save_cmdlist; /* Return whether a breakpoint is an active enabled breakpoint. */ -static int +static bool breakpoint_enabled (struct breakpoint *b) { return (b->enable_state == bp_enabled); @@ -1558,19 +1558,19 @@ commands_from_control_command (const char *arg, struct command_line *cmd) return simple_control; } -/* Return non-zero if BL->TARGET_INFO contains valid information. */ +/* Return true if BL->TARGET_INFO contains valid information. */ -static int +static bool bp_location_has_shadow (struct bp_location *bl) { if (bl->loc_type != bp_loc_software_breakpoint) - return 0; + return false; if (!bl->inserted) - return 0; + return false; if (bl->target_info.shadow_len == 0) /* BL isn't valid, or doesn't shadow memory. */ - return 0; - return 1; + return false; + return true; } /* Update BUF, which is LEN bytes read from the target address @@ -1808,7 +1808,7 @@ is_watchpoint (const struct breakpoint *bpt) and in any state. It is presently left to the target allowing memory accesses when threads are running. */ -static int +static bool watchpoint_in_thread_scope (struct watchpoint *b) { return (b->pspace == current_program_space @@ -1924,7 +1924,7 @@ add_dummy_location (struct breakpoint *b, watchpoint removal from inferior. */ static void -update_watchpoint (struct watchpoint *b, int reparse) +update_watchpoint (struct watchpoint *b, bool reparse) { bool within_current_scope; @@ -2228,27 +2228,27 @@ in which its expression is valid.\n"), } } - -/* Returns 1 iff breakpoint location should be +/* Returns true iff breakpoint location should be inserted in the inferior. We don't differentiate the type of BL's owner (breakpoint vs. tracepoint), although insert_location in tracepoint's breakpoint_ops is not defined, because in insert_bp_location, tracepoint's insert_location will not be called. */ -static int + +static bool should_be_inserted (struct bp_location *bl) { if (bl->owner == NULL || !breakpoint_enabled (bl->owner)) - return 0; + return false; if (bl->owner->disposition == disp_del_at_next_stop) - return 0; + return false; if (!bl->enabled || bl->disabled_by_cond || bl->shlib_disabled || bl->duplicate) - return 0; + return false; if (user_breakpoint_p (bl->owner) && bl->pspace->executing_startup) - return 0; + return false; /* This is set for example, when we're attached to the parent of a vfork, and have detached from the child. The child is running @@ -2259,7 +2259,7 @@ should_be_inserted (struct bp_location *bl) the child could still trip on the parent's breakpoints. Since the parent is blocked anyway, it won't miss any breakpoint. */ if (bl->pspace->breakpoints_not_allowed) - return 0; + return false; /* Don't insert a breakpoint if we're trying to step past its location, except if the breakpoint is a single-step breakpoint, @@ -2281,7 +2281,7 @@ should_be_inserted (struct bp_location *bl) { infrun_debug_printf ("skipping breakpoint: stepping past insn at: %s", paddress (bl->gdbarch, bl->address)); - return 0; + return false; } /* Don't insert watchpoints if we're trying to step past the @@ -2292,10 +2292,10 @@ should_be_inserted (struct bp_location *bl) infrun_debug_printf ("stepping past non-steppable watchpoint. " "skipping watchpoint at %s:%d", paddress (bl->gdbarch, bl->address), bl->length); - return 0; + return false; } - return 1; + return true; } /* Same as should_be_inserted but does the check assuming @@ -2350,7 +2350,7 @@ parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond) static void build_target_condition_list (struct bp_location *bl) { - int null_condition_or_parse_error = 0; + bool null_condition_or_parse_error = false; int modified = bl->needs_update; /* Release conditions left over from a previous insert. */ @@ -2393,7 +2393,7 @@ build_target_condition_list (struct bp_location *bl) went wrong or we have a null condition expression. */ if (!loc->cond_bytecode) { - null_condition_or_parse_error = 1; + null_condition_or_parse_error = true; break; } } @@ -2527,7 +2527,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) static void build_target_command_list (struct bp_location *bl) { - int null_command_or_parse_error = 0; + bool null_command_or_parse_error = false; int modified = bl->needs_update; /* Clear commands left over from a previous insert. */ @@ -2576,7 +2576,7 @@ build_target_command_list (struct bp_location *bl) went wrong or we have a null command expression. */ if (!loc->cmd_bytecode) { - null_command_or_parse_error = 1; + null_command_or_parse_error = true; break; } } @@ -3021,7 +3021,7 @@ insert_breakpoints (void) { struct watchpoint *w = (struct watchpoint *) bpt; - update_watchpoint (w, 0 /* don't reparse. */); + update_watchpoint (w, false /* don't reparse. */); } /* Updating watchpoints creates new locations, so update the global @@ -3142,7 +3142,7 @@ insert_breakpoint_locations (void) them, as half-inserted watchpoint is of limited use. */ for (breakpoint *bpt : all_breakpoints ()) { - int some_failed = 0; + bool some_failed = false; if (!is_hardware_watchpoint (bpt)) continue; @@ -3156,7 +3156,7 @@ insert_breakpoint_locations (void) for (bp_location *loc : bpt->locations ()) if (!loc->inserted && should_be_inserted (loc)) { - some_failed = 1; + some_failed = true; break; } @@ -3323,7 +3323,7 @@ static struct minimal_symbol msym_not_found; /* Returns TRUE if MSYM point to the "not found" sentinel. */ -static int +static bool msym_not_found_p (const struct minimal_symbol *msym) { return msym == &msym_not_found; @@ -3685,7 +3685,7 @@ create_exception_master_breakpoint (void) /* Does B have a location spec? */ -static int +static bool breakpoint_location_spec_empty_p (const struct breakpoint *b) { return (b->locspec != nullptr && b->locspec->empty_p ()); @@ -4144,7 +4144,7 @@ breakpoint_init_inferior (enum inf_context context) enum breakpoint_here breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) { - int any_breakpoint_here = 0; + bool any_breakpoint_here = false; for (bp_location *bl : all_bp_locations ()) { @@ -4164,7 +4164,7 @@ breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) else if (bl->permanent) return permanent_breakpoint_here; else - any_breakpoint_here = 1; + any_breakpoint_here = true; } } @@ -4218,7 +4218,7 @@ moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) /* Returns non-zero iff BL is inserted at PC, in address space ASPACE. */ -static int +static bool bp_location_inserted_here_p (const struct bp_location *bl, const address_space *aspace, CORE_ADDR pc) { @@ -4226,14 +4226,12 @@ bp_location_inserted_here_p (const struct bp_location *bl, && breakpoint_address_match (bl->pspace->aspace, bl->address, aspace, pc)) { - if (overlay_debugging - && section_is_overlay (bl->section) - && !section_is_mapped (bl->section)) - return 0; /* unmapped overlay -- can't be a match */ - else - return 1; + /* An unmapped overlay can't be a match. */ + return !(overlay_debugging + && section_is_overlay (bl->section) + && !section_is_mapped (bl->section)); } - return 0; + return false; } /* Returns non-zero iff there's a breakpoint inserted at PC. */ @@ -4496,10 +4494,10 @@ breakpoint_about_to_proceed (void) breakpoint_proceeded = 1; } -/* Return non-zero iff CMD as the first line of a command sequence is `silent' +/* Return true iff CMD as the first line of a command sequence is `silent' or its equivalent. */ -static int +static bool command_line_is_silent (struct command_line *cmd) { return cmd && (strcmp ("silent", cmd->line) == 0); @@ -4514,16 +4512,16 @@ command_line_is_silent (struct command_line *cmd) case, it is the caller's responsibility to recall it again with the bpstat of the current thread. */ -static int +static bool bpstat_do_actions_1 (bpstat **bsp) { bpstat *bs; - int again = 0; + bool again = false; /* Avoid endless recursion if a `source' command is contained in bs->commands. */ if (executing_breakpoint_commands) - return 0; + return false; scoped_restore save_executing = make_scoped_restore (&executing_breakpoint_commands, 1); @@ -4589,7 +4587,7 @@ bpstat_do_actions_1 (bpstat **bsp) command, and can easily blow up GDB stack. Instead, we return true, which will trigger the caller to recall us with the new stop_bpstat. */ - again = 1; + again = true; break; } } @@ -4964,7 +4962,7 @@ watchpoint_check (bpstat *bs) { struct watchpoint *b; struct frame_info *fr; - int within_current_scope; + bool within_current_scope; /* BS is built from an existing struct breakpoint. */ gdb_assert (bs->breakpoint_at != NULL); @@ -4977,7 +4975,7 @@ watchpoint_check (bpstat *bs) return WP_IGNORE; if (b->exp_valid_block == NULL) - within_current_scope = 1; + within_current_scope = true; else { struct frame_info *frame = get_current_frame (); @@ -5008,7 +5006,7 @@ watchpoint_check (bpstat *bs) function = get_frame_function (fr); if (function == NULL || !contained_in (b->exp_valid_block, function->value_block ())) - within_current_scope = 0; + within_current_scope = false; } if (within_current_scope) @@ -5104,7 +5102,7 @@ watchpoint_check (bpstat *bs) breakpoint location BL. This function does not check if we should stop, only if BL explains the stop. */ -static int +static bool bpstat_check_location (const struct bp_location *bl, const address_space *aspace, CORE_ADDR bp_addr, const target_waitstatus &ws) @@ -5133,24 +5131,24 @@ bpstat_check_watchpoint (bpstat *bs) gdb_assert (b != NULL); { - int must_check_value = 0; - + bool must_check_value = false; + if (b->type == bp_watchpoint) /* For a software watchpoint, we must always check the watched value. */ - must_check_value = 1; + must_check_value = true; else if (b->watchpoint_triggered == watch_triggered_yes) /* We have a hardware watchpoint (read, write, or access) and the target earlier reported an address watched by this watchpoint. */ - must_check_value = 1; + must_check_value = true; else if (b->watchpoint_triggered == watch_triggered_unknown && b->type == bp_hardware_watchpoint) /* We were stopped by a hardware watchpoint, but the target could not report the data address. We must check the watchpoint's value. Access and read watchpoints are out of luck; without a data address, we can't figure it out. */ - must_check_value = 1; + must_check_value = true; if (must_check_value) { @@ -5272,7 +5270,7 @@ bpstat_check_watchpoint (bpstat *bs) break; } } - else /* must_check_value == 0 */ + else /* !must_check_value */ { /* This is a case where some watchpoint(s) triggered, but not at the address of this watchpoint, or else no @@ -5342,7 +5340,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) if (cond && b->disposition != disp_del_at_next_stop) { - int within_current_scope = 1; + bool within_current_scope = true; struct watchpoint * w; /* We use value_mark and value_free_to_mark because it could @@ -5387,7 +5385,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) if (frame != NULL) select_frame (frame); else - within_current_scope = 0; + within_current_scope = false; } if (within_current_scope) { @@ -5429,7 +5427,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) /* Returns true if we need to track moribund locations of LOC's type on the current target. */ -static int +static bool need_moribund_for_location_type (const struct bp_location *loc) { return ((loc->loc_type == bp_loc_software_breakpoint @@ -5611,7 +5609,7 @@ bpstat_stop_status (const address_space *aspace, { struct watchpoint *w = (struct watchpoint *) bs->breakpoint_at; - update_watchpoint (w, 0 /* don't reparse. */); + update_watchpoint (w, false /* don't reparse. */); need_remove_insert = 1; } @@ -6172,8 +6170,8 @@ print_one_breakpoint_location (struct breakpoint *b, static char bpenables[] = "nynny"; struct ui_out *uiout = current_uiout; - int header_of_multiple = 0; - int part_of_multiple = (loc != NULL); + bool header_of_multiple = false; + bool part_of_multiple = (loc != NULL); struct value_print_options opts; get_user_print_options (&opts); @@ -6185,7 +6183,7 @@ print_one_breakpoint_location (struct breakpoint *b, && (b->loc != NULL && (b->loc->next != NULL || !b->loc->enabled || b->loc->disabled_by_cond))) - header_of_multiple = 1; + header_of_multiple = true; if (loc == NULL) loc = b->loc; @@ -6844,7 +6842,7 @@ maintenance_info_breakpoints (const char *args, int from_tty) default_collect_info (); } -static int +static bool breakpoint_has_pc (struct breakpoint *b, struct program_space *pspace, CORE_ADDR pc, struct obj_section *section) @@ -6854,9 +6852,9 @@ breakpoint_has_pc (struct breakpoint *b, if (bl->pspace == pspace && bl->address == pc && (!overlay_debugging || bl->section == section)) - return 1; + return true; } - return 0; + return false; } /* See breakpoint.h. */ @@ -6922,7 +6920,7 @@ bl_address_is_meaningful (bp_location *loc) /* Assuming LOC1 and LOC2's owners are hardware watchpoints, returns true if LOC1 and LOC2 represent the same watchpoint location. */ -static int +static bool watchpoint_locations_match (const struct bp_location *loc1, const struct bp_location *loc2) { @@ -6949,7 +6947,7 @@ watchpoint_locations_match (const struct bp_location *loc1, loc2->length, loc2->watchpoint_type, w2->cond_exp.get ()))) - return 0; + return false; /* Note that this checks the owner's type, not the location's. In case the target does not support read watchpoints, but does @@ -6979,7 +6977,7 @@ breakpoint_address_match (const address_space *aspace1, CORE_ADDR addr1, matches ASPACE2. On targets that have global breakpoints, the address space doesn't really matter. */ -static int +static bool breakpoint_address_match_range (const address_space *aspace1, CORE_ADDR addr1, int len1, const address_space *aspace2, @@ -6995,7 +6993,7 @@ breakpoint_address_match_range (const address_space *aspace1, matches the breakpoint's address space. On targets that have global breakpoints, the address space doesn't really matter. */ -static int +static bool breakpoint_location_address_match (struct bp_location *bl, const address_space *aspace, CORE_ADDR addr) @@ -7014,7 +7012,7 @@ breakpoint_location_address_match (struct bp_location *bl, space. On targets that have global breakpoints, the address space doesn't really matter. */ -static int +static bool breakpoint_location_address_range_overlap (struct bp_location *bl, const address_space *aspace, CORE_ADDR addr, int len) @@ -7034,7 +7032,7 @@ breakpoint_location_address_range_overlap (struct bp_location *bl, Then, if LOC1 and LOC2 represent the same tracepoint location, returns true, otherwise returns false. */ -static int +static bool tracepoint_locations_match (const struct bp_location *loc1, const struct bp_location *loc2) { @@ -7044,7 +7042,7 @@ tracepoint_locations_match (const struct bp_location *loc1, different locations. */ return (loc1->address == loc2->address && loc1->owner == loc2->owner); else - return 0; + return false; } /* Assuming LOC1 and LOC2's types' have meaningful target addresses @@ -7053,7 +7051,7 @@ tracepoint_locations_match (const struct bp_location *loc1, breakpoint locations and hardware breakpoint locations match, otherwise they don't. */ -static int +static bool breakpoint_locations_match (const struct bp_location *loc1, const struct bp_location *loc2, bool sw_hw_bps_match) @@ -7068,7 +7066,7 @@ breakpoint_locations_match (const struct bp_location *loc1, hw_point2 = is_hardware_watchpoint (loc2->owner); if (hw_point1 != hw_point2) - return 0; + return false; else if (hw_point1) return watchpoint_locations_match (loc1, loc2); else if (is_tracepoint (loc1->owner) || is_tracepoint (loc2->owner)) @@ -7085,7 +7083,7 @@ breakpoint_locations_match (const struct bp_location *loc1, static void breakpoint_adjustment_warning (CORE_ADDR from_addr, CORE_ADDR to_addr, - int bnum, int have_bnum) + int bnum, bool have_bnum) { /* The longest string possibly returned by hex_string_custom is 50 chars. These must be at least that big for safety. */ @@ -7157,7 +7155,7 @@ adjust_breakpoint_address (struct gdbarch *gdbarch, a user's expectations. Print a warning if an adjustment is required. */ if (adjusted_bpaddr != bpaddr) - breakpoint_adjustment_warning (bpaddr, adjusted_bpaddr, 0, 0); + breakpoint_adjustment_warning (bpaddr, adjusted_bpaddr, 0, false); return adjusted_bpaddr; } @@ -7667,7 +7665,7 @@ disable_breakpoints_in_shlibs (void) static void disable_breakpoints_in_unloaded_shlib (struct so_list *solib) { - int disabled_shlib_breaks = 0; + bool disabled_shlib_breaks = false; for (bp_location *loc : all_bp_locations ()) { @@ -7700,7 +7698,7 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib) "for unloaded shared library \"%s\""), solib->so_name); } - disabled_shlib_breaks = 1; + disabled_shlib_breaks = true; } } } @@ -7732,7 +7730,7 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) for (breakpoint *b : all_breakpoints ()) { - int bp_modified = 0; + bool bp_modified = false; if (!is_breakpoint (b) && !is_tracepoint (b)) continue; @@ -7766,7 +7764,7 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) mark_breakpoint_location_modified (loc); - bp_modified = 1; + bp_modified = true; } } @@ -8802,7 +8800,7 @@ create_breakpoint (struct gdbarch *gdbarch, unsigned flags) { struct linespec_result canonical; - int pending = 0; + bool pending = false; int task = 0; int prev_bkpt_count = breakpoint_count; @@ -8841,7 +8839,7 @@ create_breakpoint (struct gdbarch *gdbarch, a pending breakpoint and selected yes, or pending breakpoint behavior is on and thus a pending breakpoint is defaulted on behalf of the user. */ - pending = 1; + pending = true; } else throw; @@ -9435,7 +9433,7 @@ watchpoint::re_set () Don't do anything about disabled watchpoints, since they will be reevaluated again when enabled. */ - update_watchpoint (this, 1 /* reparse */); + update_watchpoint (this, true /* reparse */); } /* Implement the "insert" method for hardware watchpoints. */ @@ -10178,7 +10176,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, /* Finally update the new watchpoint. This creates the locations that should be inserted. */ - update_watchpoint (w.get (), 1); + update_watchpoint (w.get (), true /* reparse */); install_breakpoint (internal, std::move (w), 1); } @@ -10829,7 +10827,7 @@ download_tracepoint_locations (void) for (breakpoint *b : all_tracepoints ()) { struct tracepoint *t; - int bp_location_downloaded = 0; + bool bp_location_downloaded = false; if ((b->type == bp_fast_tracepoint ? !may_insert_fast_tracepoints @@ -10860,7 +10858,7 @@ download_tracepoint_locations (void) target_download_tracepoint (bl); bl->inserted = 1; - bp_location_downloaded = 1; + bp_location_downloaded = true; } t = (struct tracepoint *) b; t->number_on_target = b->number; @@ -11004,10 +11002,10 @@ update_global_location_list (enum ugll_insert_mode insert_mode) { /* Tells if 'old_loc' is found among the new locations. If not, we have to free it. */ - int found_object = 0; + bool found_object = false; /* Tells if the location should remain inserted in the target. */ - int keep_in_target = 0; - int removed = 0; + bool keep_in_target = false; + bool removed = false; /* Skip LOCP entries which will definitely never be needed. Stop either at or being the one matching OLD_LOC. */ @@ -11033,7 +11031,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) } if (bp_locations[loc2_i] == old_loc) - found_object = 1; + found_object = true; } /* We have already handled this address, update it so that we don't @@ -11059,7 +11057,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) { /* The location is still present in the location list, and still should be inserted. Don't do anything. */ - keep_in_target = 1; + keep_in_target = true; } else { @@ -11102,7 +11100,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) if (unduplicated_should_be_inserted (loc2)) { swap_insertion (old_loc, loc2); - keep_in_target = 1; + keep_in_target = true; break; } } @@ -11126,7 +11124,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) "breakpoint %d\n"), old_loc->owner->number); } - removed = 1; + removed = true; } } @@ -11600,7 +11598,7 @@ ordinary_breakpoint::print_it (const bpstat *bs) const if (bl->address != bl->requested_address) breakpoint_adjustment_warning (bl->requested_address, bl->address, - number, 1); + number, true); annotate_breakpoint (number); maybe_print_thread_hit_breakpoint (uiout); @@ -12082,7 +12080,7 @@ static struct breakpoint_ops strace_marker_breakpoint_ops = strace_marker_create_breakpoints_sal, }; -static int +static bool strace_marker_p (struct breakpoint *b) { return b->type == bp_static_marker_tracepoint; @@ -12254,7 +12252,7 @@ delete_command (const char *arg, int from_tty) PSPACE is NULL, all locations of all program spaces are considered. */ -static int +static bool all_locations_are_pending (struct breakpoint *b, struct program_space *pspace) { for (bp_location *loc : b->locations ()) @@ -12262,15 +12260,15 @@ all_locations_are_pending (struct breakpoint *b, struct program_space *pspace) || loc->pspace == pspace) && !loc->shlib_disabled && !loc->pspace->executing_startup) - return 0; - return 1; + return false; + return true; } /* Subroutine of update_breakpoint_locations to simplify it. - Return non-zero if multiple fns in list LOC have the same name. + Return true if multiple fns in list LOC have the same name. Null names are ignored. */ -static int +static bool ambiguous_names_p (struct bp_location *loc) { struct bp_location *l; @@ -12291,11 +12289,11 @@ ambiguous_names_p (struct bp_location *loc) /* NOTE: We can assume slot != NULL here because xcalloc never returns NULL. */ if (*slot != NULL) - return 1; + return true; *slot = name; } - return 0; + return false; } /* When symbols change, it probably means the sources changed as well, @@ -12422,34 +12420,34 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal) return sal; } -/* Returns 1 iff locations A and B are sufficiently same that +/* Returns true iff locations A and B are sufficiently same that we don't need to report breakpoint as changed. */ -static int +static bool locations_are_equal (struct bp_location *a, struct bp_location *b) { while (a && b) { if (a->address != b->address) - return 0; + return false; if (a->shlib_disabled != b->shlib_disabled) - return 0; + return false; if (a->enabled != b->enabled) - return 0; + return false; if (a->disabled_by_cond != b->disabled_by_cond) - return 0; + return false; a = a->next; b = b->next; } if ((a == NULL) != (b == NULL)) - return 0; + return false; - return 1; + return true; } /* Split all locations of B that are bound to PSPACE out of B's @@ -12634,7 +12632,7 @@ code_breakpoint::location_spec_to_sals (location_spec *locspec, } catch (gdb_exception_error &e) { - int not_found_and_ok = 0; + int not_found_and_ok = false; /* For pending breakpoints, it's expected that parsing will fail until the right shared library is loaded. User has @@ -12651,7 +12649,7 @@ code_breakpoint::location_spec_to_sals (location_spec *locspec, || (loc && loc->shlib_disabled) || (loc && loc->pspace->executing_startup) || enable_state == bp_disabled)) - not_found_and_ok = 1; + not_found_and_ok = true; if (!not_found_and_ok) { @@ -13336,7 +13334,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition, orig_enable_state = bpt->enable_state; bpt->enable_state = bp_enabled; - update_watchpoint (w, 1 /* reparse */); + update_watchpoint (w, true /* reparse */); } catch (const gdb_exception &e) { @@ -13977,7 +13975,7 @@ static void save_breakpoints (const char *filename, int from_tty, bool (*filter) (const struct breakpoint *)) { - int any = 0; + bool any = false; int extra_trace_bits = 0; if (filename == 0 || *filename == 0) @@ -13994,7 +13992,7 @@ save_breakpoints (const char *filename, int from_tty, if (filter && !filter (tp)) continue; - any = 1; + any = true; if (is_tracepoint (tp)) { @@ -14182,18 +14180,15 @@ add_catch_command (const char *name, const char *docstring, set_cmd_completer (command, completer); } -/* Zero if any of the breakpoint's locations could be a location where - functions have been inlined, nonzero otherwise. */ +/* False if any of the breakpoint's locations could be a location where + functions have been inlined, true otherwise. */ -static int +static bool is_non_inline_function (struct breakpoint *b) { /* The shared library event breakpoint is set on the address of a non-inline function. */ - if (b->type == bp_shlib_event) - return 1; - - return 0; + return (b->type == bp_shlib_event); } /* Nonzero if the specified PC cannot be a location where functions -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 0/4] Const args, int->bool, and scoped_restore in breakpoint.c 2022-10-05 9:32 [PATCH 0/4] Const args, int->bool, and scoped_restore in breakpoint.c Andrew Burgess ` (3 preceding siblings ...) 2022-10-05 9:32 ` [PATCH 4/4] gdb: some int to bool conversion in breakpoint.c Andrew Burgess @ 2022-10-19 13:07 ` Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 1/4] gdb: make some bp_location arguments const " Andrew Burgess ` (3 more replies) 4 siblings, 4 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-19 13:07 UTC (permalink / raw) To: gdb-patches A series of patches that does some general code polishing in breakpoint.c. I don't think there should be any user visible changes after any of these patches. Changes in v2: - Rebased on latest upstream master (5ac37f0643a) and resolved merged conflict. --- Andrew Burgess (4): gdb: make some bp_location arguments const in breakpoint.c gdb: used scoped_restore_frame in update_watchpoint gdb: make use of scoped_restore in unduplicated_should_be_inserted gdb: some int to bool conversion in breakpoint.c gdb/breakpoint.c | 319 ++++++++++++++++++++++------------------------- 1 file changed, 152 insertions(+), 167 deletions(-) -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 1/4] gdb: make some bp_location arguments const in breakpoint.c 2022-10-19 13:07 ` [PATCHv2 0/4] Const args, int->bool, and scoped_restore " Andrew Burgess @ 2022-10-19 13:07 ` Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 2/4] gdb: used scoped_restore_frame in update_watchpoint Andrew Burgess ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-19 13:07 UTC (permalink / raw) To: gdb-patches I spotted a few places where I could make some 'bp_location *' arguments constant in breakpoint.c. There should be no user visible changes after this commit. --- gdb/breakpoint.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 91e13a03360..e80a46cb01e 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -121,11 +121,11 @@ static CORE_ADDR adjust_breakpoint_address (struct gdbarch *gdbarch, enum bptype bptype, struct program_space *pspace); -static int watchpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2); +static int watchpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2); -static int breakpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2, +static int breakpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2, bool sw_hw_bps_match = false); static int breakpoint_location_address_match (struct bp_location *bl, @@ -2641,7 +2641,7 @@ build_target_command_list (struct bp_location *bl) registers state. */ static int -breakpoint_kind (struct bp_location *bl, CORE_ADDR *addr) +breakpoint_kind (const struct bp_location *bl, CORE_ADDR *addr) { if (bl->owner->type == bp_single_step) { @@ -4227,7 +4227,7 @@ moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) ASPACE. */ static int -bp_location_inserted_here_p (struct bp_location *bl, +bp_location_inserted_here_p (const struct bp_location *bl, const address_space *aspace, CORE_ADDR pc) { if (bl->inserted @@ -5453,7 +5453,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) on the current target. */ static int -need_moribund_for_location_type (struct bp_location *loc) +need_moribund_for_location_type (const struct bp_location *loc) { return ((loc->loc_type == bp_loc_software_breakpoint && !target_supports_stopped_by_sw_breakpoint ()) @@ -5987,7 +5987,7 @@ bp_condition_evaluator (const breakpoint *b) similar to bp_condition_evaluator, but for locations. */ static const char * -bp_location_condition_evaluator (struct bp_location *bl) +bp_location_condition_evaluator (const struct bp_location *bl) { if (bl && !is_breakpoint (bl->owner)) return NULL; @@ -6946,8 +6946,8 @@ bl_address_is_meaningful (bp_location *loc) true if LOC1 and LOC2 represent the same watchpoint location. */ static int -watchpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2) +watchpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2) { struct watchpoint *w1 = (struct watchpoint *) loc1->owner; struct watchpoint *w2 = (struct watchpoint *) loc2->owner; @@ -7058,8 +7058,8 @@ breakpoint_location_address_range_overlap (struct bp_location *bl, true, otherwise returns false. */ static int -tracepoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2) +tracepoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2) { if (is_tracepoint (loc1->owner) && is_tracepoint (loc2->owner)) /* Since tracepoint locations are never duplicated with others', tracepoint @@ -7077,8 +7077,8 @@ tracepoint_locations_match (struct bp_location *loc1, otherwise they don't. */ static int -breakpoint_locations_match (struct bp_location *loc1, - struct bp_location *loc2, +breakpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2, bool sw_hw_bps_match) { int hw_point1, hw_point2; -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 2/4] gdb: used scoped_restore_frame in update_watchpoint 2022-10-19 13:07 ` [PATCHv2 0/4] Const args, int->bool, and scoped_restore " Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 1/4] gdb: make some bp_location arguments const " Andrew Burgess @ 2022-10-19 13:07 ` Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 4/4] gdb: some int to bool conversion in breakpoint.c Andrew Burgess 3 siblings, 0 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-19 13:07 UTC (permalink / raw) To: gdb-patches I was doing some int to bool cleanup in update_watchpoint, and I noticed a manual version of scoped_restore_selected_frame. As always when these things are done manually, there is the chance that, in an error case, we might leave the wrong frame selected. This commit updates things to use scoped_restore_selected_frame, and also converts a local variable from int to bool. The only user visible change after this commit is in the case where update_watchpoint throws an error - we should now correctly restore the previously selected frame. Otherwise, this commit should be invisible to the user. --- gdb/breakpoint.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index e80a46cb01e..3ddf025acca 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1924,9 +1924,7 @@ add_dummy_location (struct breakpoint *b, static void update_watchpoint (struct watchpoint *b, int reparse) { - int within_current_scope; - struct frame_id saved_frame_id; - int frame_saved; + bool within_current_scope; /* If this is a local watchpoint, we only want to check if the watchpoint frame is in scope if the current thread is the thread @@ -1936,12 +1934,12 @@ update_watchpoint (struct watchpoint *b, int reparse) if (b->disposition == disp_del_at_next_stop) return; - - frame_saved = 0; + + gdb::optional<scoped_restore_selected_frame> restore_frame; /* Determine if the watchpoint is within scope. */ if (b->exp_valid_block == NULL) - within_current_scope = 1; + within_current_scope = true; else { frame_info_ptr fi = get_current_frame (); @@ -1960,8 +1958,7 @@ update_watchpoint (struct watchpoint *b, int reparse) /* FIXME drow/2003-09-09: It would be nice if evaluate_expression took a frame parameter, so that we didn't have to change the selected frame. */ - frame_saved = 1; - saved_frame_id = get_frame_id (get_selected_frame (NULL)); + restore_frame.emplace (); fi = frame_find_by_id (b->watchpoint_frame); within_current_scope = (fi != NULL); @@ -2227,10 +2224,6 @@ in which its expression is valid.\n"), b->number); watchpoint_del_at_next_stop (b); } - - /* Restore the selected frame. */ - if (frame_saved) - select_frame (frame_find_by_id (saved_frame_id)); } -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted 2022-10-19 13:07 ` [PATCHv2 0/4] Const args, int->bool, and scoped_restore " Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 1/4] gdb: make some bp_location arguments const " Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 2/4] gdb: used scoped_restore_frame in update_watchpoint Andrew Burgess @ 2022-10-19 13:07 ` Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 4/4] gdb: some int to bool conversion in breakpoint.c Andrew Burgess 3 siblings, 0 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-19 13:07 UTC (permalink / raw) To: gdb-patches I noticed that we could make use of a scoped_restore in the function unduplicated_should_be_inserted. I've also converted the function return type from int to bool. This change shouldn't make any difference, as I don't think anything within should_be_inserted could throw an exception, but the change doesn't hurt, and will help keep us safe if anything ever changes in the future. --- gdb/breakpoint.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 3ddf025acca..7e2b804156f 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2299,16 +2299,13 @@ should_be_inserted (struct bp_location *bl) /* Same as should_be_inserted but does the check assuming that the location is not duplicated. */ -static int +static bool unduplicated_should_be_inserted (struct bp_location *bl) { - int result; - const int save_duplicate = bl->duplicate; + scoped_restore restore_bl_duplicate + = make_scoped_restore (&bl->duplicate, 0); - bl->duplicate = 0; - result = should_be_inserted (bl); - bl->duplicate = save_duplicate; - return result; + return should_be_inserted (bl); } /* Parses a conditional described by an expression COND into an -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 4/4] gdb: some int to bool conversion in breakpoint.c 2022-10-19 13:07 ` [PATCHv2 0/4] Const args, int->bool, and scoped_restore " Andrew Burgess ` (2 preceding siblings ...) 2022-10-19 13:07 ` [PATCHv2 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted Andrew Burgess @ 2022-10-19 13:07 ` Andrew Burgess 2022-10-19 20:03 ` Simon Marchi 3 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2022-10-19 13:07 UTC (permalink / raw) To: gdb-patches Some int to bool conversion in breakpoint.c. I've only updated the function signatures of static functions, but I've updated some function local variables throughout the file. The most interesting change is in unduplicated_should_be_inserted, where I've made use of make_scoped_restore to temporarily change the bl->duplicate flag. There should be no user visible changes after this commit. --- gdb/breakpoint.c | 271 +++++++++++++++++++++++------------------------ 1 file changed, 133 insertions(+), 138 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 7e2b804156f..9cfbe5e4dcf 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -114,27 +114,27 @@ static struct breakpoint * enum bptype type, int loc_enabled, int thread); -static void breakpoint_adjustment_warning (CORE_ADDR, CORE_ADDR, int, int); +static void breakpoint_adjustment_warning (CORE_ADDR, CORE_ADDR, int, bool); static CORE_ADDR adjust_breakpoint_address (struct gdbarch *gdbarch, CORE_ADDR bpaddr, enum bptype bptype, struct program_space *pspace); -static int watchpoint_locations_match (const struct bp_location *loc1, - const struct bp_location *loc2); +static bool watchpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2); -static int breakpoint_locations_match (const struct bp_location *loc1, - const struct bp_location *loc2, - bool sw_hw_bps_match = false); +static bool breakpoint_locations_match (const struct bp_location *loc1, + const struct bp_location *loc2, + bool sw_hw_bps_match = false); -static int breakpoint_location_address_match (struct bp_location *bl, - const struct address_space *aspace, - CORE_ADDR addr); +static bool breakpoint_location_address_match (struct bp_location *bl, + const struct address_space *aspace, + CORE_ADDR addr); -static int breakpoint_location_address_range_overlap (struct bp_location *, - const address_space *, - CORE_ADDR, int); +static bool breakpoint_location_address_range_overlap (struct bp_location *, + const address_space *, + CORE_ADDR, int); static int remove_breakpoint (struct bp_location *); static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason); @@ -210,10 +210,10 @@ static void set_tracepoint_count (int num); static bool is_masked_watchpoint (const struct breakpoint *b); -/* Return 1 if B refers to a static tracepoint set by marker ("-m"), zero - otherwise. */ +/* Return true if B refers to a static tracepoint set by marker ("-m"), + zero otherwise. */ -static int strace_marker_p (struct breakpoint *b); +static bool strace_marker_p (struct breakpoint *b); static void bkpt_probe_create_sals_from_location_spec (location_spec *locspec, @@ -593,7 +593,7 @@ breakpoint_condition_evaluation_mode (void) /* Return true if GDB should evaluate breakpoint conditions or false otherwise. */ -static int +static bool gdb_evaluates_breakpoint_condition_p (void) { const char *mode = breakpoint_condition_evaluation_mode (); @@ -758,7 +758,7 @@ static struct cmd_list_element *breakpoint_show_cmdlist; struct cmd_list_element *save_cmdlist; /* Return whether a breakpoint is an active enabled breakpoint. */ -static int +static bool breakpoint_enabled (struct breakpoint *b) { return (b->enable_state == bp_enabled); @@ -1556,19 +1556,19 @@ commands_from_control_command (const char *arg, struct command_line *cmd) return simple_control; } -/* Return non-zero if BL->TARGET_INFO contains valid information. */ +/* Return true if BL->TARGET_INFO contains valid information. */ -static int +static bool bp_location_has_shadow (struct bp_location *bl) { if (bl->loc_type != bp_loc_software_breakpoint) - return 0; + return false; if (!bl->inserted) - return 0; + return false; if (bl->target_info.shadow_len == 0) /* BL isn't valid, or doesn't shadow memory. */ - return 0; - return 1; + return false; + return true; } /* Update BUF, which is LEN bytes read from the target address @@ -1806,7 +1806,7 @@ is_watchpoint (const struct breakpoint *bpt) and in any state. It is presently left to the target allowing memory accesses when threads are running. */ -static int +static bool watchpoint_in_thread_scope (struct watchpoint *b) { return (b->pspace == current_program_space @@ -1922,7 +1922,7 @@ add_dummy_location (struct breakpoint *b, watchpoint removal from inferior. */ static void -update_watchpoint (struct watchpoint *b, int reparse) +update_watchpoint (struct watchpoint *b, bool reparse) { bool within_current_scope; @@ -2226,27 +2226,27 @@ in which its expression is valid.\n"), } } - -/* Returns 1 iff breakpoint location should be +/* Returns true iff breakpoint location should be inserted in the inferior. We don't differentiate the type of BL's owner (breakpoint vs. tracepoint), although insert_location in tracepoint's breakpoint_ops is not defined, because in insert_bp_location, tracepoint's insert_location will not be called. */ -static int + +static bool should_be_inserted (struct bp_location *bl) { if (bl->owner == NULL || !breakpoint_enabled (bl->owner)) - return 0; + return false; if (bl->owner->disposition == disp_del_at_next_stop) - return 0; + return false; if (!bl->enabled || bl->disabled_by_cond || bl->shlib_disabled || bl->duplicate) - return 0; + return false; if (user_breakpoint_p (bl->owner) && bl->pspace->executing_startup) - return 0; + return false; /* This is set for example, when we're attached to the parent of a vfork, and have detached from the child. The child is running @@ -2257,7 +2257,7 @@ should_be_inserted (struct bp_location *bl) the child could still trip on the parent's breakpoints. Since the parent is blocked anyway, it won't miss any breakpoint. */ if (bl->pspace->breakpoints_not_allowed) - return 0; + return false; /* Don't insert a breakpoint if we're trying to step past its location, except if the breakpoint is a single-step breakpoint, @@ -2279,7 +2279,7 @@ should_be_inserted (struct bp_location *bl) { infrun_debug_printf ("skipping breakpoint: stepping past insn at: %s", paddress (bl->gdbarch, bl->address)); - return 0; + return false; } /* Don't insert watchpoints if we're trying to step past the @@ -2290,10 +2290,10 @@ should_be_inserted (struct bp_location *bl) infrun_debug_printf ("stepping past non-steppable watchpoint. " "skipping watchpoint at %s:%d", paddress (bl->gdbarch, bl->address), bl->length); - return 0; + return false; } - return 1; + return true; } /* Same as should_be_inserted but does the check assuming @@ -2348,7 +2348,7 @@ parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond) static void build_target_condition_list (struct bp_location *bl) { - int null_condition_or_parse_error = 0; + bool null_condition_or_parse_error = false; int modified = bl->needs_update; /* Release conditions left over from a previous insert. */ @@ -2391,7 +2391,7 @@ build_target_condition_list (struct bp_location *bl) went wrong or we have a null condition expression. */ if (!loc->cond_bytecode) { - null_condition_or_parse_error = 1; + null_condition_or_parse_error = true; break; } } @@ -2525,7 +2525,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) static void build_target_command_list (struct bp_location *bl) { - int null_command_or_parse_error = 0; + bool null_command_or_parse_error = false; int modified = bl->needs_update; /* Clear commands left over from a previous insert. */ @@ -2574,7 +2574,7 @@ build_target_command_list (struct bp_location *bl) went wrong or we have a null command expression. */ if (!loc->cmd_bytecode) { - null_command_or_parse_error = 1; + null_command_or_parse_error = true; break; } } @@ -3019,7 +3019,7 @@ insert_breakpoints (void) { struct watchpoint *w = (struct watchpoint *) bpt; - update_watchpoint (w, 0 /* don't reparse. */); + update_watchpoint (w, false /* don't reparse. */); } /* Updating watchpoints creates new locations, so update the global @@ -3140,7 +3140,7 @@ insert_breakpoint_locations (void) them, as half-inserted watchpoint is of limited use. */ for (breakpoint *bpt : all_breakpoints ()) { - int some_failed = 0; + bool some_failed = false; if (!is_hardware_watchpoint (bpt)) continue; @@ -3154,7 +3154,7 @@ insert_breakpoint_locations (void) for (bp_location *loc : bpt->locations ()) if (!loc->inserted && should_be_inserted (loc)) { - some_failed = 1; + some_failed = true; break; } @@ -3321,7 +3321,7 @@ static struct minimal_symbol msym_not_found; /* Returns TRUE if MSYM point to the "not found" sentinel. */ -static int +static bool msym_not_found_p (const struct minimal_symbol *msym) { return msym == &msym_not_found; @@ -3683,7 +3683,7 @@ create_exception_master_breakpoint (void) /* Does B have a location spec? */ -static int +static bool breakpoint_location_spec_empty_p (const struct breakpoint *b) { return (b->locspec != nullptr && b->locspec->empty_p ()); @@ -4142,7 +4142,7 @@ breakpoint_init_inferior (enum inf_context context) enum breakpoint_here breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) { - int any_breakpoint_here = 0; + bool any_breakpoint_here = false; for (bp_location *bl : all_bp_locations ()) { @@ -4162,7 +4162,7 @@ breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) else if (bl->permanent) return permanent_breakpoint_here; else - any_breakpoint_here = 1; + any_breakpoint_here = true; } } @@ -4216,7 +4216,7 @@ moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) /* Returns non-zero iff BL is inserted at PC, in address space ASPACE. */ -static int +static bool bp_location_inserted_here_p (const struct bp_location *bl, const address_space *aspace, CORE_ADDR pc) { @@ -4224,14 +4224,12 @@ bp_location_inserted_here_p (const struct bp_location *bl, && breakpoint_address_match (bl->pspace->aspace, bl->address, aspace, pc)) { - if (overlay_debugging - && section_is_overlay (bl->section) - && !section_is_mapped (bl->section)) - return 0; /* unmapped overlay -- can't be a match */ - else - return 1; + /* An unmapped overlay can't be a match. */ + return !(overlay_debugging + && section_is_overlay (bl->section) + && !section_is_mapped (bl->section)); } - return 0; + return false; } /* Returns non-zero iff there's a breakpoint inserted at PC. */ @@ -4494,10 +4492,10 @@ breakpoint_about_to_proceed (void) breakpoint_proceeded = 1; } -/* Return non-zero iff CMD as the first line of a command sequence is `silent' +/* Return true iff CMD as the first line of a command sequence is `silent' or its equivalent. */ -static int +static bool command_line_is_silent (struct command_line *cmd) { return cmd && (strcmp ("silent", cmd->line) == 0); @@ -4512,16 +4510,16 @@ command_line_is_silent (struct command_line *cmd) case, it is the caller's responsibility to recall it again with the bpstat of the current thread. */ -static int +static bool bpstat_do_actions_1 (bpstat **bsp) { bpstat *bs; - int again = 0; + bool again = false; /* Avoid endless recursion if a `source' command is contained in bs->commands. */ if (executing_breakpoint_commands) - return 0; + return false; scoped_restore save_executing = make_scoped_restore (&executing_breakpoint_commands, 1); @@ -4587,7 +4585,7 @@ bpstat_do_actions_1 (bpstat **bsp) command, and can easily blow up GDB stack. Instead, we return true, which will trigger the caller to recall us with the new stop_bpstat. */ - again = 1; + again = true; break; } } @@ -4959,7 +4957,7 @@ watchpoint_check (bpstat *bs) { struct watchpoint *b; frame_info_ptr fr; - int within_current_scope; + bool within_current_scope; /* BS is built from an existing struct breakpoint. */ gdb_assert (bs->breakpoint_at != NULL); @@ -4972,7 +4970,7 @@ watchpoint_check (bpstat *bs) return WP_IGNORE; if (b->exp_valid_block == NULL) - within_current_scope = 1; + within_current_scope = true; else { frame_info_ptr frame = get_current_frame (); @@ -5003,7 +5001,7 @@ watchpoint_check (bpstat *bs) function = get_frame_function (fr); if (function == NULL || !contained_in (b->exp_valid_block, function->value_block ())) - within_current_scope = 0; + within_current_scope = false; } if (within_current_scope) @@ -5099,7 +5097,7 @@ watchpoint_check (bpstat *bs) breakpoint location BL. This function does not check if we should stop, only if BL explains the stop. */ -static int +static bool bpstat_check_location (const struct bp_location *bl, const address_space *aspace, CORE_ADDR bp_addr, const target_waitstatus &ws) @@ -5128,24 +5126,24 @@ bpstat_check_watchpoint (bpstat *bs) gdb_assert (b != NULL); { - int must_check_value = 0; - + bool must_check_value = false; + if (b->type == bp_watchpoint) /* For a software watchpoint, we must always check the watched value. */ - must_check_value = 1; + must_check_value = true; else if (b->watchpoint_triggered == watch_triggered_yes) /* We have a hardware watchpoint (read, write, or access) and the target earlier reported an address watched by this watchpoint. */ - must_check_value = 1; + must_check_value = true; else if (b->watchpoint_triggered == watch_triggered_unknown && b->type == bp_hardware_watchpoint) /* We were stopped by a hardware watchpoint, but the target could not report the data address. We must check the watchpoint's value. Access and read watchpoints are out of luck; without a data address, we can't figure it out. */ - must_check_value = 1; + must_check_value = true; if (must_check_value) { @@ -5267,7 +5265,7 @@ bpstat_check_watchpoint (bpstat *bs) break; } } - else /* must_check_value == 0 */ + else /* !must_check_value */ { /* This is a case where some watchpoint(s) triggered, but not at the address of this watchpoint, or else no @@ -5347,7 +5345,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) if (cond != nullptr && b->disposition != disp_del_at_next_stop) { - int within_current_scope = 1; + bool within_current_scope = true; struct watchpoint * w; /* We use scoped_value_mark because it could be a long time @@ -5391,7 +5389,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) if (frame != NULL) select_frame (frame); else - within_current_scope = 0; + within_current_scope = false; } if (within_current_scope) { @@ -5442,7 +5440,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) /* Returns true if we need to track moribund locations of LOC's type on the current target. */ -static int +static bool need_moribund_for_location_type (const struct bp_location *loc) { return ((loc->loc_type == bp_loc_software_breakpoint @@ -5624,7 +5622,7 @@ bpstat_stop_status (const address_space *aspace, { struct watchpoint *w = (struct watchpoint *) bs->breakpoint_at; - update_watchpoint (w, 0 /* don't reparse. */); + update_watchpoint (w, false /* don't reparse. */); need_remove_insert = 1; } @@ -6185,8 +6183,8 @@ print_one_breakpoint_location (struct breakpoint *b, static char bpenables[] = "nynny"; struct ui_out *uiout = current_uiout; - int header_of_multiple = 0; - int part_of_multiple = (loc != NULL); + bool header_of_multiple = false; + bool part_of_multiple = (loc != NULL); struct value_print_options opts; get_user_print_options (&opts); @@ -6198,7 +6196,7 @@ print_one_breakpoint_location (struct breakpoint *b, && (b->loc != NULL && (b->loc->next != NULL || !b->loc->enabled || b->loc->disabled_by_cond))) - header_of_multiple = 1; + header_of_multiple = true; if (loc == NULL) loc = b->loc; @@ -6857,7 +6855,7 @@ maintenance_info_breakpoints (const char *args, int from_tty) default_collect_info (); } -static int +static bool breakpoint_has_pc (struct breakpoint *b, struct program_space *pspace, CORE_ADDR pc, struct obj_section *section) @@ -6867,9 +6865,9 @@ breakpoint_has_pc (struct breakpoint *b, if (bl->pspace == pspace && bl->address == pc && (!overlay_debugging || bl->section == section)) - return 1; + return true; } - return 0; + return false; } /* See breakpoint.h. */ @@ -6935,7 +6933,7 @@ bl_address_is_meaningful (bp_location *loc) /* Assuming LOC1 and LOC2's owners are hardware watchpoints, returns true if LOC1 and LOC2 represent the same watchpoint location. */ -static int +static bool watchpoint_locations_match (const struct bp_location *loc1, const struct bp_location *loc2) { @@ -6962,7 +6960,7 @@ watchpoint_locations_match (const struct bp_location *loc1, loc2->length, loc2->watchpoint_type, w2->cond_exp.get ()))) - return 0; + return false; /* Note that this checks the owner's type, not the location's. In case the target does not support read watchpoints, but does @@ -6992,7 +6990,7 @@ breakpoint_address_match (const address_space *aspace1, CORE_ADDR addr1, matches ASPACE2. On targets that have global breakpoints, the address space doesn't really matter. */ -static int +static bool breakpoint_address_match_range (const address_space *aspace1, CORE_ADDR addr1, int len1, const address_space *aspace2, @@ -7008,7 +7006,7 @@ breakpoint_address_match_range (const address_space *aspace1, matches the breakpoint's address space. On targets that have global breakpoints, the address space doesn't really matter. */ -static int +static bool breakpoint_location_address_match (struct bp_location *bl, const address_space *aspace, CORE_ADDR addr) @@ -7027,7 +7025,7 @@ breakpoint_location_address_match (struct bp_location *bl, space. On targets that have global breakpoints, the address space doesn't really matter. */ -static int +static bool breakpoint_location_address_range_overlap (struct bp_location *bl, const address_space *aspace, CORE_ADDR addr, int len) @@ -7047,7 +7045,7 @@ breakpoint_location_address_range_overlap (struct bp_location *bl, Then, if LOC1 and LOC2 represent the same tracepoint location, returns true, otherwise returns false. */ -static int +static bool tracepoint_locations_match (const struct bp_location *loc1, const struct bp_location *loc2) { @@ -7057,7 +7055,7 @@ tracepoint_locations_match (const struct bp_location *loc1, different locations. */ return (loc1->address == loc2->address && loc1->owner == loc2->owner); else - return 0; + return false; } /* Assuming LOC1 and LOC2's types' have meaningful target addresses @@ -7066,7 +7064,7 @@ tracepoint_locations_match (const struct bp_location *loc1, breakpoint locations and hardware breakpoint locations match, otherwise they don't. */ -static int +static bool breakpoint_locations_match (const struct bp_location *loc1, const struct bp_location *loc2, bool sw_hw_bps_match) @@ -7081,7 +7079,7 @@ breakpoint_locations_match (const struct bp_location *loc1, hw_point2 = is_hardware_watchpoint (loc2->owner); if (hw_point1 != hw_point2) - return 0; + return false; else if (hw_point1) return watchpoint_locations_match (loc1, loc2); else if (is_tracepoint (loc1->owner) || is_tracepoint (loc2->owner)) @@ -7098,7 +7096,7 @@ breakpoint_locations_match (const struct bp_location *loc1, static void breakpoint_adjustment_warning (CORE_ADDR from_addr, CORE_ADDR to_addr, - int bnum, int have_bnum) + int bnum, bool have_bnum) { /* The longest string possibly returned by hex_string_custom is 50 chars. These must be at least that big for safety. */ @@ -7170,7 +7168,7 @@ adjust_breakpoint_address (struct gdbarch *gdbarch, a user's expectations. Print a warning if an adjustment is required. */ if (adjusted_bpaddr != bpaddr) - breakpoint_adjustment_warning (bpaddr, adjusted_bpaddr, 0, 0); + breakpoint_adjustment_warning (bpaddr, adjusted_bpaddr, 0, false); return adjusted_bpaddr; } @@ -7680,7 +7678,7 @@ disable_breakpoints_in_shlibs (void) static void disable_breakpoints_in_unloaded_shlib (struct so_list *solib) { - int disabled_shlib_breaks = 0; + bool disabled_shlib_breaks = false; for (bp_location *loc : all_bp_locations ()) { @@ -7713,7 +7711,7 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib) "for unloaded shared library \"%s\""), solib->so_name); } - disabled_shlib_breaks = 1; + disabled_shlib_breaks = true; } } } @@ -7745,7 +7743,7 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) for (breakpoint *b : all_breakpoints ()) { - int bp_modified = 0; + bool bp_modified = false; if (!is_breakpoint (b) && !is_tracepoint (b)) continue; @@ -7779,7 +7777,7 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) mark_breakpoint_location_modified (loc); - bp_modified = 1; + bp_modified = true; } } @@ -8815,7 +8813,7 @@ create_breakpoint (struct gdbarch *gdbarch, unsigned flags) { struct linespec_result canonical; - int pending = 0; + bool pending = false; int task = 0; int prev_bkpt_count = breakpoint_count; @@ -8854,7 +8852,7 @@ create_breakpoint (struct gdbarch *gdbarch, a pending breakpoint and selected yes, or pending breakpoint behavior is on and thus a pending breakpoint is defaulted on behalf of the user. */ - pending = 1; + pending = true; } else throw; @@ -9448,7 +9446,7 @@ watchpoint::re_set () Don't do anything about disabled watchpoints, since they will be reevaluated again when enabled. */ - update_watchpoint (this, 1 /* reparse */); + update_watchpoint (this, true /* reparse */); } /* Implement the "insert" method for hardware watchpoints. */ @@ -10190,7 +10188,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, /* Finally update the new watchpoint. This creates the locations that should be inserted. */ - update_watchpoint (w.get (), 1); + update_watchpoint (w.get (), true /* reparse */); install_breakpoint (internal, std::move (w), 1); } @@ -10841,7 +10839,7 @@ download_tracepoint_locations (void) for (breakpoint *b : all_tracepoints ()) { struct tracepoint *t; - int bp_location_downloaded = 0; + bool bp_location_downloaded = false; if ((b->type == bp_fast_tracepoint ? !may_insert_fast_tracepoints @@ -10872,7 +10870,7 @@ download_tracepoint_locations (void) target_download_tracepoint (bl); bl->inserted = 1; - bp_location_downloaded = 1; + bp_location_downloaded = true; } t = (struct tracepoint *) b; t->number_on_target = b->number; @@ -11016,10 +11014,10 @@ update_global_location_list (enum ugll_insert_mode insert_mode) { /* Tells if 'old_loc' is found among the new locations. If not, we have to free it. */ - int found_object = 0; + bool found_object = false; /* Tells if the location should remain inserted in the target. */ - int keep_in_target = 0; - int removed = 0; + bool keep_in_target = false; + bool removed = false; /* Skip LOCP entries which will definitely never be needed. Stop either at or being the one matching OLD_LOC. */ @@ -11045,7 +11043,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) } if (bp_locations[loc2_i] == old_loc) - found_object = 1; + found_object = true; } /* We have already handled this address, update it so that we don't @@ -11071,7 +11069,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) { /* The location is still present in the location list, and still should be inserted. Don't do anything. */ - keep_in_target = 1; + keep_in_target = true; } else { @@ -11114,7 +11112,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) if (unduplicated_should_be_inserted (loc2)) { swap_insertion (old_loc, loc2); - keep_in_target = 1; + keep_in_target = true; break; } } @@ -11138,7 +11136,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) "breakpoint %d\n"), old_loc->owner->number); } - removed = 1; + removed = true; } } @@ -11612,7 +11610,7 @@ ordinary_breakpoint::print_it (const bpstat *bs) const if (bl->address != bl->requested_address) breakpoint_adjustment_warning (bl->requested_address, bl->address, - number, 1); + number, true); annotate_breakpoint (number); maybe_print_thread_hit_breakpoint (uiout); @@ -12108,7 +12106,7 @@ static struct breakpoint_ops strace_marker_breakpoint_ops = strace_marker_create_breakpoints_sal, }; -static int +static bool strace_marker_p (struct breakpoint *b) { return b->type == bp_static_marker_tracepoint; @@ -12280,7 +12278,7 @@ delete_command (const char *arg, int from_tty) PSPACE is NULL, all locations of all program spaces are considered. */ -static int +static bool all_locations_are_pending (struct breakpoint *b, struct program_space *pspace) { for (bp_location *loc : b->locations ()) @@ -12288,15 +12286,15 @@ all_locations_are_pending (struct breakpoint *b, struct program_space *pspace) || loc->pspace == pspace) && !loc->shlib_disabled && !loc->pspace->executing_startup) - return 0; - return 1; + return false; + return true; } /* Subroutine of update_breakpoint_locations to simplify it. - Return non-zero if multiple fns in list LOC have the same name. + Return true if multiple fns in list LOC have the same name. Null names are ignored. */ -static int +static bool ambiguous_names_p (struct bp_location *loc) { struct bp_location *l; @@ -12317,11 +12315,11 @@ ambiguous_names_p (struct bp_location *loc) /* NOTE: We can assume slot != NULL here because xcalloc never returns NULL. */ if (*slot != NULL) - return 1; + return true; *slot = name; } - return 0; + return false; } /* When symbols change, it probably means the sources changed as well, @@ -12448,34 +12446,34 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal) return sal; } -/* Returns 1 iff locations A and B are sufficiently same that +/* Returns true iff locations A and B are sufficiently same that we don't need to report breakpoint as changed. */ -static int +static bool locations_are_equal (struct bp_location *a, struct bp_location *b) { while (a && b) { if (a->address != b->address) - return 0; + return false; if (a->shlib_disabled != b->shlib_disabled) - return 0; + return false; if (a->enabled != b->enabled) - return 0; + return false; if (a->disabled_by_cond != b->disabled_by_cond) - return 0; + return false; a = a->next; b = b->next; } if ((a == NULL) != (b == NULL)) - return 0; + return false; - return 1; + return true; } /* Split all locations of B that are bound to PSPACE out of B's @@ -12660,7 +12658,7 @@ code_breakpoint::location_spec_to_sals (location_spec *locspec, } catch (gdb_exception_error &e) { - int not_found_and_ok = 0; + int not_found_and_ok = false; /* For pending breakpoints, it's expected that parsing will fail until the right shared library is loaded. User has @@ -12677,7 +12675,7 @@ code_breakpoint::location_spec_to_sals (location_spec *locspec, || (loc && loc->shlib_disabled) || (loc && loc->pspace->executing_startup) || enable_state == bp_disabled)) - not_found_and_ok = 1; + not_found_and_ok = true; if (!not_found_and_ok) { @@ -13337,7 +13335,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition, orig_enable_state = bpt->enable_state; bpt->enable_state = bp_enabled; - update_watchpoint (w, 1 /* reparse */); + update_watchpoint (w, true /* reparse */); } catch (const gdb_exception &e) { @@ -13979,7 +13977,7 @@ static void save_breakpoints (const char *filename, int from_tty, bool (*filter) (const struct breakpoint *)) { - int any = 0; + bool any = false; int extra_trace_bits = 0; if (filename == 0 || *filename == 0) @@ -13996,7 +13994,7 @@ save_breakpoints (const char *filename, int from_tty, if (filter && !filter (tp)) continue; - any = 1; + any = true; if (is_tracepoint (tp)) { @@ -14184,18 +14182,15 @@ add_catch_command (const char *name, const char *docstring, set_cmd_completer (command, completer); } -/* Zero if any of the breakpoint's locations could be a location where - functions have been inlined, nonzero otherwise. */ +/* False if any of the breakpoint's locations could be a location where + functions have been inlined, true otherwise. */ -static int +static bool is_non_inline_function (struct breakpoint *b) { /* The shared library event breakpoint is set on the address of a non-inline function. */ - if (b->type == bp_shlib_event) - return 1; - - return 0; + return (b->type == bp_shlib_event); } /* Nonzero if the specified PC cannot be a location where functions -- 2.25.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2 4/4] gdb: some int to bool conversion in breakpoint.c 2022-10-19 13:07 ` [PATCHv2 4/4] gdb: some int to bool conversion in breakpoint.c Andrew Burgess @ 2022-10-19 20:03 ` Simon Marchi 2022-10-20 15:48 ` Andrew Burgess 0 siblings, 1 reply; 13+ messages in thread From: Simon Marchi @ 2022-10-19 20:03 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 10/19/22 09:07, Andrew Burgess via Gdb-patches wrote: > Some int to bool conversion in breakpoint.c. I've only updated the > function signatures of static functions, but I've updated some > function local variables throughout the file. > > The most interesting change is in unduplicated_should_be_inserted, > where I've made use of make_scoped_restore to temporarily change the > bl->duplicate flag. This isn't part of this patch. Some nits below, but otherwise, the series LGTM: Approved-By: Simon Marchi <simon.marchi@efficios.com> > @@ -1922,7 +1922,7 @@ add_dummy_location (struct breakpoint *b, > watchpoint removal from inferior. */ > > static void > -update_watchpoint (struct watchpoint *b, int reparse) > +update_watchpoint (struct watchpoint *b, bool reparse) Update the REPARSE doc, non-zero -> true. > @@ -4216,7 +4216,7 @@ moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) > /* Returns non-zero iff BL is inserted at PC, in address space > ASPACE. */ > > -static int > +static bool > bp_location_inserted_here_p (const struct bp_location *bl, non-zero -> true in the doc Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv2 4/4] gdb: some int to bool conversion in breakpoint.c 2022-10-19 20:03 ` Simon Marchi @ 2022-10-20 15:48 ` Andrew Burgess 0 siblings, 0 replies; 13+ messages in thread From: Andrew Burgess @ 2022-10-20 15:48 UTC (permalink / raw) To: Simon Marchi, gdb-patches Simon Marchi <simark@simark.ca> writes: > On 10/19/22 09:07, Andrew Burgess via Gdb-patches wrote: >> Some int to bool conversion in breakpoint.c. I've only updated the >> function signatures of static functions, but I've updated some >> function local variables throughout the file. >> >> The most interesting change is in unduplicated_should_be_inserted, >> where I've made use of make_scoped_restore to temporarily change the >> bl->duplicate flag. > > This isn't part of this patch. Ooops, I decided to split this into a separate patch and forgot to update the commit message. I fixed this before committing. > > Some nits below, but otherwise, the series LGTM: > > Approved-By: Simon Marchi <simon.marchi@efficios.com> > >> @@ -1922,7 +1922,7 @@ add_dummy_location (struct breakpoint *b, >> watchpoint removal from inferior. */ >> >> static void >> -update_watchpoint (struct watchpoint *b, int reparse) >> +update_watchpoint (struct watchpoint *b, bool reparse) > > Update the REPARSE doc, non-zero -> true. > >> @@ -4216,7 +4216,7 @@ moribund_breakpoint_here_p (const address_space *aspace, CORE_ADDR pc) >> /* Returns non-zero iff BL is inserted at PC, in address space >> ASPACE. */ >> >> -static int >> +static bool >> bp_location_inserted_here_p (const struct bp_location *bl, > > non-zero -> true in the doc Thanks, I made the fixes you suggested, and pushed this series. Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-10-20 15:48 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-05 9:32 [PATCH 0/4] Const args, int->bool, and scoped_restore in breakpoint.c Andrew Burgess 2022-10-05 9:32 ` [PATCH 1/4] gdb: make some bp_location arguments const " Andrew Burgess 2022-10-18 13:46 ` Bruno Larsen 2022-10-05 9:32 ` [PATCH 2/4] gdb: used scoped_restore_frame in update_watchpoint Andrew Burgess 2022-10-05 9:32 ` [PATCH 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted Andrew Burgess 2022-10-05 9:32 ` [PATCH 4/4] gdb: some int to bool conversion in breakpoint.c Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 0/4] Const args, int->bool, and scoped_restore " Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 1/4] gdb: make some bp_location arguments const " Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 2/4] gdb: used scoped_restore_frame in update_watchpoint Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 3/4] gdb: make use of scoped_restore in unduplicated_should_be_inserted Andrew Burgess 2022-10-19 13:07 ` [PATCHv2 4/4] gdb: some int to bool conversion in breakpoint.c Andrew Burgess 2022-10-19 20:03 ` Simon Marchi 2022-10-20 15:48 ` Andrew Burgess
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).