[PATCHv3,5/9] gdb: don't display inferior list for pending breakpoints

Message ID d84f0fe8f6afeb470b89ba049221c2c83bc5c7c4.1685479504.git.aburgess@redhat.com
State New
Headers
Series thread-specific breakpoints in just some inferiors |

Commit Message

Andrew Burgess May 30, 2023, 8:46 p.m. UTC
  I noticed that in the 'info breakpoints' output, GDB sometimes prints
the inferior list for pending breakpoints, this doesn't seem right to
me.  A pending breakpoint has no locations (at least, as far as we
display things in the 'info breakpoints' output), so including an
inferior list seems odd.

Here's what I see right now:

  (gdb) info breakpoint 5
  Num     Type           Disp Enb Address            What
  5       breakpoint     keep y   <PENDING>          foo inf 1
  (gdb)

It's the 'inf 1' at the end of the line that I'm objecting too.

To trigger this behaviour we need to be in a multi-inferior debug
session.  The breakpoint must have been non-pending at some point in
the past, and so have a location assigned to it.

The breakpoint becomes pending again as a result of a shared library
being unloaded.  When this happens the location itself is marked
pending (via bp_location::shlib_disabled).

In print_one_breakpoint_location, in order to print the inferior list
we check that the breakpoint has a location, and that we have multiple
inferiors, but we don't check if the location itself is pending.

This commit adds that check, which means the output is now:

  (gdb) info breakpoint 5
  Num     Type           Disp Enb Address            What
  5       breakpoint     keep y   <PENDING>          foo
  (gdb)

Which I think makes more sense.
---
 gdb/breakpoint.c                         |   2 +-
 gdb/testsuite/gdb.multi/pending-bp-lib.c |  22 +++++
 gdb/testsuite/gdb.multi/pending-bp.c     |  66 +++++++++++++
 gdb/testsuite/gdb.multi/pending-bp.exp   | 115 +++++++++++++++++++++++
 4 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.multi/pending-bp-lib.c
 create mode 100644 gdb/testsuite/gdb.multi/pending-bp.c
 create mode 100644 gdb/testsuite/gdb.multi/pending-bp.exp
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4eb40c01274..6a76bfbefe8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6452,7 +6452,7 @@  print_one_breakpoint_location (struct breakpoint *b,
 	}
     }
 
-  if (loc != NULL && !header_of_multiple)
+  if (loc != NULL && !header_of_multiple && !loc->shlib_disabled)
     {
       std::vector<int> inf_nums;
       int mi_only = 1;
diff --git a/gdb/testsuite/gdb.multi/pending-bp-lib.c b/gdb/testsuite/gdb.multi/pending-bp-lib.c
new file mode 100644
index 00000000000..15d1b9833dd
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/pending-bp-lib.c
@@ -0,0 +1,22 @@ 
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int global_var = 0;
+
+void
+foo (int arg)
+{
+  global_var = arg;
+}
diff --git a/gdb/testsuite/gdb.multi/pending-bp.c b/gdb/testsuite/gdb.multi/pending-bp.c
new file mode 100644
index 00000000000..ca8a3c20b72
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/pending-bp.c
@@ -0,0 +1,66 @@ 
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <dlfcn.h>
+#include <stdlib.h>
+
+void
+breakpt ()
+{
+  /* Nothing.  */
+}
+
+volatile int global_counter = 0;
+
+volatile int call_count = 1;
+
+int
+main (void)
+{
+  void *handle;
+  void (*func)(int);
+
+  /* Some filler work so that we don't initially stop on the breakpt call
+     below.  */
+  ++global_counter;
+
+  breakpt ();	/* Break before open.  */
+
+  /* Now load the shared library.  */
+  handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+  if (handle == NULL)
+    abort ();
+
+  breakpt ();	/* Break after open.  */
+
+  /* Find the function symbol.  */
+  func = (void (*)(int)) dlsym (handle, "foo");
+
+  for (; call_count > 0; --call_count)
+    {
+      /* Call the library function.  */
+      func (1);
+    }
+
+  breakpt ();	/* Break before close.  */
+
+  /* Unload the shared library.  */
+  if (dlclose (handle) != 0)
+    abort ();
+
+  breakpt ();	/* Break after close.  */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/pending-bp.exp b/gdb/testsuite/gdb.multi/pending-bp.exp
new file mode 100644
index 00000000000..4a65f79cfc4
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/pending-bp.exp
@@ -0,0 +1,115 @@ 
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Tests related to pending breakpoints in a multi-inferior environment.
+
+require allow_shlib_tests !use_gdb_stub
+
+standard_testfile
+
+set libname $testfile-lib
+set srcfile_lib $srcdir/$subdir/$libname.c
+set binfile_lib [standard_output_file $libname.so]
+
+if { [gdb_compile_shlib $srcfile_lib $binfile_lib {}] != "" } {
+    untested "failed to compile shared library 1"
+    return -1
+}
+
+set binfile_lib_target [gdb_download_shlib $binfile_lib]
+
+if { [build_executable "failed to prepare" $testfile $srcfile \
+	  [list debug \
+	       additional_flags=-DSHLIB_NAME=\"$binfile_lib_target\" \
+	       shlib_load pthreads]] } {
+    return -1
+}
+
+# Used to override delete_breakpoints when we don't want breakpoints
+# deleted.
+proc do_nothing {} {}
+
+# Start two inferiors, both running the same test binary.  The arguments
+# INF_1_STOP and INF_2_STOP are source code patterns that are passed to
+# gdb_get_line_number to figure out where each inferior should be stopped.
+#
+# This proc does a clean_restart and leaves inferior 2 selected.  Also the
+# 'breakpoint pending' flag is enabled, so pending breakpoints can be created
+# without GDB prompting the user.
+proc do_test_setup { inf_1_stop inf_2_stop } {
+    clean_restart ${::binfile}
+
+    gdb_locate_shlib $::binfile_lib
+
+    if ![runto_main] {
+	return false
+    }
+
+    gdb_breakpoint [gdb_get_line_number ${inf_1_stop}] temporary
+    gdb_continue_to_breakpoint "move inferior 1 into position"
+
+    gdb_test "add-inferior -exec ${::binfile}" \
+	"Added inferior 2.*" "add inferior 2"
+    gdb_test "inferior 2" "Switching to inferior 2 .*" "switch to inferior 2"
+
+    with_override delete_breakpoints do_nothing {
+	if {![runto_main]} {
+	    return false
+	}
+    }
+
+    gdb_breakpoint [gdb_get_line_number ${inf_2_stop}] temporary
+    gdb_continue_to_breakpoint "move inferior 2 into position"
+
+    gdb_test_no_output "set breakpoint pending on"
+
+    return true
+}
+
+# Check that when a breakpoint is in the pending state, but that breakpoint
+# does have some locations (those locations themselves are pending), GDB
+# doesn't display the inferior list in the 'info breakpoints' output.
+proc_with_prefix test_no_inf_display {} {
+    do_test_setup "Break before close" "Break before open"
+
+    # Create a breakpoint on 'foo'.  This will find a location in inferior 1,
+    # but will not find a location in inferior 2 -- the shared library is not
+    # yet loaded in that inferior.
+    gdb_breakpoint "foo"
+    set bpnum [get_integer_valueof "\$bpnum" "*INVALID*" \
+		   "get foo breakpoint number"]
+
+    # Check the 'info breakpoints' output.  Notice we display the inferior
+    # list at the end of the breakpoint line.
+    gdb_test "info breakpoint $bpnum" \
+	"$bpnum\\s+breakpoint\\s+keep\\s+y\\s+$::hex\\s+<foo\[^>\]*>\\s+inf 1" \
+	"check info breakpoints while breakpoint is inserted"
+
+    # Ensure we are in inferior 1, move the inferior forward until the shared
+    # library has been unloaded.  The breakpoint on 'foo' will return to the
+    # pending state.
+    gdb_test "inferior 1" "Switching to inferior 1 .*"
+    gdb_breakpoint [gdb_get_line_number "Break after close"] temporary
+    gdb_continue_to_breakpoint "after close library"
+
+    # Check the 'info breakpoints' output, check there is no 'inf 1' at the
+    # end of the breakpoint line.
+    gdb_test "info breakpoint $bpnum" \
+	"$bpnum\\s+breakpoint\\s+keep\\s+y\\s+<PENDING>\\s+foo" \
+	"check info breakpoints while breakpoint is pending"
+}
+
+# Run all the tests.
+test_no_inf_display