gdb: assign a valid section in convert_address_location_to_sals

The convert_address_location_to_sals function builds a symtab_and_line
from an explicit pc. Unless overlay debugging is enabled, the sal does not
contain a valid section (as find_pc_overlay will simply return nullptr).

While it is usually not a problem (as the sal users often recompute the
proper section, when needed), it may lead to the proper gdbarch not
being assigned when setting a breakpoint.

In code_breakpoint::add_location, gdb attempts to retrieve the gdbarch
through get_sal_arch by checking for the section or the symtab. However,
neither are currently set by cinvert_address_location_to_sals if the
debug symbols cannot be found. We then fall back to the current
architecture, which may cause errors in heterogeneous programs
(in ROCm, a breakpoint was not being hit since GDB was setting an
x86 int3 instruction instead of the architecture-appropriate s_trap 1).

This is a rework of a patch that was approved, but never merged
upstream (https://inbox.sourceware.org/gdb-patches/20241108195257.485488-2-lancelot.six@amd.com/).
The original change proposed to set the objfile field in the sal, and
check this field in get_sal_arch() if neither the section, nor the
symtab is defined. This patch makes GDB compute the section from the pc
instead of checking from the objfile in get_sal_arch, in accordance with
the rule of trying to set the section when creating the sal implemented
in this patch series. The test cases from the original patch are
included in this new one.

This should have minimal impact on other parts of GDB as users of this
section field would either (1) recompute it the same way (2) not use it
at all. In the case of overlay debugging, then the preceding call to
find_pc_overlay would likely assign a section.

Co-Authored-By: Lancelot SIX <lancelot.six@amd.com>
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I23cef6ad5a66f696536c7c49c885a074bfea9b23

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 4560459ad3c03ca8846efa1d230bbe552d97672e..08f7fdd0daa70073b13cfe8812491d577b5c6351 100644 (file)
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2164,6 +2164,10 @@ convert_address_location_to_sals (struct linespec_state *self,
   symtab_and_line sal = find_sal_for_pc (address, 0);
   sal.pc = address;
   sal.section = find_pc_overlay (address);
+
+  if (sal.section == nullptr)
+    sal.section = find_pc_section (address);
+
   sal.explicit_pc = 1;
   sal.symbol = find_symbol_for_pc_sect_maybe_inline (sal.pc, sal.section);
 

diff --git a/gdb/testsuite/gdb.rocm/addr-bp-gpu-no-deb-info.cpp b/gdb/testsuite/gdb.rocm/addr-bp-gpu-no-deb-info.cpp
new file mode 100644 (file)
index 0000000..f46a57c
--- /dev/null
+++ b/gdb/testsuite/gdb.rocm/addr-bp-gpu-no-deb-info.cpp
@@ -0,0 +1,30 @@
+/* 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/>.  */
+
+#include <hip/hip_runtime.h>
+
+__global__ void
+kern ()
+{
+}
+
+int
+main ()
+{
+  kern<<<1, 1>>> ();
+  return hipDeviceSynchronize () != hipSuccess;
+}

diff --git a/gdb/testsuite/gdb.rocm/addr-bp-gpu-no-deb-info.exp b/gdb/testsuite/gdb.rocm/addr-bp-gpu-no-deb-info.exp
new file mode 100644 (file)
index 0000000..be45967
--- /dev/null
+++ b/gdb/testsuite/gdb.rocm/addr-bp-gpu-no-deb-info.exp
@@ -0,0 +1,68 @@
+# 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/>.
+
+# Test setting a breakpoint by address on a GPU code object compiled without
+# debug information while focussing on a CPU thread.
+
+load_lib rocm.exp
+
+standard_testfile .cpp
+
+require allow_hipcc_tests
+
+if {[build_executable "failed to prepare" $testfile $srcfile {hip}]} {
+    return
+}
+
+clean_restart $::testfile
+
+# We may have multiple GPUs, resulting in many possible locations. This is
+# needed to ensure we get a single address to break on.
+gdb_test_no_output "set environment ROCR_VISIBLE_DEVICES=1"
+
+# Make the HIP runtime load all the GPU code objects during initialization.
+gdb_test_no_output "set environment HIP_ENABLE_DEFERRED_LOADING=0"
+
+with_rocm_gpu_lock {
+    if { ![runto_main] } {
+       return
+    }
+
+    # Create the breakpoint by name to have GDB resolve the symbol address.
+    set bp_addr 0
+    gdb_test_multiple "b kern" "" {
+       -re -wrap "Breakpoint $::decimal at ($::hex)" {
+           set bp_addr $expect_out(1,string)
+           pass $gdb_test_name
+       }
+    }
+
+    # Ensure current focus is on a host thread.
+    gdb_assert {[get_integer_valueof "\$_thread" 0] == 1} \
+       "selected host thread"
+
+    # Remove this breakpoint...
+    gdb_test_no_output "delete \$bpnum"
+
+    # ...and re-create it by address.
+    gdb_breakpoint "*$bp_addr"
+
+    gdb_continue_to_breakpoint "breakpoint by address"
+    gdb_assert {[get_valueof "/x" "\$pc" 0] == $bp_addr} \
+       "stopped at breakpoint"
+
+    gdb_continue_to_end "" continue 1
+}
+