gdb: prevent assertion after 'set debug breakpoint on'

Turns out that using 'set debug breakpoint on' will trigger an
assertion for 'catch' style breakpoints, e.g.:

  (gdb) file /tmp/hello.x
  Reading symbols from /tmp/hello.x...
  (gdb) catch exec
  Catchpoint 1 (exec)
  (gdb) set debug breakpoint on
  (gdb) start
  [breakpoint] dump_condition_tokens: Tokens: { INFERIOR: "1" }
  Temporary breakpoint 2 at 0x401198: file /tmp/hello.c, line 18.
  [breakpoint] update_global_location_list: insert_mode = UGLL_MAY_INSERT
  Starting program: /tmp/hello.x
  [breakpoint] update_global_location_list: insert_mode = UGLL_MAY_INSERT
  ../../gdb-16.1/gdb/gdbarch-gen.c:1764: internal-error: gdbarch_addr_bit: Assertion `gdbarch != NULL' failed.
  .... etc ...

The problem is that catch breakpoints don't set the
bp_location::gdbarch member variable, they a "dummy" location added
with a call to add_dummy_location (breakpoint.c).

The breakpoint_location_address_str function (which is only used for
breakpoint debug output) relies on bp_location::gdbarch being set in
order to call the paddress function.

I considered trying to ensure that the bp_location::gdbarch variable
is always set to sane value.  For example, in add_dummy_location I
tried copying the gdbarch from the breakpoint object, and this does
work for the catchpoint case, but for some of the watchpoint cases,
even the breakpoint object has no gdbarch value set.

Now this seemed a little suspect, but, the more I thought about it, I
wondered if "fixing" the gdbarch was allowing me to solve the wrong
problem.

If the gdbarch was set, then this would allow us to print the address
field of the bp_location, which is going to be 0, after all, as this
is a dummy location, which has no address.

But does it really make sense to print the address 0?  For some
targets, 0 is a valid address.  But that wasn't an address we actually
selected, it's just the default value for dummy locations.

And we already have a helper function bl_address_is_meaningful, which
returns false for dummy locations.

So, I propose that in breakpoint_location_address_str, we use
bl_address_is_meaningful to detect dummy locations, and skip the
address printing code in that case.

For testing, I temporarily changed insert_bp_location so that
breakpoint_location_address_str was always called, even when
breakpoint debugging was off.  I then ran the whole testsuite.
Without the fixes included in this commit I saw lots of assertion
failures, but with the fixes from this commit in place, I now see no
assertion failures.

I've added a new test which reveals the original assertion failure.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3fd7f2f099fa2cae788fe497ddf8fa1338497444..dab52f1fae720828786b2a8938bb95c8251659a6 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -254,14 +254,22 @@ DIAGNOSTIC_POP
 static std::string
 breakpoint_location_address_str (const bp_location *bl)
 {
-  std::string str = string_printf ("Breakpoint %d (%s) at address %s",
+  std::string str = string_printf ("Breakpoint %d (%s) ",
                                   bl->owner->number,
-                                  host_address_to_string (bl),
-                                  paddress (bl->gdbarch, bl->address));
+                                  host_address_to_string (bl));
 
-  std::string loc_string = bl->to_string ();
-  if (!loc_string.empty ())
-    str += string_printf (" %s", loc_string.c_str ());
+  if (bl_address_is_meaningful (bl))
+    {
+      gdb_assert (bl->gdbarch != nullptr);
+      str += string_printf ("at address %s",
+                           paddress (bl->gdbarch, bl->address));
+
+      std::string loc_string = bl->to_string ();
+      if (!loc_string.empty ())
+       str += string_printf (" %s", loc_string.c_str ());
+    }
+  else
+    str += "with dummy location";
 
   return str;
 }

diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 6b421f2f8d48396eb5126578b5fd3a05dd41eaa0..93411122e9079925d4c828124a9624817a8f82c2 100644 (file)
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -425,7 +425,9 @@ public:
      simplicity is more important than memory usage for breakpoints.  */
 
   /* Architecture associated with this location's address.  May be
-     different from the breakpoint architecture.  */
+     different from the breakpoint architecture.  Not every location has
+     an address (e.g. see add_dummy_location), so not every location has
+     an associated gdbarch -- this can be NULL for a valid location.  */
   struct gdbarch *gdbarch = NULL;
 
   /* The program space associated with this breakpoint location

diff --git a/gdb/testsuite/gdb.base/break-dbg.cc b/gdb/testsuite/gdb.base/break-dbg.cc
new file mode 100644 (file)
index 0000000..642ded0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-dbg.cc
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+volatile int global_var = 0;
+
+int
+foo ()
+{
+  return global_var;
+}
+
+int
+main ()
+{
+  int res = foo ();
+  return res;
+}

diff --git a/gdb/testsuite/gdb.base/break-dbg.exp b/gdb/testsuite/gdb.base/break-dbg.exp
new file mode 100644 (file)
index 0000000..3652b8e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-dbg.exp
@@ -0,0 +1,70 @@
+# Copyright 2025 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/>.  */
+
+# Some basic testing of 'set debug breakpoint on'.  At one point a bug
+# meant that some breakpoints would immediately trigger a segfault if
+# GDB tried to run with breakpoint debugging turned on.
+#
+# Test is compiled as C++ only so 'catch catch/throw/rethrow' have a
+# something to do.  The original bug would trigger for any 'catch'
+# style breakpoint, so C++ isn't really a hard requirement.
+
+standard_testfile .cc
+
+require allow_cplus_tests
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+         {debug c++}] } {
+    return
+}
+
+if {![runto_main]} {
+    return
+}
+
+gdb_test "catch catch" "^Catchpoint $decimal \\(catch\\)"
+gdb_test "catch throw" "^Catchpoint $decimal \\(throw\\)"
+gdb_test "catch rethrow" "^Catchpoint $decimal \\(rethrow\\)"
+
+gdb_test "catch exec" "^Catchpoint $decimal \\(exec\\)"
+gdb_test "catch fork" "^Catchpoint $decimal \\(fork\\)"
+gdb_test "catch vfork" "^Catchpoint $decimal \\(vfork\\)"
+
+gdb_test "catch load" "^Catchpoint $decimal \\(load\\)"
+gdb_test "catch unload" "^Catchpoint $decimal \\(unload\\)"
+
+gdb_test "catch signal" "^Catchpoint $decimal \\(standard signals\\)"
+gdb_test "catch syscall" "^Catchpoint $decimal \\(any syscall\\)"
+
+gdb_test "watch -l global_var" "\[Ww]atchpoint $decimal: -location global_var"
+
+gdb_test_no_output "set debug breakpoint on"
+
+set saw_bp_debug_line false
+gdb_test_multiple "step" "" {
+    -re "^step\r\n" {
+       exp_continue
+    }
+    -re "^\\\[breakpoint\\\] \[^\r\n\]+\r\n" {
+       set saw_bp_debug_line true
+       exp_continue
+    }
+    -re "^$gdb_prompt $" {
+       gdb_assert { $saw_bp_debug_line } $gdb_test_name
+    }
+    -re "^\[^\r\n\]*\r\n" {
+       exp_continue
+    }
+}