From f5951b9ff8a018c9234656e9b26b372c6b3d238b Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 9 May 2023 10:28:09 -0400 Subject: [PATCH] gdb: add breakpoint::first_loc methods Add convenience first_loc methods to struct breakpoint (const and non-const overloads). A subsequent patch changes the list of locations to be an intrusive_list and makes the actual list private, so these spots would need to change from: b->loc to something ugly like: *b->locations ().begin () That would make the code much heavier and not readable. There is a surprisingly big number of places that access the first location of breakpoints. Whether this is correct, or these spots fail to consider the possibility of multi-location breakpoints, I don't know. But anyhow, I think that using this instead: b->first_loc () conveys the intention better than the other two forms. Change-Id: Ibbefe3e4ca6cdfe570351fe7e2725f2ce11d1e95 Reviewed-By: Andrew Burgess --- gdb/breakpoint.c | 112 ++++++++++++++++++++-------------------- gdb/breakpoint.h | 14 +++++ gdb/elfread.c | 4 +- gdb/infrun.c | 11 ++-- gdb/tracectf.c | 2 +- gdb/tracefile-tfile.c | 2 +- gdb/tracefile.c | 2 +- gdb/tracepoint.c | 2 +- gdb/tui/tui-winsource.c | 2 +- 9 files changed, 82 insertions(+), 69 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 4fcaf007d05..b9fd2c87101 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -4108,7 +4108,7 @@ breakpoint_init_inferior (enum inf_context context) for (breakpoint *b : all_breakpoints_safe ()) { - if (b->has_locations () && b->loc->pspace != pspace) + if (b->has_locations () && b->first_loc ().pspace != pspace) continue; switch (b->type) @@ -5635,7 +5635,7 @@ build_bpstat_chain (const address_space *aspace, CORE_ADDR bp_addr, entire expression, not the individual locations. For read watchpoints, the watchpoints_triggered function has checked all locations already. */ - if (b->type == bp_hardware_watchpoint && bl != b->loc) + if (b->type == bp_hardware_watchpoint && bl != &b->first_loc ()) break; if (!bl->enabled || bl->disabled_by_cond || bl->shlib_disabled) @@ -6162,8 +6162,7 @@ bp_location_condition_evaluator (const struct bp_location *bl) /* Print the LOC location out of the list of B->LOC locations. */ static void -print_breakpoint_location (const breakpoint *b, - struct bp_location *loc) +print_breakpoint_location (const breakpoint *b, const bp_location *loc) { struct ui_out *uiout = current_uiout; @@ -6363,11 +6362,11 @@ print_one_breakpoint_location (struct breakpoint *b, if (loc == NULL && (b->has_locations () && (b->has_multiple_locations () - || !b->loc->enabled || b->loc->disabled_by_cond))) + || !b->first_loc ().enabled || b->first_loc ().disabled_by_cond))) header_of_multiple = true; - if (loc == NULL) - loc = b->loc; + if (loc == NULL && b->has_locations ()) + loc = &b->first_loc (); annotate_record (); @@ -6467,7 +6466,7 @@ print_one_breakpoint_location (struct breakpoint *b, if (!header_of_multiple) print_breakpoint_location (b, loc); if (b->has_locations ()) - *last_loc = b->loc; + *last_loc = &b->first_loc (); } } @@ -6747,8 +6746,8 @@ print_one_breakpoint (breakpoint *b, const bp_location **last_loc, int allflag) && (allflag || (b->has_locations () && (b->has_multiple_locations () - || !b->loc->enabled - || b->loc->disabled_by_cond)))) + || !b->first_loc ().enabled + || b->first_loc ().disabled_by_cond)))) { gdb::optional locations_list; @@ -7745,7 +7744,7 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address) b->enable_state = bp_enabled; /* locspec has to be used or breakpoint_re_set will delete me. */ - b->locspec = new_address_location_spec (b->loc->address, NULL, 0); + b->locspec = new_address_location_spec (b->first_loc ().address, NULL, 0); update_global_location_list_nothrow (UGLL_MAY_INSERT); @@ -7773,7 +7772,7 @@ remove_jit_event_breakpoints (void) { for (breakpoint *b : all_breakpoints_safe ()) if (b->type == bp_jit_event - && b->loc->pspace == current_program_space) + && b->first_loc ().pspace == current_program_space) delete_breakpoint (b); } @@ -7782,7 +7781,7 @@ remove_solib_event_breakpoints (void) { for (breakpoint *b : all_breakpoints_safe ()) if (b->type == bp_shlib_event - && b->loc->pspace == current_program_space) + && b->first_loc ().pspace == current_program_space) delete_breakpoint (b); } @@ -7793,7 +7792,7 @@ remove_solib_event_breakpoints_at_next_stop (void) { for (breakpoint *b : all_breakpoints_safe ()) if (b->type == bp_shlib_event - && b->loc->pspace == current_program_space) + && b->first_loc ().pspace == current_program_space) b->disposition = disp_del_at_next_stop; } @@ -7828,7 +7827,7 @@ create_and_insert_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR add /* Explicitly tell update_global_location_list to insert locations. */ b = create_solib_event_breakpoint_1 (gdbarch, address, UGLL_INSERT); - if (!b->loc->inserted) + if (!b->first_loc ().inserted) { delete_breakpoint (b); return NULL; @@ -8180,17 +8179,18 @@ momentary_breakpoint_from_master (struct breakpoint *orig, std::unique_ptr copy (new_momentary_breakpoint (orig->gdbarch, type, orig->pspace, orig->frame_id, thread)); + const bp_location &orig_loc = orig->first_loc (); copy->loc = copy->allocate_location (); set_breakpoint_location_function (copy->loc); - copy->loc->gdbarch = orig->loc->gdbarch; - copy->loc->requested_address = orig->loc->requested_address; - copy->loc->address = orig->loc->address; - copy->loc->section = orig->loc->section; - copy->loc->pspace = orig->loc->pspace; - copy->loc->probe = orig->loc->probe; - copy->loc->line_number = orig->loc->line_number; - copy->loc->symtab = orig->loc->symtab; + copy->loc->gdbarch = orig_loc.gdbarch; + copy->loc->requested_address = orig_loc.requested_address; + copy->loc->address = orig_loc.address; + copy->loc->section = orig_loc.section; + copy->loc->pspace = orig_loc.pspace; + copy->loc->probe = orig_loc.probe; + copy->loc->line_number = orig_loc.line_number; + copy->loc->symtab = orig_loc.symtab; copy->loc->enabled = loc_enabled; breakpoint *b = add_to_breakpoint_chain (std::move (copy)); @@ -8577,7 +8577,7 @@ code_breakpoint::code_breakpoint (struct gdbarch *gdbarch_, if (locspec_ != nullptr) locspec = std::move (locspec_); else - locspec = new_address_location_spec (this->loc->address, NULL, 0); + locspec = new_address_location_spec (this->first_loc ().address, NULL, 0); filter = std::move (filter_); } @@ -9413,7 +9413,6 @@ ranged_breakpoint::print_it (const bpstat *bs) const bool ranged_breakpoint::print_one (const bp_location **last_loc) const { - struct bp_location *bl = loc; struct value_print_options opts; struct ui_out *uiout = current_uiout; @@ -9427,8 +9426,8 @@ ranged_breakpoint::print_one (const bp_location **last_loc) const by ranged_breakpoint::print_one_detail. */ uiout->field_skip ("addr"); annotate_field (5); - print_breakpoint_location (this, bl); - *last_loc = bl; + print_breakpoint_location (this, &this->first_loc ()); + *last_loc = &this->first_loc (); return true; } @@ -9439,18 +9438,16 @@ void ranged_breakpoint::print_one_detail (struct ui_out *uiout) const { CORE_ADDR address_start, address_end; - struct bp_location *bl = loc; + const bp_location &bl = this->first_loc (); string_file stb; - gdb_assert (bl); - - address_start = bl->address; - address_end = address_start + bl->length - 1; + address_start = bl.address; + address_end = address_start + bl.length - 1; uiout->text ("\taddress range: "); stb.printf ("[%s, %s]", - print_core_address (bl->gdbarch, address_start), - print_core_address (bl->gdbarch, address_end)); + print_core_address (bl.gdbarch, address_start), + print_core_address (bl.gdbarch, address_end)); uiout->field_stream ("addr", stb); uiout->text ("\n"); } @@ -9460,15 +9457,14 @@ ranged_breakpoint::print_one_detail (struct ui_out *uiout) const void ranged_breakpoint::print_mention () const { - struct bp_location *bl = loc; + const bp_location &bl = this->first_loc (); struct ui_out *uiout = current_uiout; - gdb_assert (bl); gdb_assert (type == bp_hardware_breakpoint); uiout->message (_("Hardware assisted ranged breakpoint %d from %s to %s."), - number, paddress (bl->gdbarch, bl->address), - paddress (bl->gdbarch, bl->address + bl->length - 1)); + number, paddress (bl.gdbarch, bl.address), + paddress (bl.gdbarch, bl.address + bl.length - 1)); } /* Implement the "print_recreate" method for ranged breakpoints. */ @@ -9994,7 +9990,7 @@ masked_watchpoint::print_one_detail (struct ui_out *uiout) const gdb_assert (this->has_single_location ()); uiout->text ("\tmask "); - uiout->field_core_addr ("mask", loc->gdbarch, hw_wp_mask); + uiout->field_core_addr ("mask", this->first_loc ().gdbarch, hw_wp_mask); uiout->text ("\n"); } @@ -10304,11 +10300,11 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, scope_breakpoint->frame_id = caller_frame_id; /* Set the address at which we will stop. */ - scope_breakpoint->loc->gdbarch = caller_arch; - scope_breakpoint->loc->requested_address = caller_pc; - scope_breakpoint->loc->address - = adjust_breakpoint_address (scope_breakpoint->loc->gdbarch, - scope_breakpoint->loc->requested_address, + bp_location &loc = scope_breakpoint->first_loc (); + loc.gdbarch = caller_arch; + loc.requested_address = caller_pc; + loc.address + = adjust_breakpoint_address (loc.gdbarch, loc.requested_address, scope_breakpoint->type, current_program_space); } @@ -11601,23 +11597,24 @@ code_breakpoint::say_where () const } else { - if (opts.addressprint || loc->symtab == NULL) + const bp_location &bl = this->first_loc (); + if (opts.addressprint || bl.symtab == nullptr) gdb_printf (" at %ps", styled_string (address_style.style (), - paddress (loc->gdbarch, - loc->address))); - if (loc->symtab != NULL) + paddress (bl.gdbarch, + bl.address))); + if (bl.symtab != NULL) { /* If there is a single location, we can print the location more nicely. */ if (!this->has_multiple_locations ()) { const char *filename - = symtab_to_filename_for_display (loc->symtab); + = symtab_to_filename_for_display (bl.symtab); gdb_printf (": file %ps, line %d.", styled_string (file_name_style.style (), filename), - loc->line_number); + bl.line_number); } else /* This is not ideal, but each location may have a @@ -12631,14 +12628,14 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal) uiout->field_signed ("line", sal2.line); uiout->text ("\n"); - b->loc->line_number = sal2.line; - b->loc->symtab = sym != NULL ? sal2.symtab : NULL; + b->first_loc ().line_number = sal2.line; + b->first_loc ().symtab = sym != NULL ? sal2.symtab : NULL; std::unique_ptr els (new explicit_location_spec ()); els->source_filename = xstrdup (symtab_to_filename_for_display (sal2.symtab)); - els->line_offset.offset = b->loc->line_number; + els->line_offset.offset = b->first_loc ().line_number; els->line_offset.sign = LINE_OFFSET_NONE; b->locspec = std::move (els); @@ -12875,9 +12872,10 @@ code_breakpoint::location_spec_to_sals (location_spec *locspec, && (condition_not_parsed || (this->has_locations () && search_pspace != NULL - && loc->pspace != search_pspace) - || (this->has_locations () && loc->shlib_disabled) - || (this->has_locations () && loc->pspace->executing_startup) + && this->first_loc ().pspace != search_pspace) + || (this->has_locations () && this->first_loc ().shlib_disabled) + || (this->has_locations () + && this->first_loc ().pspace->executing_startup) || enable_state == bp_disabled)) not_found_and_ok = true; @@ -13049,7 +13047,7 @@ breakpoint_re_set_thread (struct breakpoint *b) selected as current, and unless this was a vfork will have a different program space from the original thread. Reset that as well. */ - b->loc->pspace = current_program_space; + b->first_loc ().pspace = current_program_space; } } diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 2a3a5cf57b8..9f49ed62a9a 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -645,6 +645,20 @@ struct breakpoint bool has_multiple_locations () const { return this->loc != nullptr && this->loc->next != nullptr; } + /* Return a reference to the first location of this breakpoint. */ + bp_location &first_loc () + { + gdb_assert (this->has_locations ()); + return *this->loc; + } + + /* Return a reference to the first location of this breakpoint. */ + const bp_location &first_loc () const + { + gdb_assert (this->has_locations ()); + return *this->loc; + } + /* Reevaluate a breakpoint. This is necessary after symbols change (e.g., an executable or DSO was loaded, or the inferior just started). */ diff --git a/gdb/elfread.c b/gdb/elfread.c index 83b20a388c8..799e3b914f8 100644 --- a/gdb/elfread.c +++ b/gdb/elfread.c @@ -977,7 +977,7 @@ elf_gnu_ifunc_resolver_stop (code_breakpoint *b) gdb_assert (frame_id_p (b_return->frame_id)); if (b_return->thread == thread_id - && b_return->loc->requested_address == prev_pc + && b_return->first_loc ().requested_address == prev_pc && b_return->frame_id == prev_frame_id) break; } @@ -1046,7 +1046,7 @@ elf_gnu_ifunc_resolver_return_stop (code_breakpoint *b) func_func = value::allocate (func_func_type); func_func->set_lval (lval_memory); - func_func->set_address (b->loc->related_address); + func_func->set_address (b->first_loc ().related_address); value = value::allocate (value_type); gdbarch_return_value_as_value (gdbarch, func_func, value_type, regcache, diff --git a/gdb/infrun.c b/gdb/infrun.c index efe2c00c489..a9e2a2423b1 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -950,14 +950,14 @@ follow_inferior_reset_breakpoints (void) if (tp->control.step_resume_breakpoint) { breakpoint_re_set_thread (tp->control.step_resume_breakpoint); - tp->control.step_resume_breakpoint->loc->enabled = 1; + tp->control.step_resume_breakpoint->first_loc ().enabled = 1; } /* Treat exception_resume breakpoints like step_resume breakpoints. */ if (tp->control.exception_resume_breakpoint) { breakpoint_re_set_thread (tp->control.exception_resume_breakpoint); - tp->control.exception_resume_breakpoint->loc->enabled = 1; + tp->control.exception_resume_breakpoint->first_loc ().enabled = 1; } /* Reinsert all breakpoints in the child. The user may have set @@ -2551,7 +2551,8 @@ resume_1 (enum gdb_signal sig) user breakpoints at PC to trigger (again) when this hits. */ insert_hp_step_resume_breakpoint_at_frame (get_current_frame ()); - gdb_assert (tp->control.step_resume_breakpoint->loc->permanent); + gdb_assert (tp->control.step_resume_breakpoint->first_loc () + .permanent); tp->step_after_step_resume_breakpoint = step; } @@ -7008,9 +7009,9 @@ process_event_stop_test (struct execution_control_state *ecs) = ecs->event_thread->control.step_resume_breakpoint; if (sr_bp != nullptr - && sr_bp->loc->permanent + && sr_bp->first_loc ().permanent && sr_bp->type == bp_hp_step_resume - && sr_bp->loc->address == ecs->event_thread->prev_pc) + && sr_bp->first_loc ().address == ecs->event_thread->prev_pc) { infrun_debug_printf ("stepped permanent breakpoint, stopped in handler"); delete_step_resume_breakpoint (ecs->event_thread); diff --git a/gdb/tracectf.c b/gdb/tracectf.c index ab513b1fa15..16461711da1 100644 --- a/gdb/tracectf.c +++ b/gdb/tracectf.c @@ -1536,7 +1536,7 @@ ctf_get_traceframe_address (void) = get_tracepoint_by_number_on_target (tpnum); if (tp != nullptr && tp->has_locations ()) - addr = tp->loc->address; + addr = tp->first_loc ().address; } /* Restore the position. */ diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c index 9c1adee11bc..9203bcc2e4b 100644 --- a/gdb/tracefile-tfile.c +++ b/gdb/tracefile-tfile.c @@ -668,7 +668,7 @@ tfile_get_traceframe_address (off_t tframe_offset) tp = get_tracepoint_by_number_on_target (tpnum); /* FIXME this is a poor heuristic if multiple locations. */ if (tp != nullptr && tp->has_locations ()) - addr = tp->loc->address; + addr = tp->first_loc ().address; /* Restore our seek position. */ cur_offset = saved_offset; diff --git a/gdb/tracefile.c b/gdb/tracefile.c index 883ce4bf375..5a2641919f7 100644 --- a/gdb/tracefile.c +++ b/gdb/tracefile.c @@ -412,7 +412,7 @@ tracefile_fetch_registers (struct regcache *regcache, int regno) /* Guess what we can from the tracepoint location. */ gdbarch_guess_tracepoint_registers (gdbarch, regcache, - tp->loc->address); + tp->first_loc ().address); } /* This is the implementation of target_ops method to_has_all_memory. */ diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index 4b775d1a4a2..607595813d0 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -2752,7 +2752,7 @@ get_traceframe_location (int *stepping_frame_p) /* If this is a stepping frame, we don't know which location triggered. The first is as good (or bad) a guess as any... */ *stepping_frame_p = 1; - return t->loc; + return &t->first_loc (); } /* Return the default collect actions of a tracepoint T. */ diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c index 3c4ce501e5e..3fc38b7be23 100644 --- a/gdb/tui/tui-winsource.c +++ b/gdb/tui/tui-winsource.c @@ -640,7 +640,7 @@ tui_source_window_base::update_breakpoint_info mode |= TUI_BP_ENABLED; if (bp->hit_count) mode |= TUI_BP_HIT; - if (bp->loc->cond) + if (bp->first_loc ().cond) mode |= TUI_BP_CONDITIONAL; if (bp->type == bp_hardware_breakpoint) mode |= TUI_BP_HARDWARE; -- 2.30.2