gdb: add breakpoint::first_loc methods
authorSimon Marchi <simon.marchi@efficios.com>
Tue, 9 May 2023 14:28:09 +0000 (10:28 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Thu, 25 May 2023 12:47:00 +0000 (08:47 -0400)
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 <aburgess@redhat.com>
gdb/breakpoint.c
gdb/breakpoint.h
gdb/elfread.c
gdb/infrun.c
gdb/tracectf.c
gdb/tracefile-tfile.c
gdb/tracefile.c
gdb/tracepoint.c
gdb/tui/tui-winsource.c

index 4fcaf007d05abf8e284fab8a1cd6595b7359069b..b9fd2c8710105a05fa0bad8ba0397ca897437590 100644 (file)
@@ -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<ui_out_emit_list> 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<breakpoint> 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<explicit_location_spec> 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;
     }
 }
 
index 2a3a5cf57b8376c811fa0aa5b76b23981f511100..9f49ed62a9ad9131281ec9df6c4c7b3bdddbf1b5 100644 (file)
@@ -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).  */
index 83b20a388c8b2c5b0a672ff0feeabeb9921c178c..799e3b914f839f402c7fbea901316ae48df10cfe 100644 (file)
@@ -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,
index efe2c00c489a60c98f323dee92ae97f2598c6f22..a9e2a2423b175af0717992964b0b6bcf4811e354 100644 (file)
@@ -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);
index ab513b1fa158a539b80d7a2dd722aaff69e56bff..16461711da181b17acf76e4f71cbdfa0878f049c 100644 (file)
@@ -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.  */
index 9c1adee11bc37b4407607bde27bf22da373dd621..9203bcc2e4bb419468e2930760898f67d34c9b2f 100644 (file)
@@ -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;
index 883ce4bf375ac215e75f368ba2c87a53897eafc1..5a2641919f745dd7f8afaa19a4e7d5cce70af3fd 100644 (file)
@@ -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.  */
index 4b775d1a4a2e0ce2c3baf3c4c5b3b73baa47e7a2..607595813d0cc473e18fca6362fd791ae4072ea4 100644 (file)
@@ -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.  */
index 3c4ce501e5eef97e42f83313c1f21019bad7d71b..3fc38b7be233231d11b628ff3239db8faab34d26 100644 (file)
@@ -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;