[PATCH 05/12] gdb: add breakpoint::first_loc methods

[Simon Marchi <simon.marchi@efficios.com>]

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 become something ugly like:

  -b->loc
  +*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
---
 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 fefe18644cf9..e9b6574bba77 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4106,7 +4106,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)
@@ -5633,7 +5633,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)
@@ -6160,8 +6160,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;
 
@@ -6361,11 +6360,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 ();
 
@@ -6465,7 +6464,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 ();
 	}
     }
 
@@ -6745,8 +6744,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;
 
@@ -7743,7 +7742,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);
 
@@ -7771,7 +7770,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);
 }
 
@@ -7780,7 +7779,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);
 }
 
@@ -7791,7 +7790,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;
 }
 
@@ -7826,7 +7825,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;
@@ -8178,17 +8177,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));
+  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));
@@ -8575,7 +8575,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_);
 }
 
@@ -9411,7 +9411,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;
 
@@ -9425,8 +9424,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;
 }
@@ -9437,18 +9436,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");
 }
@@ -9458,15 +9455,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.  */
@@ -9992,7 +9988,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");
 }
 
@@ -10302,11 +10298,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);
 	}
@@ -11599,23 +11595,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
@@ -12629,14 +12626,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);
@@ -12873,9 +12870,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;
 
@@ -13047,7 +13045,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 2a3a5cf57b83..9f49ed62a9ad 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 7169ae8808a8..2d4f23416245 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -974,7 +974,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;
     }
@@ -1043,7 +1043,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 efe2c00c489a..a9e2a2423b17 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 ab513b1fa158..16461711da18 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 9c1adee11bc3..9203bcc2e4bb 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 883ce4bf375a..5a2641919f74 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 4a9d8491a54b..99d175d32e42 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2750,7 +2750,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 3c4ce501e5ee..3fc38b7be233 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.40.1