[RFC] Improve handling of breakpoints with multiple locations.

Message ID 20200523211132.18980-1-philippe.waroquiers@skynet.be
State New
Headers show
Series
  • [RFC] Improve handling of breakpoints with multiple locations.
Related show

Commit Message

Simon Marchi via Gdb-patches May 23, 2020, 9:11 p.m.
(RFC to get comments on the idea before updating tests and user manual).

Currently, when a breakpoint that has multiple locations is reached,
GDB prints:
  Thread 1 "zeoes" hit Breakpoint 1, some_func () at somefunc1.c:5

This patch changes the message so that bkpt_print_id prints the precise
encountered breakpoint:
  Thread 1 "zeoes" hit Breakpoint 1.2, some_func () at somefunc1.c:5

Note that bkpt_print_it thus (optionally) prints a new table field "locno":
  locno is printed when the breakpoint has more than one location.

According to the GDB user manual node 'GDB/MI Development and Front Ends',
it is ok to add new fields in the output of a command without
changing the MI version.

Also, when a breakpoint is reached, the convenience variables
$bkptno and $locno are set to the reached breakpoint number
and location number.

$bkptno and $locno can a.o. be used in breakpoint commands,
to disable the specific encountered breakpoint.

In case the breakpoint has only one location, $locno is still set to
the value 1, so as to allow a command such as:
  disable $bkptno.$locno
even when the breakpoint has only one location.

This also fixes a strange behaviour: when a breakpoint X has only
one location,
  enable|disable X.1
is accepted but transforms the breakpoint in a MULTIPLE locations
breakpoint having only one location.

Still to do:
  * user manual
  * NEWS
  * add and update tests to new output.

gdb/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* breakpoint.c (has_multiple_locations): New function.
	(bpstat_num): Replace comment by 'See Breakpoint.h.'.
	(bpstat_locno): New function.
	(bkpt_print_it): Print the locno.
	(enable_disable_command): Avoid that enable|disable y.1
	transforms y into a MULTIPLE locations breakpoint.
	* breakpoint.h (bpstat_locno): New function.
	* infrun.c (print_stop_location): Initialize convenience vars
	$bkptno and $locno.
---
 gdb/breakpoint.c | 78 ++++++++++++++++++++++++++++++++++++++++--------
 gdb/breakpoint.h |  5 ++++
 gdb/infrun.c     | 13 +++++++-
 3 files changed, 82 insertions(+), 14 deletions(-)

-- 
2.20.1

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index aead882acd..b478593fde 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -656,6 +656,21 @@  get_breakpoint (int num)
   return NULL;
 }
 
+/* Return TRUE if NUM refer to an existing breakpoint that has
+   multiple locations.  */
+
+static bool
+has_multiple_locations (int num)
+{
+  struct breakpoint *b;
+
+  ALL_BREAKPOINTS (b)
+    if (b->number == num)
+      return b->loc != nullptr && b->loc->next != nullptr;
+
+  return false;
+}
+
 
 
 /* Mark locations as "conditions have changed" in case the target supports
@@ -4229,15 +4244,7 @@  bpstat_explains_signal (bpstat bsp, enum gdb_signal sig)
   return false;
 }
 
-/* Put in *NUM the breakpoint number of the first breakpoint we are
-   stopped at.  *BSP upon return is a bpstat which points to the
-   remaining breakpoints stopped at (but which is not guaranteed to be
-   good for anything but further calls to bpstat_num).
-
-   Return 0 if passed a bpstat which does not indicate any breakpoints.
-   Return -1 if stopped at a breakpoint that has been deleted since
-   we set it.
-   Return 1 otherwise.  */
+/* See breakpoint.h.  */
 
 int
 bpstat_num (bpstat *bsp, int *num)
@@ -4259,6 +4266,39 @@  bpstat_num (bpstat *bsp, int *num)
   return 1;
 }
 
+/* See breakpoint.h  */
+
+int
+bpstat_locno (bpstat bs)
+{
+  const struct breakpoint *b = bs->breakpoint_at;
+  const struct bp_location *bl = bs->bp_location_at;
+
+  int locno = 0;
+
+  if (b != nullptr && b->loc->next != nullptr)
+    {
+      const bp_location *bl_i;
+
+      for (bl_i = b->loc;
+	   bl_i != bl && bl_i->next != nullptr;
+	   bl_i = bl_i->next)
+	locno++;
+
+      if (bl_i == bl)
+	locno++;
+      else
+	{
+	  warning (_("location number not found for breakpoint %d address %p."),
+		   b->number, paddress (bl->gdbarch, bl->address));
+	  locno = 0;
+	}
+    }
+
+  return locno;
+}
+
+
 /* See breakpoint.h.  */
 
 void
@@ -12455,13 +12495,21 @@  bkpt_print_it (bpstat bs)
 			   async_reason_lookup (EXEC_ASYNC_BREAKPOINT_HIT));
       uiout->field_string ("disp", bpdisp_text (b->disposition));
     }
+
+  int locno = bpstat_locno (bs);
+
   if (bp_temp)
-    uiout->message ("Temporary breakpoint %pF, ",
+    uiout->message ("Temporary breakpoint %pF",
 		    signed_field ("bkptno", b->number));
   else
-    uiout->message ("Breakpoint %pF, ",
+    uiout->message ("Breakpoint %pF",
 		    signed_field ("bkptno", b->number));
 
+  if (locno == 0)
+    uiout->message (", ");
+  else
+    uiout->message (".%pF, ",
+		    signed_field ("locno", locno));
   return PRINT_SRC_AND_LOC;
 }
 
@@ -14243,9 +14291,13 @@  enable_disable_command (const char *args, int from_tty, bool enable)
 	  extract_bp_number_and_location (num, bp_num_range, bp_loc_range);
 
 	  if (bp_loc_range.first == bp_loc_range.second
-	      && bp_loc_range.first == 0)
+	      && (bp_loc_range.first == 0
+		  || (bp_loc_range.first == 1
+		      && bp_num_range.first == bp_num_range.second
+		      && !has_multiple_locations (bp_num_range.first))))
 	    {
-	      /* Handle breakpoint ids with formats 'x' or 'x-z'.  */
+	      /* Handle breakpoint ids with formats 'x' or 'x-z'
+		 or 'y.1' where y has only one location.  */
 	      map_breakpoint_number_range (bp_num_range,
 					   enable
 					   ? enable_breakpoint
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 347aeb75f3..eaf6d86848 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1091,6 +1091,11 @@  extern enum print_stop_action bpstat_print (bpstat, int);
    Return 1 otherwise.  */
 extern int bpstat_num (bpstat *, int *);
 
+/* If BS indicates a breakpoint and this breakpoint has several locations,
+   return the location number of BS, otherwise return 0.  */
+
+extern int bpstat_locno (bpstat bs);
+
 /* Perform actions associated with the stopped inferior.  Actually, we
    just use this for breakpoint commands.  Perhaps other actions will
    go here later, but this is executed at a late time (from the
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 95fc3bfe45..8b84c6e22e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8349,7 +8349,18 @@  print_stop_location (struct target_waitstatus *ws)
      LOCATION: Print only location
      SRC_AND_LOC: Print location and source line.  */
   if (do_frame_printing)
-    print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
+    {
+      const struct breakpoint *b = tp->control.stop_bpstat->breakpoint_at;
+
+      if (b != nullptr)
+	{
+	  int locno = bpstat_locno (tp->control.stop_bpstat);
+	  set_internalvar_integer (lookup_internalvar ("bkptno"), b->number);
+	  set_internalvar_integer (lookup_internalvar ("locno"),
+				   (locno > 0 ? locno : 1));
+	}
+      print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
+    }
 }
 
 /* See infrun.h.  */