[PATCH] Show enabled locations with disabled breakpoint parent as "y-" (Re: [PATCH 0/2] info breakpoints improvements)

[Pedro Alves <pedro@palves.net>]

On 2022-05-24 15:17, Pedro Alves wrote:
> On 2022-05-24 15:11, Andreas Schwab wrote:
>> On Mai 24 2022, Eli Zaretskii via Gdb-patches wrote:
>>
>>>> Date: Tue, 24 May 2022 14:50:01 +0100
>>>> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
>>>> From: Pedro Alves <pedro@palves.net>
>>>>
>>>>>> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
>>>>>
>>>>> That's what I thought.  But then why not "propagate" the "n" of the
>>>>> disabled breakpoint to all of its locations?  
>>>>
>>>> Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
>>>> state of the individual locations.
>>>
>>> I don't understand why would that be lost.  I'm not proposing to
>>> actually disable each location, I propose to _display_ them as
>>> disabled in that case.
>>
>> I think it would be better in that case to omit the column or display it
>> as "-", to indicate that it is ignored right now.
>>
> 
> Yes, we could print them differently, preserving the user-specified enable/disable
> state.  We print them in uppercase N/Y when the location is disabled due to an invalid
> condition already, for example.

Here's a patch.  Code, manual, NEWS, testcase, all included.

From 9675cd4b4948c6ca8f62d8c2cbadc42d4f94a1f9 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 24 May 2022 19:30:10 +0100
Subject: [PATCH] Show enabled locations with disabled breakpoint parent as
 "y-"

Currently, breakpoint locations that are enabled while their parent
breakpoint is disabled are displayed with "y" in the Enb colum of
"info breakpoints":

 (gdb) info breakpoints
 Num     Type           Disp Enb Address            What
 1       breakpoint     keep n   <MULTIPLE>
 1.1                         y   0x00000000000011b6 in ...
 1.2                         y   0x00000000000011c2 in ...
 1.3                         n   0x00000000000011ce in ...

Such locations won't trigger a break, so to avoid confusion, show "y-"
instead.  For example:

 (gdb) info breakpoints
 Num     Type           Disp Enb Address            What
 1       breakpoint     keep n   <MULTIPLE>
 1.1                         y-  0x00000000000011b6 in ...
 1.2                         y-  0x00000000000011c2 in ...
 1.3                         n   0x00000000000011ce in ...

The "-" sign is inspired on how the TUI represents breakpoints on the
left side of the source window, with "b-" for a disabled breakpoint.

Change-Id: I9952313743c51bf21b4b380c72360ef7d4396a09
---
 gdb/doc/gdb.texinfo                       | 13 ++++++++
 gdb/NEWS                                  | 10 ++++++
 gdb/breakpoint.c                          | 37 +++++++++++++++++++----
 gdb/testsuite/gdb.cp/ena-dis-br-range.exp | 28 ++++++++---------
 4 files changed, 68 insertions(+), 20 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e5c1ee33aac..68679982919 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4668,6 +4668,19 @@ in which case @value{GDBN} acts on all the locations in the range (inclusive).
 Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
 all of the locations that belong to that breakpoint.
 
+Locations that are enabled while their parent breakpoint is disabled
+won't trigger a break, and are denoted by @code{y-} in the @code{Enb}
+column.  For example:
+
+@smallexample
+(@value{GDBP}) info breakpoints
+Num     Type           Disp Enb Address            What
+1       breakpoint     keep n   <MULTIPLE>
+1.1                         y-  0x00000000000011b6 in ...
+1.2                         y-  0x00000000000011c2 in ...
+1.3                         n   0x00000000000011ce in ...
+@end smallexample
+
 @cindex pending breakpoints
 It's quite common to have a breakpoint inside a shared library.
 Shared libraries can be loaded and unloaded explicitly,
diff --git a/gdb/NEWS b/gdb/NEWS
index a72fee81550..52ffdc4c83a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,16 @@
 
 *** Changes since GDB 12
 
+* "info breakpoints" now displays enabled breakpoint locations of
+  disabled breakpoints as in the "y-" state.  For example:
+
+   (gdb) info breakpoints
+   Num     Type           Disp Enb Address            What
+   1       breakpoint     keep n   <MULTIPLE>
+   1.1                         y-  0x00000000000011b6 in ...
+   1.2                         y-  0x00000000000011c2 in ...
+   1.3                         n   0x00000000000011ce in ...
+
 * Support for Thread Local Storage (TLS) variables on FreeBSD arm and
   aarch64 architectures.
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index eac6410a578..ed932a19ed7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6234,13 +6234,38 @@ print_one_breakpoint_location (struct breakpoint *b,
 
   /* 4 */
   annotate_field (3);
-  /* For locations that are disabled because of an invalid condition,
-     display "N*" on CLI, where "*" refers to a footnote below the
-     table.  For MI, simply display a "N" without a footnote.  */
-  const char *N = (uiout->is_mi_like_p ()) ? "N" : "N*";
   if (part_of_multiple)
-    uiout->field_string ("enabled", (loc->disabled_by_cond ? N
-				     : (loc->enabled ? "y" : "n")));
+    {
+      /* For locations that are disabled because of an invalid
+	 condition, display "N*" on the CLI, where "*" refers to a
+	 footnote below the table.  For MI, simply display a "N"
+	 without a footnote.  On the CLI, for enabled locations whose
+	 breakpoint is disabled, display "y-".  */
+      auto get_enable_state = [uiout, loc] () -> const char *
+        {
+	  if (uiout->is_mi_like_p ())
+	    {
+	      if (loc->disabled_by_cond)
+		return "N";
+	      else if (!loc->enabled)
+		return "n";
+	      else
+		return "y";
+	    }
+	  else
+	    {
+	      if (loc->disabled_by_cond)
+		return "N*";
+	      else if (!loc->enabled)
+		return "n";
+	      else if (!breakpoint_enabled (loc->owner))
+		return "y-";
+	      else
+		return "y";
+	    }
+	};
+      uiout->field_string ("enabled", get_enable_state ());
+    }
   else
     uiout->field_fmt ("enabled", "%c", bpenables[(int) b->enable_state]);
 
diff --git a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
index 782756e9ab1..83c9838aa42 100644
--- a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
+++ b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
@@ -44,10 +44,10 @@ proc make_info_breakpoint_reply_re {b1 b2 b21 b22 b23 b24} {
 		"1${ws}breakpoint     keep ${b1}${ws}.* in marker\\(\\) at .*" \
 		"${ws}breakpoint already hit 1 time.*" \
 		"2${ws}breakpoint${ws}keep${ws}${b2}${ws}<MULTIPLE>.*" \
-		"2.1${ws}${b21}.*" \
-		"2.2${ws}${b22}.*" \
-		"2.3${ws}${b23}.*" \
-		"2.4${ws}${b24}.*" \
+		"2.1${ws}${b21}${ws}.*" \
+		"2.2${ws}${b22}${ws}.*" \
+		"2.3${ws}${b23}${ws}.*" \
+		"2.4${ws}${b24}${ws}.*" \
 	       ]
 }
 
@@ -74,18 +74,18 @@ proc test_enable_disable {cmd b1 b2 b21 b22 b23 b24} {
 test_enable_disable "disable 1" n y y y y y
 test_enable_disable "enable  1" y y y y y y
 
-# Check that we can disable/disable a breakpoint with multiple
+# Check that we can disable/enable a breakpoint with multiple
 # locations.
-test_enable_disable "disable 2" y n y y y y
-test_enable_disable "enable  2" y y y y y y
+test_enable_disable "disable 2" y n y- y- y- y-
+test_enable_disable "enable  2" y y y  y  y  y
 
-# Check that we can disable/disable a range of breakpoints.
-test_enable_disable "disable 1-2" n n y y y y
-test_enable_disable "enable 1-2"  y y y y y y
+# Check that we can disable/enable a range of breakpoints.
+test_enable_disable "disable 1-2" n n y- y- y- y-
+test_enable_disable "enable 1-2"  y y y  y  y  y
 
-# Check that we can disable/disable a list of breakpoints.
-test_enable_disable "disable 1 2" n n y y y y
-test_enable_disable "enable 1 2"  y y y y y y
+# Check that we can disable/enable a list of breakpoints.
+test_enable_disable "disable 1 2" n n y- y- y- y-
+test_enable_disable "enable 1 2"  y y y  y  y  y
 
 # Check that we can disable/enable a single location breakpoint.
 test_enable_disable "disable 2.2" y y y n y y
@@ -100,7 +100,7 @@ test_enable_disable "enable  2.2-3" y y y y y y
 test_enable_disable "disable 2.2-2" y y y n y y
 test_enable_disable "enable  2.2-2" y y y y y y
 
-# Check that we can disable/disable a list of breakpoints that
+# Check that we can disable/enable a list of breakpoints that
 # includes some elements with location ranges and others without.
 test_enable_disable "disable 1 2.1 2.3-4" n y n y n n
 test_enable_disable "enable  1 2.1 2.3-4" y y y y y y

base-commit: e8123c847f61c7458200b349615c47e9df17a0ed
-- 
2.36.0