[PATCH] gdb: reject inserting breakpoints between functions

[PATCH] gdb: reject inserting breakpoints between functions

[Simon Marchi]

From: Simon Marchi <Simon.Marchi@amd.com>

In the downstream ROCm-GDB port (to debug AMD GPUs), you can have code
like this:

Consider the following code:

  __global__ void kernel ()
  {
    ...
    // break here
    ...
  }

  int main ()
  {
    // Code to call `kernel`
  }

... where kernel is a function compiled to execute on the GPU.  It does
not exist in the host x86-64 program that runs the main function, and
GDB doesn't know about that function until it is called, at which point
the runtime loads the corresponding code object and GDB learns about the
"kernel" symbol.  Before the GPU code object is loaded, from the point
of view of GDB, you might as well have blank lines instead of the
"kernel" function.  The DWARF in the host program doesn't describe
anything at these lines.

So, a common problem that users face is:

 - Start GDB with the host binary
 - Place a breakpoint by line number at the "break here" line
 - At this point, GDB only knows about the host code, the lines of the
   `kernel` function are a big void.
 - GDB finds no code mapped to the "break here" line, searches for the
   first following line that has code mapped to it.
 - GDB finds that the line with the opening bracket of the `main`
   function (or around there) has code mapped to it, places breakpoint
   there.
 - User runs the program.
 - The programs hits the breakpoint at the start of main.
 - User is confused, because they didn't ask for a breakpoint in main.

If they continue, the code object eventually gets loaded, GDB reads the
debug info from it, re-evaluates the breakpoint locations, and at this
point the breakpoint is placed at the expected location.

The goal of this patch is to get rid of this annoyance.

A case similar to the one shown above can actually be simulated without
GPU-specific code: using a single source file used to generate a library
and an executable loading that library (see the new test
gdb.linespec/line-breakpoint-outside-function.c).  Before the library is
loaded, trying to place a breakpoint in the library code results in the
breakpoint "drifting" down to the main function.

To address this problem, I suggest making it so that when a user
requests a breakpoint outside a function, GDB makes a pending
breakpoint, rather than placing a breakpoint at the next line with code,
which happens to be in the next function.  When the GPU kernel or shared
library gets loaded, the breakpoint resolves to a location in the kernel
or library.

Note that we still want breakpoints placed inside a function to
"drift" down to the next line with code.  For example, here:

   9
  10 void foo()
  11 {
  12   int x;
  13
  14   x++;

There is probably no code associated to lines 10, 12 and 13, but the
user can still reasonably expect to be able to put a breakpoint there.
In my experience, GCC maps the function prologue to the line with the
opening curly bracket, so the user will be able to place a breakpoint
there anyway (line 11 in the example).  But I don't really see a use
case to put a breakpoint above line 10 and expect to get a breakpoint in
foo.  So I think that is a reasonable behavior change for GDB.

This is implemented using the following heuristic:

 - If a breakpoint is requested at line L but there is no code mapped to
   L, search for a following line with associated code (this already
   exists today).
 - However, if:

     1. the found location falls in a function symbol's block
     2. the found location's address is equal the entry PC of that
        function
     3. the found location's line is greater that the requested line

   ... then we don't place a breakpoint at the found location, we will
   end up with a pending breakpoint.

Change the message "No line X in file..." to "No compiled code for line
X in file...".  There is clearly a line 9 in the example above, so it
would be weird to say "No line 9 in file...".  What we mean is that
there is no code associated to line 9.

All the regressions that I found this patch to cause were:

 1. tests specifically this behavior where placing a breakpoint before
    a function results in a breakpoint on that function, in which case I
    removed the tests or changed them to expect a pending breakpoint
 2. linespec tests expecting things like "break -line N garbage" to
    error out because of the following garbage, but we now got a
    different error because line N now doesn't resolve to something
    anymore.  For example, before:

      (gdb) break -line 3 if foofoofoo == 1
      No symbol "foofoofoo" in current context.

    became

      (gdb) break -line 3 if foofoofoo == 1
      No line 3 in the current file.

    These tests were modified to refer to a valid line with code, so
    that we can still test what we intended to test.

Notes:

 - The CUDA compiler "solves" this problem by adding dummy function
   symbols between functions, that are never called.  So when you try to
   insert a breakpoint in the not-yet-loaded kernel, the breakpoint
   still drifts, but is placed on some dummy symbol.  For reasons that
   would be too long to explain here, the ROCm compiler does not do
   that, and it is not a desirable option.

 - You can have constructs like this:

   void host_function()
   {
     struct foo
     {
       static void __global__ kernel ()
       {
	 // Place breakpoint here
       }
     };

     // Host code that calls `kernel`
   }

   The heuristic won't work then, as the breakpoint will drift somewhere
   inside the enclosing function, but won't be at the start of that
   function.  So a bogus breakpoint location will be created on the host
   side.  I don't think that people are going to use this kind of
   construct often though, so we can probably ignore it.

   ROCm doesn't support passing a lambda kernel function to
   hipLaunchKernelGGL (the function used to launch kernels on the
   device), but if it eventually does, there will be the same
   problem.

   I think that to properly support this, we will need some DWARF
   improvements to be able to say "there is really nothing at these
   lines" in the line table.

Change-Id: I310b79af3009354e50d5a298b5ae32f90b72b9a3
---
 gdb/linespec.c                                | 58 ++++++++++++++++---
 .../gdb.base/break-on-linker-gcd-function.exp |  8 +--
 gdb/testsuite/gdb.base/break.exp              |  2 +-
 gdb/testsuite/gdb.base/ending-run.exp         | 38 +++++-------
 gdb/testsuite/gdb.base/foll-exec-mode.exp     |  2 +-
 gdb/testsuite/gdb.base/hbreak2.exp            |  2 +-
 gdb/testsuite/gdb.base/sepdebug.exp           |  2 +-
 gdb/testsuite/gdb.linespec/cpexplicit.exp     |  2 +-
 gdb/testsuite/gdb.linespec/explicit.exp       |  2 +-
 .../line-breakpoint-outside-function.c        | 51 ++++++++++++++++
 .../line-breakpoint-outside-function.exp      | 53 +++++++++++++++++
 gdb/testsuite/gdb.linespec/ls-errs.c          | 10 ++++
 gdb/testsuite/gdb.linespec/ls-errs.exp        | 14 ++---
 gdb/testsuite/gdb.python/py-breakpoint.exp    |  2 +-
 gdb/testsuite/gdb.trace/tfind.exp             |  4 +-
 gdb/testsuite/gdb.trace/tracecmd.exp          |  2 +-
 16 files changed, 199 insertions(+), 53 deletions(-)
 create mode 100644 gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
 create mode 100644 gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 9d4707cbb4e7..dd31cf2a9fc4 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2085,12 +2085,19 @@ create_sals_line_offset (struct linespec_state *self,
       struct linetable_entry *best_entry = NULL;
       int i, j;
 
+      /* True if the provided line gave an exact match.  False if we had to
+         search for the next following line with code.  */
+      bool was_exact = true;
+
       std::vector<symtab_and_line> intermediate_results
 	= decode_digits_ordinary (self, ls, val.line, &best_entry);
       if (intermediate_results.empty () && best_entry != NULL)
-	intermediate_results = decode_digits_ordinary (self, ls,
-						       best_entry->line,
-						       &best_entry);
+	{
+	  was_exact = false;
+	  intermediate_results = decode_digits_ordinary (self, ls,
+							 best_entry->line,
+							 &best_entry);
+	}
 
       /* For optimized code, the compiler can scatter one source line
 	 across disjoint ranges of PC values, even when no duplicate
@@ -2133,11 +2140,44 @@ create_sals_line_offset (struct linespec_state *self,
 	    struct symbol *sym = (blocks[i]
 				  ? block_containing_function (blocks[i])
 				  : NULL);
+	    symtab_and_line *sal = &intermediate_results[i];
+
+	    /* Don't consider a match if:
+
+	        - the provided line did not give an exact match (so we started
+	          looking for lines below until we found one with code
+	          associated to it)
+	        - the found location is exactly the start of a function
+	        - the provided line is above the declaration line of the function
+
+	       Consider the following source:
+
+	       10 } // end of a previous function
+	       11
+	       12 int
+	       13 main (void)
+	       14 {
+	       15   int i = 1;
+	       16
+	       17   return 0;
+	       18 }
+
+	       The intent of this heuristic is that a breakpoint requested on
+	       line 11 and 12 will not result on a breakpoint on main, but a
+	       breakpoint on line 13 will.  A breakpoint requested on the empty
+	       line 16 will also result in a breakpoint in main, at line 17.  */
+	    if (!was_exact
+		&& sym != nullptr
+		&& sym->aclass () == LOC_BLOCK
+		&& sal->pc == BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym))
+		&& val.line < sym->line ())
+	      continue;
 
 	    if (self->funfirstline)
-	      skip_prologue_sal (&intermediate_results[i]);
-	    intermediate_results[i].symbol = sym;
-	    add_sal_to_sals (self, &values, &intermediate_results[i],
+	      skip_prologue_sal (sal);
+
+	    sal->symbol = sym;
+	    add_sal_to_sals (self, &values, sal,
 			     sym ? sym->natural_name () : NULL, 0);
 	  }
     }
@@ -2145,10 +2185,12 @@ create_sals_line_offset (struct linespec_state *self,
   if (values.empty ())
     {
       if (ls->explicit_loc.source_filename)
-	throw_error (NOT_FOUND_ERROR, _("No line %d in file \"%s\"."),
+	throw_error (NOT_FOUND_ERROR,
+		     _("No compiled code for line %d in file \"%s\"."),
 		     val.line, ls->explicit_loc.source_filename);
       else
-	throw_error (NOT_FOUND_ERROR, _("No line %d in the current file."),
+	throw_error (NOT_FOUND_ERROR,
+		     _("No compiled code for line %d in the current file."),
 		     val.line);
     }
 
diff --git a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
index aa1e328cdab9..08ab469317c0 100644
--- a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
+++ b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
@@ -44,10 +44,10 @@ proc set_breakpoint_on_gcd_function {} {
     # Single hex digit
     set xd {[0-9a-f]}
 
-    # This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
-    # but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
-    gdb_test "b [gdb_get_line_number "gdb break here"]" \
-	"Breakpoint \[0-9\] at 0x${xd}${xd}+: .*"
+    set lineno [gdb_get_line_number "gdb break here"]
+    gdb_test "set breakpoint pending on"
+    gdb_test "b $lineno" \
+	"No compiled code for line $lineno in the current file.\r\nBreakpoint $::decimal \\($lineno\\) pending."
 }
 
 set_breakpoint_on_gcd_function
diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
index 2c939ada14ac..19f5309b449c 100644
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -491,7 +491,7 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
 #
 gdb_test_no_output "set breakpoint pending off"
 gdb_test "break 999" \
-    "No line 999 in the current file." \
+    "No compiled code for line 999 in the current file." \
     "break on non-existent source line"
 
 # Run to the desired default location. If not positioned here, the
diff --git a/gdb/testsuite/gdb.base/ending-run.exp b/gdb/testsuite/gdb.base/ending-run.exp
index 906f1ac40cad..0d4e3c9be91f 100644
--- a/gdb/testsuite/gdb.base/ending-run.exp
+++ b/gdb/testsuite/gdb.base/ending-run.exp
@@ -28,24 +28,15 @@ if { [prepare_for_testing "failed to prepare" $testfile] } {
 }
 remote_exec build "rm -f core"
 
-# CHFts23469: Test that you can "clear" a bp set at
-# a line _before_ the routine (which will default to the
-# first line in the routine, which turns out to correspond
-# to the prolog--that's another bug...)
-#
-
-gdb_test "b ending-run.c:1" ".*Breakpoint.*ending-run.c, line 1.*" \
-	"bpt at line before routine"
-
 set break1_line [gdb_get_line_number "-break1-"]
 gdb_test "b ending-run.c:$break1_line" \
-	".*Note.*also.*Breakpoint 2.*ending-run.c, line $break1_line.*" \
+	"Breakpoint 1 at ${::hex}.*" \
 	"b ending-run.c:$break1_line, one"
 
 # Set up to go to the next-to-last line of the program
 #
 set break2_line [gdb_get_line_number "-break2-"]
-gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $break2_line.*"
+gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 2.*ending-run.c, line $break2_line.*"
 
 # Expect to hit the bp at line "1", but symbolize this
 # as line "13".  Then try to clear it--this should work.
@@ -53,29 +44,28 @@ gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $brea
 gdb_run_cmd
 gdb_test "" ".*Breakpoint.*1.*callee.*$break1_line.*" "run"
 
-gdb_test "cle" ".*Deleted breakpoints 1 2.*" "clear worked"
-gdb_test_multiple "i b" "cleared bp at line before routine" {
-    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" { 
-	fail "cleared bp at line before routine" 
+gdb_test "cle" "Deleted breakpoint 1 " "clear worked"
+gdb_test_multiple "i b" "cleared bp at stopped line" {
+    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
+	fail $gdb_test_name
     }
-    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
-	pass "cleared bp at line before routine" 
+    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
+	pass $gdb_test_name
     }
 }
 
 # Test some other "clear" combinations
 #
-gdb_test "b ending-run.c:1" ".*Breakpoint.*4.*"
-gdb_test "b ending-run.c:$break1_line" ".*Note.*also.*Breakpoint.*5.*" "b ending-run.c:$break1_line, two"
+gdb_test "b ending-run.c:$break1_line" "Breakpoint 3 at ${::hex}.*" "b ending-run.c:$break1_line, two"
 gdb_test "cle ending-run.c:$break1_line" \
-	".*Deleted breakpoints 4 5.*" "Cleared 2 by line"
+	"Deleted breakpoint 3 " "Cleared 2 by line"
 
 gdb_test_multiple "info line ending-run.c:$break1_line" "" {
     -re ".*address (0x\[0-9a-fA-F]*).*$gdb_prompt $" {
         set line_nine $expect_out(1,string)
-        gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 6.*ending-run.c, line $break1_line.*"
-        gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 7.*" "breakpoint 7 at *ending-run.c:$break1_line"
-        gdb_test "cle" ".*Deleted breakpoints 6 7.*" "clear 2 by default"
+        gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 4.*ending-run.c, line $break1_line.*"
+        gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 5.*" "breakpoint 7 at *ending-run.c:$break1_line"
+        gdb_test "cle" "Deleted breakpoints 4 5 " "clear 2 by default"
     }
     -re ".*$gdb_prompt $" {
         fail "need to fix test for new compile outcome"
@@ -86,7 +76,7 @@ gdb_test_multiple "i b" "all set to continue" {
     -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
         fail "all set to continue (didn't clear bps)" 
     }
-    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
+    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
         pass "all set to continue"
     }
     -re ".*$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.base/foll-exec-mode.exp b/gdb/testsuite/gdb.base/foll-exec-mode.exp
index 986e46ecd61d..0a52449837c6 100644
--- a/gdb/testsuite/gdb.base/foll-exec-mode.exp
+++ b/gdb/testsuite/gdb.base/foll-exec-mode.exp
@@ -131,7 +131,7 @@ proc do_follow_exec_mode_tests { mode cmd infswitch } {
 	# past it.
 	#
 	if {$cmd == "continue"} {
-	    gdb_breakpoint "$execd_line"
+	    gdb_breakpoint "$execd_line" "allow-pending"
 	}
 
 	# Execute past the exec call.
diff --git a/gdb/testsuite/gdb.base/hbreak2.exp b/gdb/testsuite/gdb.base/hbreak2.exp
index aecf613643d6..cbeba8d9bcb0 100644
--- a/gdb/testsuite/gdb.base/hbreak2.exp
+++ b/gdb/testsuite/gdb.base/hbreak2.exp
@@ -296,7 +296,7 @@ if ![runto_main] then {
 #
 gdb_test_no_output "set breakpoint pending off"
 gdb_test "hbreak 999" \
-    "No line 999 in the current file." \
+    "No compiled code for line 999 in the current file." \
     "hardware break on non-existent source line"
 
 # Run to the desired default location.  If not positioned here, the
diff --git a/gdb/testsuite/gdb.base/sepdebug.exp b/gdb/testsuite/gdb.base/sepdebug.exp
index 20a7f346994d..ce0030a11549 100644
--- a/gdb/testsuite/gdb.base/sepdebug.exp
+++ b/gdb/testsuite/gdb.base/sepdebug.exp
@@ -296,7 +296,7 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
 #
 
 gdb_test_no_output "set breakpoint pending off"
-gdb_test "break 999" "No line 999 in the current file." \
+gdb_test "break 999" "No compiled code for line 999 in the current file." \
     "break on non-existent source line"
 
 # Run to the desired default location. If not positioned here, the
diff --git a/gdb/testsuite/gdb.linespec/cpexplicit.exp b/gdb/testsuite/gdb.linespec/cpexplicit.exp
index 038c09f96fdd..0a9d0f43f9a9 100644
--- a/gdb/testsuite/gdb.linespec/cpexplicit.exp
+++ b/gdb/testsuite/gdb.linespec/cpexplicit.exp
@@ -83,7 +83,7 @@ namespace eval $testfile {
     add linespecs "-function myclass::myfunction -line 3" $location(normal)
     add linespecs "-function myclass::myfunction -label top -line 3" \
 	$location(top)
-    add linespecs "-line 3" $location(normal)
+    add linespecs "-line 25" $location(normal)
     add linespecs "-function myclass::operator," $location(operator)
     add linespecs "-function 'myclass::operator,'" $location(operator)
     add linespecs "-function \"myclass::operator,\"" $location(operator)
diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp
index 9064c137e136..ac8f461242b8 100644
--- a/gdb/testsuite/gdb.linespec/explicit.exp
+++ b/gdb/testsuite/gdb.linespec/explicit.exp
@@ -86,7 +86,7 @@ namespace eval $testfile {
     # These are also not yet supported; -line is silently ignored.
     add linespecs "-function myfunction -line 3" $location(normal)
     add linespecs "-function myfunction -label top -line 3" $location(top)
-    add linespecs "-line 3" $location(normal)
+    add linespecs "-line 25" $location(normal)
 
     # Fire up gdb.
     if {![runto_main]} {
diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
new file mode 100644
index 000000000000..0c1006ac4f1d
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 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/>.  */
+
+/* The section where THE_LIB_PATH is not defined is compiled as a shared
+   library.  The rest is compiled as the main executable (which loads the
+   shared library.  */
+
+#if !defined(THE_LIB_PATH)
+
+void
+the_lib_func (void)
+{
+  static int x;
+  /* break here */
+  x++;
+}
+
+#else
+#include <dlfcn.h>
+#include <assert.h>
+#include <stdlib.h>
+
+int
+main (void)
+{
+  void *lib = dlopen (THE_LIB_PATH, RTLD_NOW);
+  assert (lib != NULL);
+
+  void (*the_lib_func) (void) = dlsym (lib, "the_lib_func");
+  assert (the_lib_func != NULL);
+
+  the_lib_func ();
+
+  return 0;
+}
+
+#endif
diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
new file mode 100644
index 000000000000..f2083e4e9c2c
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
@@ -0,0 +1,53 @@
+# Copyright 2022 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 that placing a line breakpoint outside a function results in a pending
+# breakpoint.  More importantly, that it does "drift" and place a
+# breakpoint on the next function.
+#
+# See the .c file for more details.
+
+standard_testfile
+
+set shlib_path [standard_output_file ${testfile}-lib.so]
+
+if { [gdb_compile_shlib $srcdir/$subdir/$srcfile $shlib_path {debug}] != "" } {
+    return
+}
+
+set opts [list debug shlib_load additional_flags=-DTHE_LIB_PATH="${shlib_path}"]
+if { [build_executable "failed to prepare"  ${testfile} ${srcfile} $opts] } {
+    return
+}
+
+proc do_test {} {
+    clean_restart $::binfile
+
+    # To make things easier, just so we don't have to deal with the question.
+    gdb_test_no_output "set breakpoint pending on"
+
+    set lineno [gdb_get_line_number "break here"]
+    gdb_test "break $lineno" \
+	"No compiled code for line $lineno in the current file.\r\nBreakpoint 1 \\($lineno\\) pending."
+
+    gdb_run_cmd
+    gdb_test_multiple "" "stop on lib function breakpoint" {
+	-re -wrap "Breakpoint 1, the_lib_func .*29.*x\\+\\+.*" {
+	    pass $gdb_test_name
+	}
+    }
+}
+
+do_test
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
index a53c133d5acc..a8a95f3d8254 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.c
+++ b/gdb/testsuite/gdb.linespec/ls-errs.c
@@ -21,6 +21,16 @@ myfunction (int aa)
   int i;
 
   i = aa + 42;
+
+  /* These lines are intentionally left blank such that the tests trying
+     to place breakpoints at line -10 relative to the "set.breakpoint.here"
+     line below land on a valid breakpoint location, inside the function.  */
+
+
+
+
+
+
   return i;    /* set breakpoint here */
 }
 
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
index ef01bbe85602..3837cffd7d0a 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.exp
+++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
@@ -71,8 +71,8 @@ proc do_test {lang} {
 	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
 	invalid_label "No label \"%s\" defined in function \"%s\"."
 	invalid_parm "invalid linespec argument, \"%s\""
-	invalid_offset "No line %d in the current file."
-	invalid_offset_f "No line %d in file \"%s\"."
+	invalid_offset "No compiled code for line %d in the current file."
+	invalid_offset_f "No compiled code for line %d in file \"%s\"."
 	malformed_line_offset "malformed line offset: \"%s\""
 	source_incomplete \
 	    "Source filename requires function, label, or line offset."
@@ -135,14 +135,14 @@ proc do_test {lang} {
 
     foreach x {1 +1 +100 -10} {
 	test_break "3 $x" unexpected_opt "number" $x
-	test_break "-line 3 $x" garbage $x
+	test_break "-line 34 $x" garbage $x
 	test_break "+10 $x" unexpected_opt "number" $x
 	test_break "-line +10 $x" garbage $x
 	test_break "-10 $x" unexpected_opt "number" $x
 	test_break "-line -10 $x" garbage $x
     }
 
-    foreach x {3 +10 -10} {
+    foreach x {34 +10 -10} {
 	test_break "$x foo" unexpected_opt "string" "foo"
 	test_break "-line $x foo" garbage "foo"
     }
@@ -207,12 +207,12 @@ proc do_test {lang} {
 
     test_break "${srcfile}::" invalid_function "${srcfile}::"
     test_break "$srcfile:3 1" unexpected_opt "number" "1"
-    test_break "-source $srcfile -line 3 1" garbage "1"
+    test_break "-source $srcfile -line 34 1" garbage "1"
     test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
-    test_break "-source $srcfile -line 3 +100" garbage "+100"
+    test_break "-source $srcfile -line 34 +100" garbage "+100"
     test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
     test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
-    test_break "-source $srcfile -line 3 foo" garbage "foo"
+    test_break "-source $srcfile -line 34 foo" garbage "foo"
 
     foreach x $invalid_offsets {
 	test_break "$srcfile:$x" invalid_offset_f $x $srcfile
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 58b1af3a0daf..f8e13a085aae 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -627,7 +627,7 @@ proc_with_prefix test_bkpt_explicit_loc {} {
 	"No source file named foo.*" \
 	"set invalid explicit breakpoint by missing source and line"
     gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"900\")" \
-	"No line 900 in file \"$srcfile\".*" \
+	"No compiled code for line 900 in file \"$srcfile\".*" \
 	"set invalid explicit breakpoint by source and invalid line"
     gdb_test "python bp1 = gdb.Breakpoint (function=\"blah\")" \
 	"Function \"blah\" not defined.*" \
diff --git a/gdb/testsuite/gdb.trace/tfind.exp b/gdb/testsuite/gdb.trace/tfind.exp
index c987ab14e4df..c45458f4dc96 100644
--- a/gdb/testsuite/gdb.trace/tfind.exp
+++ b/gdb/testsuite/gdb.trace/tfind.exp
@@ -351,10 +351,10 @@ gdb_test "disassemble gdb_c_test" \
     "8.36: trace disassembly"
 
 gdb_test "tfind line 0" \
-	"out of range.*|failed to find.*|No line 0 in .*" \
+	"out of range.*|failed to find.*|No compiled code for line 0 in .*" \
 	"8.18: tfind line 0"
 gdb_test "tfind line 32767" \
-	"out of range.*|failed to find.*|No line 32767 in .*" \
+	"out of range.*|failed to find.*|No compiled code for line 32767 in .*" \
 	"8.27: tfind line 32767"
 gdb_test "tfind line NoSuChFiLe.c:$baseline" \
 	"No source file named.*" \
diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
index c2ec95a7a4eb..395ad8429b7f 100644
--- a/gdb/testsuite/gdb.trace/tracecmd.exp
+++ b/gdb/testsuite/gdb.trace/tracecmd.exp
@@ -73,7 +73,7 @@ gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2.
 # 1.2 trace invalid source line
 gdb_delete_tracepoints
 gdb_test_no_output "set breakpoint pending off"
-gdb_test "trace $srcfile:99999" "No line 99999 in file \".*$srcfile\"." \
+gdb_test "trace $srcfile:99999" "No compiled code for line 99999 in file \".*$srcfile\"." \
 	"1.2a: trace invalid line in sourcefile"
 gdb_test "info trace" "No tracepoints.*" \
 	"1.2b: reject invalid line in srcfile"
-- 
2.26.2

Re: [PATCH] gdb: reject inserting breakpoints between functions

[Lancelot SIX]

Hi,

I have just sent a patch[1] which sits on top of this one.  I do not think
there have been feedbacks on this yet, so I'd like to ping on this one on
behalf on Simon.

For what it is worth, I do agree with the change proposed changes as
it is a pre-requisite for what I am proposing in [1].

Thanks,
Lancelot.

[1] https://sourceware.org/pipermail/gdb-patches/2022-June/190150.html


> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 9d4707cbb4e7..dd31cf2a9fc4 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -2133,11 +2140,44 @@ create_sals_line_offset (struct linespec_state *self,
>  	    struct symbol *sym = (blocks[i]
>  				  ? block_containing_function (blocks[i])
>  				  : NULL);
> +	    symtab_and_line *sal = &intermediate_results[i];
> +
> +	    /* Don't consider a match if:
> +
> +	        - the provided line did not give an exact match (so we started
> +	          looking for lines below until we found one with code
> +	          associated to it)
> +	        - the found location is exactly the start of a function
> +	        - the provided line is above the declaration line of the function
> +
> +	       Consider the following source:
> +
> +	       10 } // end of a previous function
> +	       11
> +	       12 int
> +	       13 main (void)
> +	       14 {
> +	       15   int i = 1;
> +	       16
> +	       17   return 0;
> +	       18 }
> +
> +	       The intent of this heuristic is that a breakpoint requested on
> +	       line 11 and 12 will not result on a breakpoint on main, but a
> +	       breakpoint on line 13 will.  A breakpoint requested on the empty
> +	       line 16 will also result in a breakpoint in main, at line 17.  */
> +	    if (!was_exact
> +		&& sym != nullptr
> +		&& sym->aclass () == LOC_BLOCK
> +		&& sal->pc == BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym))

The BLOCK_ENTRY_PC and SYMBOL_BLOCK_VALUE macros have been removed.
This should now be:

+		&& sal->pc == sym->value_block ()->entry_pc ()

> +		&& val.line < sym->line ())
> +	      continue;
>  
>  	    if (self->funfirstline)
> -	      skip_prologue_sal (&intermediate_results[i]);
> -	    intermediate_results[i].symbol = sym;
> -	    add_sal_to_sals (self, &values, &intermediate_results[i],
> +	      skip_prologue_sal (sal);
> +
> +	    sal->symbol = sym;
> +	    add_sal_to_sals (self, &values, sal,
>  			     sym ? sym->natural_name () : NULL, 0);
>  	  }
>      }

Re: [PATCH] gdb: reject inserting breakpoints between functions

[Andrew Burgess]

Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> From: Simon Marchi <Simon.Marchi@amd.com>
>
> In the downstream ROCm-GDB port (to debug AMD GPUs), you can have code
> like this:
>
> Consider the following code:
>
>   __global__ void kernel ()
>   {
>     ...
>     // break here
>     ...
>   }
>
>   int main ()
>   {
>     // Code to call `kernel`
>   }
>
> ... where kernel is a function compiled to execute on the GPU.  It does
> not exist in the host x86-64 program that runs the main function, and
> GDB doesn't know about that function until it is called, at which point
> the runtime loads the corresponding code object and GDB learns about the
> "kernel" symbol.  Before the GPU code object is loaded, from the point
> of view of GDB, you might as well have blank lines instead of the
> "kernel" function.  The DWARF in the host program doesn't describe
> anything at these lines.
>
> So, a common problem that users face is:
>
>  - Start GDB with the host binary
>  - Place a breakpoint by line number at the "break here" line
>  - At this point, GDB only knows about the host code, the lines of the
>    `kernel` function are a big void.
>  - GDB finds no code mapped to the "break here" line, searches for the
>    first following line that has code mapped to it.
>  - GDB finds that the line with the opening bracket of the `main`
>    function (or around there) has code mapped to it, places breakpoint
>    there.
>  - User runs the program.
>  - The programs hits the breakpoint at the start of main.
>  - User is confused, because they didn't ask for a breakpoint in main.
>
> If they continue, the code object eventually gets loaded, GDB reads the
> debug info from it, re-evaluates the breakpoint locations, and at this
> point the breakpoint is placed at the expected location.
>
> The goal of this patch is to get rid of this annoyance.
>
> A case similar to the one shown above can actually be simulated without
> GPU-specific code: using a single source file used to generate a library
> and an executable loading that library (see the new test
> gdb.linespec/line-breakpoint-outside-function.c).  Before the library is
> loaded, trying to place a breakpoint in the library code results in the
> breakpoint "drifting" down to the main function.
>
> To address this problem, I suggest making it so that when a user
> requests a breakpoint outside a function, GDB makes a pending
> breakpoint, rather than placing a breakpoint at the next line with code,
> which happens to be in the next function.  When the GPU kernel or shared
> library gets loaded, the breakpoint resolves to a location in the kernel
> or library.
>
> Note that we still want breakpoints placed inside a function to
> "drift" down to the next line with code.  For example, here:
>
>    9
>   10 void foo()
>   11 {
>   12   int x;
>   13
>   14   x++;
>
> There is probably no code associated to lines 10, 12 and 13, but the
> user can still reasonably expect to be able to put a breakpoint there.
> In my experience, GCC maps the function prologue to the line with the
> opening curly bracket, so the user will be able to place a breakpoint
> there anyway (line 11 in the example).  But I don't really see a use
> case to put a breakpoint above line 10 and expect to get a breakpoint in
> foo.  So I think that is a reasonable behavior change for GDB.
>
> This is implemented using the following heuristic:
>
>  - If a breakpoint is requested at line L but there is no code mapped to
>    L, search for a following line with associated code (this already
>    exists today).
>  - However, if:
>
>      1. the found location falls in a function symbol's block
>      2. the found location's address is equal the entry PC of that
>         function
>      3. the found location's line is greater that the requested line
>
>    ... then we don't place a breakpoint at the found location, we will
>    end up with a pending breakpoint.
>
> Change the message "No line X in file..." to "No compiled code for line
> X in file...".  There is clearly a line 9 in the example above, so it
> would be weird to say "No line 9 in file...".  What we mean is that
> there is no code associated to line 9.
>
> All the regressions that I found this patch to cause were:
>
>  1. tests specifically this behavior where placing a breakpoint before
>     a function results in a breakpoint on that function, in which case I
>     removed the tests or changed them to expect a pending breakpoint
>  2. linespec tests expecting things like "break -line N garbage" to
>     error out because of the following garbage, but we now got a
>     different error because line N now doesn't resolve to something
>     anymore.  For example, before:
>
>       (gdb) break -line 3 if foofoofoo == 1
>       No symbol "foofoofoo" in current context.
>
>     became
>
>       (gdb) break -line 3 if foofoofoo == 1
>       No line 3 in the current file.
>
>     These tests were modified to refer to a valid line with code, so
>     that we can still test what we intended to test.
>
> Notes:
>
>  - The CUDA compiler "solves" this problem by adding dummy function
>    symbols between functions, that are never called.  So when you try to
>    insert a breakpoint in the not-yet-loaded kernel, the breakpoint
>    still drifts, but is placed on some dummy symbol.  For reasons that
>    would be too long to explain here, the ROCm compiler does not do
>    that, and it is not a desirable option.
>
>  - You can have constructs like this:
>
>    void host_function()
>    {
>      struct foo
>      {
>        static void __global__ kernel ()
>        {
> 	 // Place breakpoint here
>        }
>      };
>
>      // Host code that calls `kernel`
>    }
>
>    The heuristic won't work then, as the breakpoint will drift somewhere
>    inside the enclosing function, but won't be at the start of that
>    function.  So a bogus breakpoint location will be created on the host
>    side.  I don't think that people are going to use this kind of
>    construct often though, so we can probably ignore it.
>
>    ROCm doesn't support passing a lambda kernel function to
>    hipLaunchKernelGGL (the function used to launch kernels on the
>    device), but if it eventually does, there will be the same
>    problem.
>
>    I think that to properly support this, we will need some DWARF
>    improvements to be able to say "there is really nothing at these
>    lines" in the line table.

I took a look through this patch, and it looks good.  I only skimmed
most of the testsuite changes (except for the new test) though.

There's some obvious updates needed in linespec.c to take account of
recent GDB changes to replace macros with member functions.

I also noticed when applying this patch that there's lots of whitespace
issues (spaces before tabs, spaces instead of tabs) that could do with
being cleaned up before this is merged.

But with those fixes I think this is a good improvement.

Thanks,
Andrew


>
> Change-Id: I310b79af3009354e50d5a298b5ae32f90b72b9a3
> ---
>  gdb/linespec.c                                | 58 ++++++++++++++++---
>  .../gdb.base/break-on-linker-gcd-function.exp |  8 +--
>  gdb/testsuite/gdb.base/break.exp              |  2 +-
>  gdb/testsuite/gdb.base/ending-run.exp         | 38 +++++-------
>  gdb/testsuite/gdb.base/foll-exec-mode.exp     |  2 +-
>  gdb/testsuite/gdb.base/hbreak2.exp            |  2 +-
>  gdb/testsuite/gdb.base/sepdebug.exp           |  2 +-
>  gdb/testsuite/gdb.linespec/cpexplicit.exp     |  2 +-
>  gdb/testsuite/gdb.linespec/explicit.exp       |  2 +-
>  .../line-breakpoint-outside-function.c        | 51 ++++++++++++++++
>  .../line-breakpoint-outside-function.exp      | 53 +++++++++++++++++
>  gdb/testsuite/gdb.linespec/ls-errs.c          | 10 ++++
>  gdb/testsuite/gdb.linespec/ls-errs.exp        | 14 ++---
>  gdb/testsuite/gdb.python/py-breakpoint.exp    |  2 +-
>  gdb/testsuite/gdb.trace/tfind.exp             |  4 +-
>  gdb/testsuite/gdb.trace/tracecmd.exp          |  2 +-
>  16 files changed, 199 insertions(+), 53 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
>  create mode 100644 gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 9d4707cbb4e7..dd31cf2a9fc4 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -2085,12 +2085,19 @@ create_sals_line_offset (struct linespec_state *self,
>        struct linetable_entry *best_entry = NULL;
>        int i, j;
>  
> +      /* True if the provided line gave an exact match.  False if we had to
> +         search for the next following line with code.  */
> +      bool was_exact = true;
> +
>        std::vector<symtab_and_line> intermediate_results
>  	= decode_digits_ordinary (self, ls, val.line, &best_entry);
>        if (intermediate_results.empty () && best_entry != NULL)
> -	intermediate_results = decode_digits_ordinary (self, ls,
> -						       best_entry->line,
> -						       &best_entry);
> +	{
> +	  was_exact = false;
> +	  intermediate_results = decode_digits_ordinary (self, ls,
> +							 best_entry->line,
> +							 &best_entry);
> +	}
>  
>        /* For optimized code, the compiler can scatter one source line
>  	 across disjoint ranges of PC values, even when no duplicate
> @@ -2133,11 +2140,44 @@ create_sals_line_offset (struct linespec_state *self,
>  	    struct symbol *sym = (blocks[i]
>  				  ? block_containing_function (blocks[i])
>  				  : NULL);
> +	    symtab_and_line *sal = &intermediate_results[i];
> +
> +	    /* Don't consider a match if:
> +
> +	        - the provided line did not give an exact match (so we started
> +	          looking for lines below until we found one with code
> +	          associated to it)
> +	        - the found location is exactly the start of a function
> +	        - the provided line is above the declaration line of the function
> +
> +	       Consider the following source:
> +
> +	       10 } // end of a previous function
> +	       11
> +	       12 int
> +	       13 main (void)
> +	       14 {
> +	       15   int i = 1;
> +	       16
> +	       17   return 0;
> +	       18 }
> +
> +	       The intent of this heuristic is that a breakpoint requested on
> +	       line 11 and 12 will not result on a breakpoint on main, but a
> +	       breakpoint on line 13 will.  A breakpoint requested on the empty
> +	       line 16 will also result in a breakpoint in main, at line 17.  */
> +	    if (!was_exact
> +		&& sym != nullptr
> +		&& sym->aclass () == LOC_BLOCK
> +		&& sal->pc == BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym))
> +		&& val.line < sym->line ())
> +	      continue;
>  
>  	    if (self->funfirstline)
> -	      skip_prologue_sal (&intermediate_results[i]);
> -	    intermediate_results[i].symbol = sym;
> -	    add_sal_to_sals (self, &values, &intermediate_results[i],
> +	      skip_prologue_sal (sal);
> +
> +	    sal->symbol = sym;
> +	    add_sal_to_sals (self, &values, sal,
>  			     sym ? sym->natural_name () : NULL, 0);
>  	  }
>      }
> @@ -2145,10 +2185,12 @@ create_sals_line_offset (struct linespec_state *self,
>    if (values.empty ())
>      {
>        if (ls->explicit_loc.source_filename)
> -	throw_error (NOT_FOUND_ERROR, _("No line %d in file \"%s\"."),
> +	throw_error (NOT_FOUND_ERROR,
> +		     _("No compiled code for line %d in file \"%s\"."),
>  		     val.line, ls->explicit_loc.source_filename);
>        else
> -	throw_error (NOT_FOUND_ERROR, _("No line %d in the current file."),
> +	throw_error (NOT_FOUND_ERROR,
> +		     _("No compiled code for line %d in the current file."),
>  		     val.line);
>      }
>  
> diff --git a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
> index aa1e328cdab9..08ab469317c0 100644
> --- a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
> +++ b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
> @@ -44,10 +44,10 @@ proc set_breakpoint_on_gcd_function {} {
>      # Single hex digit
>      set xd {[0-9a-f]}
>  
> -    # This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
> -    # but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
> -    gdb_test "b [gdb_get_line_number "gdb break here"]" \
> -	"Breakpoint \[0-9\] at 0x${xd}${xd}+: .*"
> +    set lineno [gdb_get_line_number "gdb break here"]
> +    gdb_test "set breakpoint pending on"
> +    gdb_test "b $lineno" \
> +	"No compiled code for line $lineno in the current file.\r\nBreakpoint $::decimal \\($lineno\\) pending."
>  }
>  
>  set_breakpoint_on_gcd_function
> diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
> index 2c939ada14ac..19f5309b449c 100644
> --- a/gdb/testsuite/gdb.base/break.exp
> +++ b/gdb/testsuite/gdb.base/break.exp
> @@ -491,7 +491,7 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
>  #
>  gdb_test_no_output "set breakpoint pending off"
>  gdb_test "break 999" \
> -    "No line 999 in the current file." \
> +    "No compiled code for line 999 in the current file." \
>      "break on non-existent source line"
>  
>  # Run to the desired default location. If not positioned here, the
> diff --git a/gdb/testsuite/gdb.base/ending-run.exp b/gdb/testsuite/gdb.base/ending-run.exp
> index 906f1ac40cad..0d4e3c9be91f 100644
> --- a/gdb/testsuite/gdb.base/ending-run.exp
> +++ b/gdb/testsuite/gdb.base/ending-run.exp
> @@ -28,24 +28,15 @@ if { [prepare_for_testing "failed to prepare" $testfile] } {
>  }
>  remote_exec build "rm -f core"
>  
> -# CHFts23469: Test that you can "clear" a bp set at
> -# a line _before_ the routine (which will default to the
> -# first line in the routine, which turns out to correspond
> -# to the prolog--that's another bug...)
> -#
> -
> -gdb_test "b ending-run.c:1" ".*Breakpoint.*ending-run.c, line 1.*" \
> -	"bpt at line before routine"
> -
>  set break1_line [gdb_get_line_number "-break1-"]
>  gdb_test "b ending-run.c:$break1_line" \
> -	".*Note.*also.*Breakpoint 2.*ending-run.c, line $break1_line.*" \
> +	"Breakpoint 1 at ${::hex}.*" \
>  	"b ending-run.c:$break1_line, one"
>  
>  # Set up to go to the next-to-last line of the program
>  #
>  set break2_line [gdb_get_line_number "-break2-"]
> -gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $break2_line.*"
> +gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 2.*ending-run.c, line $break2_line.*"
>  
>  # Expect to hit the bp at line "1", but symbolize this
>  # as line "13".  Then try to clear it--this should work.
> @@ -53,29 +44,28 @@ gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $brea
>  gdb_run_cmd
>  gdb_test "" ".*Breakpoint.*1.*callee.*$break1_line.*" "run"
>  
> -gdb_test "cle" ".*Deleted breakpoints 1 2.*" "clear worked"
> -gdb_test_multiple "i b" "cleared bp at line before routine" {
> -    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" { 
> -	fail "cleared bp at line before routine" 
> +gdb_test "cle" "Deleted breakpoint 1 " "clear worked"
> +gdb_test_multiple "i b" "cleared bp at stopped line" {
> +    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
> +	fail $gdb_test_name
>      }
> -    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
> -	pass "cleared bp at line before routine" 
> +    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
> +	pass $gdb_test_name
>      }
>  }
>  
>  # Test some other "clear" combinations
>  #
> -gdb_test "b ending-run.c:1" ".*Breakpoint.*4.*"
> -gdb_test "b ending-run.c:$break1_line" ".*Note.*also.*Breakpoint.*5.*" "b ending-run.c:$break1_line, two"
> +gdb_test "b ending-run.c:$break1_line" "Breakpoint 3 at ${::hex}.*" "b ending-run.c:$break1_line, two"
>  gdb_test "cle ending-run.c:$break1_line" \
> -	".*Deleted breakpoints 4 5.*" "Cleared 2 by line"
> +	"Deleted breakpoint 3 " "Cleared 2 by line"
>  
>  gdb_test_multiple "info line ending-run.c:$break1_line" "" {
>      -re ".*address (0x\[0-9a-fA-F]*).*$gdb_prompt $" {
>          set line_nine $expect_out(1,string)
> -        gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 6.*ending-run.c, line $break1_line.*"
> -        gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 7.*" "breakpoint 7 at *ending-run.c:$break1_line"
> -        gdb_test "cle" ".*Deleted breakpoints 6 7.*" "clear 2 by default"
> +        gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 4.*ending-run.c, line $break1_line.*"
> +        gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 5.*" "breakpoint 7 at *ending-run.c:$break1_line"
> +        gdb_test "cle" "Deleted breakpoints 4 5 " "clear 2 by default"
>      }
>      -re ".*$gdb_prompt $" {
>          fail "need to fix test for new compile outcome"
> @@ -86,7 +76,7 @@ gdb_test_multiple "i b" "all set to continue" {
>      -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
>          fail "all set to continue (didn't clear bps)" 
>      }
> -    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
> +    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
>          pass "all set to continue"
>      }
>      -re ".*$gdb_prompt $" {
> diff --git a/gdb/testsuite/gdb.base/foll-exec-mode.exp b/gdb/testsuite/gdb.base/foll-exec-mode.exp
> index 986e46ecd61d..0a52449837c6 100644
> --- a/gdb/testsuite/gdb.base/foll-exec-mode.exp
> +++ b/gdb/testsuite/gdb.base/foll-exec-mode.exp
> @@ -131,7 +131,7 @@ proc do_follow_exec_mode_tests { mode cmd infswitch } {
>  	# past it.
>  	#
>  	if {$cmd == "continue"} {
> -	    gdb_breakpoint "$execd_line"
> +	    gdb_breakpoint "$execd_line" "allow-pending"
>  	}
>  
>  	# Execute past the exec call.
> diff --git a/gdb/testsuite/gdb.base/hbreak2.exp b/gdb/testsuite/gdb.base/hbreak2.exp
> index aecf613643d6..cbeba8d9bcb0 100644
> --- a/gdb/testsuite/gdb.base/hbreak2.exp
> +++ b/gdb/testsuite/gdb.base/hbreak2.exp
> @@ -296,7 +296,7 @@ if ![runto_main] then {
>  #
>  gdb_test_no_output "set breakpoint pending off"
>  gdb_test "hbreak 999" \
> -    "No line 999 in the current file." \
> +    "No compiled code for line 999 in the current file." \
>      "hardware break on non-existent source line"
>  
>  # Run to the desired default location.  If not positioned here, the
> diff --git a/gdb/testsuite/gdb.base/sepdebug.exp b/gdb/testsuite/gdb.base/sepdebug.exp
> index 20a7f346994d..ce0030a11549 100644
> --- a/gdb/testsuite/gdb.base/sepdebug.exp
> +++ b/gdb/testsuite/gdb.base/sepdebug.exp
> @@ -296,7 +296,7 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
>  #
>  
>  gdb_test_no_output "set breakpoint pending off"
> -gdb_test "break 999" "No line 999 in the current file." \
> +gdb_test "break 999" "No compiled code for line 999 in the current file." \
>      "break on non-existent source line"
>  
>  # Run to the desired default location. If not positioned here, the
> diff --git a/gdb/testsuite/gdb.linespec/cpexplicit.exp b/gdb/testsuite/gdb.linespec/cpexplicit.exp
> index 038c09f96fdd..0a9d0f43f9a9 100644
> --- a/gdb/testsuite/gdb.linespec/cpexplicit.exp
> +++ b/gdb/testsuite/gdb.linespec/cpexplicit.exp
> @@ -83,7 +83,7 @@ namespace eval $testfile {
>      add linespecs "-function myclass::myfunction -line 3" $location(normal)
>      add linespecs "-function myclass::myfunction -label top -line 3" \
>  	$location(top)
> -    add linespecs "-line 3" $location(normal)
> +    add linespecs "-line 25" $location(normal)
>      add linespecs "-function myclass::operator," $location(operator)
>      add linespecs "-function 'myclass::operator,'" $location(operator)
>      add linespecs "-function \"myclass::operator,\"" $location(operator)
> diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp
> index 9064c137e136..ac8f461242b8 100644
> --- a/gdb/testsuite/gdb.linespec/explicit.exp
> +++ b/gdb/testsuite/gdb.linespec/explicit.exp
> @@ -86,7 +86,7 @@ namespace eval $testfile {
>      # These are also not yet supported; -line is silently ignored.
>      add linespecs "-function myfunction -line 3" $location(normal)
>      add linespecs "-function myfunction -label top -line 3" $location(top)
> -    add linespecs "-line 3" $location(normal)
> +    add linespecs "-line 25" $location(normal)
>  
>      # Fire up gdb.
>      if {![runto_main]} {
> diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
> new file mode 100644
> index 000000000000..0c1006ac4f1d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
> @@ -0,0 +1,51 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 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/>.  */
> +
> +/* The section where THE_LIB_PATH is not defined is compiled as a shared
> +   library.  The rest is compiled as the main executable (which loads the
> +   shared library.  */
> +
> +#if !defined(THE_LIB_PATH)
> +
> +void
> +the_lib_func (void)
> +{
> +  static int x;
> +  /* break here */
> +  x++;
> +}
> +
> +#else
> +#include <dlfcn.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +
> +int
> +main (void)
> +{
> +  void *lib = dlopen (THE_LIB_PATH, RTLD_NOW);
> +  assert (lib != NULL);
> +
> +  void (*the_lib_func) (void) = dlsym (lib, "the_lib_func");
> +  assert (the_lib_func != NULL);
> +
> +  the_lib_func ();
> +
> +  return 0;
> +}
> +
> +#endif
> diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
> new file mode 100644
> index 000000000000..f2083e4e9c2c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
> @@ -0,0 +1,53 @@
> +# Copyright 2022 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 that placing a line breakpoint outside a function results in a pending
> +# breakpoint.  More importantly, that it does "drift" and place a
> +# breakpoint on the next function.
> +#
> +# See the .c file for more details.
> +
> +standard_testfile
> +
> +set shlib_path [standard_output_file ${testfile}-lib.so]
> +
> +if { [gdb_compile_shlib $srcdir/$subdir/$srcfile $shlib_path {debug}] != "" } {
> +    return
> +}
> +
> +set opts [list debug shlib_load additional_flags=-DTHE_LIB_PATH="${shlib_path}"]
> +if { [build_executable "failed to prepare"  ${testfile} ${srcfile} $opts] } {
> +    return
> +}
> +
> +proc do_test {} {
> +    clean_restart $::binfile
> +
> +    # To make things easier, just so we don't have to deal with the question.
> +    gdb_test_no_output "set breakpoint pending on"
> +
> +    set lineno [gdb_get_line_number "break here"]
> +    gdb_test "break $lineno" \
> +	"No compiled code for line $lineno in the current file.\r\nBreakpoint 1 \\($lineno\\) pending."
> +
> +    gdb_run_cmd
> +    gdb_test_multiple "" "stop on lib function breakpoint" {
> +	-re -wrap "Breakpoint 1, the_lib_func .*29.*x\\+\\+.*" {
> +	    pass $gdb_test_name
> +	}
> +    }
> +}
> +
> +do_test
> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
> index a53c133d5acc..a8a95f3d8254 100644
> --- a/gdb/testsuite/gdb.linespec/ls-errs.c
> +++ b/gdb/testsuite/gdb.linespec/ls-errs.c
> @@ -21,6 +21,16 @@ myfunction (int aa)
>    int i;
>  
>    i = aa + 42;
> +
> +  /* These lines are intentionally left blank such that the tests trying
> +     to place breakpoints at line -10 relative to the "set.breakpoint.here"
> +     line below land on a valid breakpoint location, inside the function.  */
> +
> +
> +
> +
> +
> +
>    return i;    /* set breakpoint here */
>  }
>  
> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
> index ef01bbe85602..3837cffd7d0a 100644
> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
> @@ -71,8 +71,8 @@ proc do_test {lang} {
>  	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>  	invalid_label "No label \"%s\" defined in function \"%s\"."
>  	invalid_parm "invalid linespec argument, \"%s\""
> -	invalid_offset "No line %d in the current file."
> -	invalid_offset_f "No line %d in file \"%s\"."
> +	invalid_offset "No compiled code for line %d in the current file."
> +	invalid_offset_f "No compiled code for line %d in file \"%s\"."
>  	malformed_line_offset "malformed line offset: \"%s\""
>  	source_incomplete \
>  	    "Source filename requires function, label, or line offset."
> @@ -135,14 +135,14 @@ proc do_test {lang} {
>  
>      foreach x {1 +1 +100 -10} {
>  	test_break "3 $x" unexpected_opt "number" $x
> -	test_break "-line 3 $x" garbage $x
> +	test_break "-line 34 $x" garbage $x
>  	test_break "+10 $x" unexpected_opt "number" $x
>  	test_break "-line +10 $x" garbage $x
>  	test_break "-10 $x" unexpected_opt "number" $x
>  	test_break "-line -10 $x" garbage $x
>      }
>  
> -    foreach x {3 +10 -10} {
> +    foreach x {34 +10 -10} {
>  	test_break "$x foo" unexpected_opt "string" "foo"
>  	test_break "-line $x foo" garbage "foo"
>      }
> @@ -207,12 +207,12 @@ proc do_test {lang} {
>  
>      test_break "${srcfile}::" invalid_function "${srcfile}::"
>      test_break "$srcfile:3 1" unexpected_opt "number" "1"
> -    test_break "-source $srcfile -line 3 1" garbage "1"
> +    test_break "-source $srcfile -line 34 1" garbage "1"
>      test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
> -    test_break "-source $srcfile -line 3 +100" garbage "+100"
> +    test_break "-source $srcfile -line 34 +100" garbage "+100"
>      test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>      test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
> -    test_break "-source $srcfile -line 3 foo" garbage "foo"
> +    test_break "-source $srcfile -line 34 foo" garbage "foo"
>  
>      foreach x $invalid_offsets {
>  	test_break "$srcfile:$x" invalid_offset_f $x $srcfile
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index 58b1af3a0daf..f8e13a085aae 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -627,7 +627,7 @@ proc_with_prefix test_bkpt_explicit_loc {} {
>  	"No source file named foo.*" \
>  	"set invalid explicit breakpoint by missing source and line"
>      gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"900\")" \
> -	"No line 900 in file \"$srcfile\".*" \
> +	"No compiled code for line 900 in file \"$srcfile\".*" \
>  	"set invalid explicit breakpoint by source and invalid line"
>      gdb_test "python bp1 = gdb.Breakpoint (function=\"blah\")" \
>  	"Function \"blah\" not defined.*" \
> diff --git a/gdb/testsuite/gdb.trace/tfind.exp b/gdb/testsuite/gdb.trace/tfind.exp
> index c987ab14e4df..c45458f4dc96 100644
> --- a/gdb/testsuite/gdb.trace/tfind.exp
> +++ b/gdb/testsuite/gdb.trace/tfind.exp
> @@ -351,10 +351,10 @@ gdb_test "disassemble gdb_c_test" \
>      "8.36: trace disassembly"
>  
>  gdb_test "tfind line 0" \
> -	"out of range.*|failed to find.*|No line 0 in .*" \
> +	"out of range.*|failed to find.*|No compiled code for line 0 in .*" \
>  	"8.18: tfind line 0"
>  gdb_test "tfind line 32767" \
> -	"out of range.*|failed to find.*|No line 32767 in .*" \
> +	"out of range.*|failed to find.*|No compiled code for line 32767 in .*" \
>  	"8.27: tfind line 32767"
>  gdb_test "tfind line NoSuChFiLe.c:$baseline" \
>  	"No source file named.*" \
> diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
> index c2ec95a7a4eb..395ad8429b7f 100644
> --- a/gdb/testsuite/gdb.trace/tracecmd.exp
> +++ b/gdb/testsuite/gdb.trace/tracecmd.exp
> @@ -73,7 +73,7 @@ gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2.
>  # 1.2 trace invalid source line
>  gdb_delete_tracepoints
>  gdb_test_no_output "set breakpoint pending off"
> -gdb_test "trace $srcfile:99999" "No line 99999 in file \".*$srcfile\"." \
> +gdb_test "trace $srcfile:99999" "No compiled code for line 99999 in file \".*$srcfile\"." \
>  	"1.2a: trace invalid line in sourcefile"
>  gdb_test "info trace" "No tracepoints.*" \
>  	"1.2b: reject invalid line in srcfile"
> -- 
> 2.26.2

Re: [PATCH] gdb: reject inserting breakpoints between functions

[Klaus Gerlicher]

Hi Simon,

I verified that your patch addresses many of the issues we would also
like to solve in this area.

It appears this has not published yet and it seems to be more than 2
years old.

Could you please tell me if there are any plans to commit this?

Thanks
Klaus Gerlicher

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon
Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> From: Simon Marchi <Simon.Marchi@amd.com>
>
> In the downstream ROCm-GDB port (to debug AMD GPUs), you can have code
> like this:
>
> Consider the following code:
>
>   __global__ void kernel ()
>   {
>     ...
>     // break here
>     ...
>   }
>
>   int main ()
>   {
>     // Code to call `kernel`
>   }
>
> ... where kernel is a function compiled to execute on the GPU.  It does
> not exist in the host x86-64 program that runs the main function, and
> GDB doesn't know about that function until it is called, at which point
> the runtime loads the corresponding code object and GDB learns about the
> "kernel" symbol.  Before the GPU code object is loaded, from the point
> of view of GDB, you might as well have blank lines instead of the
> "kernel" function.  The DWARF in the host program doesn't describe
> anything at these lines.
>
> So, a common problem that users face is:
>
>  - Start GDB with the host binary
>  - Place a breakpoint by line number at the "break here" line
>  - At this point, GDB only knows about the host code, the lines of the
>    `kernel` function are a big void.
>  - GDB finds no code mapped to the "break here" line, searches for the
>    first following line that has code mapped to it.
>  - GDB finds that the line with the opening bracket of the `main`
>    function (or around there) has code mapped to it, places breakpoint
>    there.
>  - User runs the program.
>  - The programs hits the breakpoint at the start of main.
>  - User is confused, because they didn't ask for a breakpoint in main.
>
> If they continue, the code object eventually gets loaded, GDB reads the
> debug info from it, re-evaluates the breakpoint locations, and at this
> point the breakpoint is placed at the expected location.
>
> The goal of this patch is to get rid of this annoyance.
>
> A case similar to the one shown above can actually be simulated without
> GPU-specific code: using a single source file used to generate a library
> and an executable loading that library (see the new test
> gdb.linespec/line-breakpoint-outside-function.c).  Before the library is
> loaded, trying to place a breakpoint in the library code results in the
> breakpoint "drifting" down to the main function.
>
> To address this problem, I suggest making it so that when a user
> requests a breakpoint outside a function, GDB makes a pending
> breakpoint, rather than placing a breakpoint at the next line with code,
> which happens to be in the next function.  When the GPU kernel or shared
> library gets loaded, the breakpoint resolves to a location in the kernel
> or library.
>
> Note that we still want breakpoints placed inside a function to
> "drift" down to the next line with code.  For example, here:
>
>    9
>   10 void foo()
>   11 {
>   12   int x;
>   13
>   14   x++;
>
> There is probably no code associated to lines 10, 12 and 13, but the
> user can still reasonably expect to be able to put a breakpoint there.
> In my experience, GCC maps the function prologue to the line with the
> opening curly bracket, so the user will be able to place a breakpoint
> there anyway (line 11 in the example).  But I don't really see a use
> case to put a breakpoint above line 10 and expect to get a breakpoint in
> foo.  So I think that is a reasonable behavior change for GDB.
>
> This is implemented using the following heuristic:
>
>  - If a breakpoint is requested at line L but there is no code mapped to
>    L, search for a following line with associated code (this already
>    exists today).
>  - However, if:
>
>      1. the found location falls in a function symbol's block
>      2. the found location's address is equal the entry PC of that
>         function
>      3. the found location's line is greater that the requested line
>
>    ... then we don't place a breakpoint at the found location, we will
>    end up with a pending breakpoint.
>
> Change the message "No line X in file..." to "No compiled code for line
> X in file...".  There is clearly a line 9 in the example above, so it
> would be weird to say "No line 9 in file...".  What we mean is that
> there is no code associated to line 9.
>
> All the regressions that I found this patch to cause were:
>
>  1. tests specifically this behavior where placing a breakpoint before
>     a function results in a breakpoint on that function, in which case I
>     removed the tests or changed them to expect a pending breakpoint
>  2. linespec tests expecting things like "break -line N garbage" to
>     error out because of the following garbage, but we now got a
>     different error because line N now doesn't resolve to something
>     anymore.  For example, before:
>
>       (gdb) break -line 3 if foofoofoo == 1
>       No symbol "foofoofoo" in current context.
>
>     became
>
>       (gdb) break -line 3 if foofoofoo == 1
>       No line 3 in the current file.
>
>     These tests were modified to refer to a valid line with code, so
>     that we can still test what we intended to test.
>
> Notes:
>
>  - The CUDA compiler "solves" this problem by adding dummy function
>    symbols between functions, that are never called.  So when you try to
>    insert a breakpoint in the not-yet-loaded kernel, the breakpoint
>    still drifts, but is placed on some dummy symbol.  For reasons that
>    would be too long to explain here, the ROCm compiler does not do
>    that, and it is not a desirable option.
>
>  - You can have constructs like this:
>
>    void host_function()
>    {
>      struct foo
>      {
>        static void __global__ kernel ()
>        {
> 	 // Place breakpoint here
>        }
>      };
>
>      // Host code that calls `kernel`
>    }
>
>    The heuristic won't work then, as the breakpoint will drift somewhere
>    inside the enclosing function, but won't be at the start of that
>    function.  So a bogus breakpoint location will be created on the host
>    side.  I don't think that people are going to use this kind of
>    construct often though, so we can probably ignore it.
>
>    ROCm doesn't support passing a lambda kernel function to
>    hipLaunchKernelGGL (the function used to launch kernels on the
>    device), but if it eventually does, there will be the same
>    problem.
>
>    I think that to properly support this, we will need some DWARF
>    improvements to be able to say "there is really nothing at these
>    lines" in the line table.

I took a look through this patch, and it looks good.  I only skimmed
most of the testsuite changes (except for the new test) though.

There's some obvious updates needed in linespec.c to take account of
recent GDB changes to replace macros with member functions.

I also noticed when applying this patch that there's lots of whitespace
issues (spaces before tabs, spaces instead of tabs) that could do with
being cleaned up before this is merged.

But with those fixes I think this is a good improvement.

Thanks,
Andrew


>
> Change-Id: I310b79af3009354e50d5a298b5ae32f90b72b9a3
> ---
>  gdb/linespec.c                                | 58 ++++++++++++++++---
>  .../gdb.base/break-on-linker-gcd-function.exp |  8 +--
>  gdb/testsuite/gdb.base/break.exp              |  2 +-
>  gdb/testsuite/gdb.base/ending-run.exp         | 38 +++++-------
>  gdb/testsuite/gdb.base/foll-exec-mode.exp     |  2 +-
>  gdb/testsuite/gdb.base/hbreak2.exp            |  2 +-
>  gdb/testsuite/gdb.base/sepdebug.exp           |  2 +-
>  gdb/testsuite/gdb.linespec/cpexplicit.exp     |  2 +-
>  gdb/testsuite/gdb.linespec/explicit.exp       |  2 +-
>  .../line-breakpoint-outside-function.c        | 51 ++++++++++++++++
>  .../line-breakpoint-outside-function.exp      | 53 +++++++++++++++++
>  gdb/testsuite/gdb.linespec/ls-errs.c          | 10 ++++
>  gdb/testsuite/gdb.linespec/ls-errs.exp        | 14 ++---
>  gdb/testsuite/gdb.python/py-breakpoint.exp    |  2 +-
>  gdb/testsuite/gdb.trace/tfind.exp             |  4 +-
>  gdb/testsuite/gdb.trace/tracecmd.exp          |  2 +-
>  16 files changed, 199 insertions(+), 53 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
>  create mode 100644 gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 9d4707cbb4e7..dd31cf2a9fc4 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -2085,12 +2085,19 @@ create_sals_line_offset (struct linespec_state *self,
>        struct linetable_entry *best_entry = NULL;
>        int i, j;
>  
> +      /* True if the provided line gave an exact match.  False if we had to
> +         search for the next following line with code.  */
> +      bool was_exact = true;
> +
>        std::vector<symtab_and_line> intermediate_results
>  	= decode_digits_ordinary (self, ls, val.line, &best_entry);
>        if (intermediate_results.empty () && best_entry != NULL)
> -	intermediate_results = decode_digits_ordinary (self, ls,
> -						       best_entry->line,
> -						       &best_entry);
> +	{
> +	  was_exact = false;
> +	  intermediate_results = decode_digits_ordinary (self, ls,
> +							 best_entry->line,
> +							 &best_entry);
> +	}
>  
>        /* For optimized code, the compiler can scatter one source line
>  	 across disjoint ranges of PC values, even when no duplicate
> @@ -2133,11 +2140,44 @@ create_sals_line_offset (struct linespec_state *self,
>  	    struct symbol *sym = (blocks[i]
>  				  ? block_containing_function (blocks[i])
>  				  : NULL);
> +	    symtab_and_line *sal = &intermediate_results[i];
> +
> +	    /* Don't consider a match if:
> +
> +	        - the provided line did not give an exact match (so we started
> +	          looking for lines below until we found one with code
> +	          associated to it)
> +	        - the found location is exactly the start of a function
> +	        - the provided line is above the declaration line of the function
> +
> +	       Consider the following source:
> +
> +	       10 } // end of a previous function
> +	       11
> +	       12 int
> +	       13 main (void)
> +	       14 {
> +	       15   int i = 1;
> +	       16
> +	       17   return 0;
> +	       18 }
> +
> +	       The intent of this heuristic is that a breakpoint requested on
> +	       line 11 and 12 will not result on a breakpoint on main, but a
> +	       breakpoint on line 13 will.  A breakpoint requested on the empty
> +	       line 16 will also result in a breakpoint in main, at line 17.  */
> +	    if (!was_exact
> +		&& sym != nullptr
> +		&& sym->aclass () == LOC_BLOCK
> +		&& sal->pc == BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym))
> +		&& val.line < sym->line ())
> +	      continue;
>  
>  	    if (self->funfirstline)
> -	      skip_prologue_sal (&intermediate_results[i]);
> -	    intermediate_results[i].symbol = sym;
> -	    add_sal_to_sals (self, &values, &intermediate_results[i],
> +	      skip_prologue_sal (sal);
> +
> +	    sal->symbol = sym;
> +	    add_sal_to_sals (self, &values, sal,
>  			     sym ? sym->natural_name () : NULL, 0);
>  	  }
>      }
> @@ -2145,10 +2185,12 @@ create_sals_line_offset (struct linespec_state *self,
>    if (values.empty ())
>      {
>        if (ls->explicit_loc.source_filename)
> -	throw_error (NOT_FOUND_ERROR, _("No line %d in file \"%s\"."),
> +	throw_error (NOT_FOUND_ERROR,
> +		     _("No compiled code for line %d in file \"%s\"."),
>  		     val.line, ls->explicit_loc.source_filename);
>        else
> -	throw_error (NOT_FOUND_ERROR, _("No line %d in the current file."),
> +	throw_error (NOT_FOUND_ERROR,
> +		     _("No compiled code for line %d in the current file."),
>  		     val.line);
>      }
>  
> diff --git a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
> index aa1e328cdab9..08ab469317c0 100644
> --- a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
> +++ b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
> @@ -44,10 +44,10 @@ proc set_breakpoint_on_gcd_function {} {
>      # Single hex digit
>      set xd {[0-9a-f]}
>  
> -    # This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
> -    # but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
> -    gdb_test "b [gdb_get_line_number "gdb break here"]" \
> -	"Breakpoint \[0-9\] at 0x${xd}${xd}+: .*"
> +    set lineno [gdb_get_line_number "gdb break here"]
> +    gdb_test "set breakpoint pending on"
> +    gdb_test "b $lineno" \
> +	"No compiled code for line $lineno in the current file.\r\nBreakpoint $::decimal \\($lineno\\) pending."
>  }
>  
>  set_breakpoint_on_gcd_function
> diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
> index 2c939ada14ac..19f5309b449c 100644
> --- a/gdb/testsuite/gdb.base/break.exp
> +++ b/gdb/testsuite/gdb.base/break.exp
> @@ -491,7 +491,7 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
>  #
>  gdb_test_no_output "set breakpoint pending off"
>  gdb_test "break 999" \
> -    "No line 999 in the current file." \
> +    "No compiled code for line 999 in the current file." \
>      "break on non-existent source line"
>  
>  # Run to the desired default location. If not positioned here, the
> diff --git a/gdb/testsuite/gdb.base/ending-run.exp b/gdb/testsuite/gdb.base/ending-run.exp
> index 906f1ac40cad..0d4e3c9be91f 100644
> --- a/gdb/testsuite/gdb.base/ending-run.exp
> +++ b/gdb/testsuite/gdb.base/ending-run.exp
> @@ -28,24 +28,15 @@ if { [prepare_for_testing "failed to prepare" $testfile] } {
>  }
>  remote_exec build "rm -f core"
>  
> -# CHFts23469: Test that you can "clear" a bp set at
> -# a line _before_ the routine (which will default to the
> -# first line in the routine, which turns out to correspond
> -# to the prolog--that's another bug...)
> -#
> -
> -gdb_test "b ending-run.c:1" ".*Breakpoint.*ending-run.c, line 1.*" \
> -	"bpt at line before routine"
> -
>  set break1_line [gdb_get_line_number "-break1-"]
>  gdb_test "b ending-run.c:$break1_line" \
> -	".*Note.*also.*Breakpoint 2.*ending-run.c, line $break1_line.*" \
> +	"Breakpoint 1 at ${::hex}.*" \
>  	"b ending-run.c:$break1_line, one"
>  
>  # Set up to go to the next-to-last line of the program
>  #
>  set break2_line [gdb_get_line_number "-break2-"]
> -gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $break2_line.*"
> +gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 2.*ending-run.c, line $break2_line.*"
>  
>  # Expect to hit the bp at line "1", but symbolize this
>  # as line "13".  Then try to clear it--this should work.
> @@ -53,29 +44,28 @@ gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $brea
>  gdb_run_cmd
>  gdb_test "" ".*Breakpoint.*1.*callee.*$break1_line.*" "run"
>  
> -gdb_test "cle" ".*Deleted breakpoints 1 2.*" "clear worked"
> -gdb_test_multiple "i b" "cleared bp at line before routine" {
> -    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" { 
> -	fail "cleared bp at line before routine" 
> +gdb_test "cle" "Deleted breakpoint 1 " "clear worked"
> +gdb_test_multiple "i b" "cleared bp at stopped line" {
> +    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
> +	fail $gdb_test_name
>      }
> -    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
> -	pass "cleared bp at line before routine" 
> +    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
> +	pass $gdb_test_name
>      }
>  }
>  
>  # Test some other "clear" combinations
>  #
> -gdb_test "b ending-run.c:1" ".*Breakpoint.*4.*"
> -gdb_test "b ending-run.c:$break1_line" ".*Note.*also.*Breakpoint.*5.*" "b ending-run.c:$break1_line, two"
> +gdb_test "b ending-run.c:$break1_line" "Breakpoint 3 at ${::hex}.*" "b ending-run.c:$break1_line, two"
>  gdb_test "cle ending-run.c:$break1_line" \
> -	".*Deleted breakpoints 4 5.*" "Cleared 2 by line"
> +	"Deleted breakpoint 3 " "Cleared 2 by line"
>  
>  gdb_test_multiple "info line ending-run.c:$break1_line" "" {
>      -re ".*address (0x\[0-9a-fA-F]*).*$gdb_prompt $" {
>          set line_nine $expect_out(1,string)
> -        gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 6.*ending-run.c, line $break1_line.*"
> -        gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 7.*" "breakpoint 7 at *ending-run.c:$break1_line"
> -        gdb_test "cle" ".*Deleted breakpoints 6 7.*" "clear 2 by default"
> +        gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 4.*ending-run.c, line $break1_line.*"
> +        gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 5.*" "breakpoint 7 at *ending-run.c:$break1_line"
> +        gdb_test "cle" "Deleted breakpoints 4 5 " "clear 2 by default"
>      }
>      -re ".*$gdb_prompt $" {
>          fail "need to fix test for new compile outcome"
> @@ -86,7 +76,7 @@ gdb_test_multiple "i b" "all set to continue" {
>      -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
>          fail "all set to continue (didn't clear bps)" 
>      }
> -    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
> +    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
>          pass "all set to continue"
>      }
>      -re ".*$gdb_prompt $" {
> diff --git a/gdb/testsuite/gdb.base/foll-exec-mode.exp b/gdb/testsuite/gdb.base/foll-exec-mode.exp
> index 986e46ecd61d..0a52449837c6 100644
> --- a/gdb/testsuite/gdb.base/foll-exec-mode.exp
> +++ b/gdb/testsuite/gdb.base/foll-exec-mode.exp
> @@ -131,7 +131,7 @@ proc do_follow_exec_mode_tests { mode cmd infswitch } {
>  	# past it.
>  	#
>  	if {$cmd == "continue"} {
> -	    gdb_breakpoint "$execd_line"
> +	    gdb_breakpoint "$execd_line" "allow-pending"
>  	}
>  
>  	# Execute past the exec call.
> diff --git a/gdb/testsuite/gdb.base/hbreak2.exp b/gdb/testsuite/gdb.base/hbreak2.exp
> index aecf613643d6..cbeba8d9bcb0 100644
> --- a/gdb/testsuite/gdb.base/hbreak2.exp
> +++ b/gdb/testsuite/gdb.base/hbreak2.exp
> @@ -296,7 +296,7 @@ if ![runto_main] then {
>  #
>  gdb_test_no_output "set breakpoint pending off"
>  gdb_test "hbreak 999" \
> -    "No line 999 in the current file." \
> +    "No compiled code for line 999 in the current file." \
>      "hardware break on non-existent source line"
>  
>  # Run to the desired default location.  If not positioned here, the
> diff --git a/gdb/testsuite/gdb.base/sepdebug.exp b/gdb/testsuite/gdb.base/sepdebug.exp
> index 20a7f346994d..ce0030a11549 100644
> --- a/gdb/testsuite/gdb.base/sepdebug.exp
> +++ b/gdb/testsuite/gdb.base/sepdebug.exp
> @@ -296,7 +296,7 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
>  #
>  
>  gdb_test_no_output "set breakpoint pending off"
> -gdb_test "break 999" "No line 999 in the current file." \
> +gdb_test "break 999" "No compiled code for line 999 in the current file." \
>      "break on non-existent source line"
>  
>  # Run to the desired default location. If not positioned here, the
> diff --git a/gdb/testsuite/gdb.linespec/cpexplicit.exp b/gdb/testsuite/gdb.linespec/cpexplicit.exp
> index 038c09f96fdd..0a9d0f43f9a9 100644
> --- a/gdb/testsuite/gdb.linespec/cpexplicit.exp
> +++ b/gdb/testsuite/gdb.linespec/cpexplicit.exp
> @@ -83,7 +83,7 @@ namespace eval $testfile {
>      add linespecs "-function myclass::myfunction -line 3" $location(normal)
>      add linespecs "-function myclass::myfunction -label top -line 3" \
>  	$location(top)
> -    add linespecs "-line 3" $location(normal)
> +    add linespecs "-line 25" $location(normal)
>      add linespecs "-function myclass::operator," $location(operator)
>      add linespecs "-function 'myclass::operator,'" $location(operator)
>      add linespecs "-function \"myclass::operator,\"" $location(operator)
> diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp
> index 9064c137e136..ac8f461242b8 100644
> --- a/gdb/testsuite/gdb.linespec/explicit.exp
> +++ b/gdb/testsuite/gdb.linespec/explicit.exp
> @@ -86,7 +86,7 @@ namespace eval $testfile {
>      # These are also not yet supported; -line is silently ignored.
>      add linespecs "-function myfunction -line 3" $location(normal)
>      add linespecs "-function myfunction -label top -line 3" $location(top)
> -    add linespecs "-line 3" $location(normal)
> +    add linespecs "-line 25" $location(normal)
>  
>      # Fire up gdb.
>      if {![runto_main]} {
> diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
> new file mode 100644
> index 000000000000..0c1006ac4f1d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
> @@ -0,0 +1,51 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 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/>.  */
> +
> +/* The section where THE_LIB_PATH is not defined is compiled as a shared
> +   library.  The rest is compiled as the main executable (which loads the
> +   shared library.  */
> +
> +#if !defined(THE_LIB_PATH)
> +
> +void
> +the_lib_func (void)
> +{
> +  static int x;
> +  /* break here */
> +  x++;
> +}
> +
> +#else
> +#include <dlfcn.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +
> +int
> +main (void)
> +{
> +  void *lib = dlopen (THE_LIB_PATH, RTLD_NOW);
> +  assert (lib != NULL);
> +
> +  void (*the_lib_func) (void) = dlsym (lib, "the_lib_func");
> +  assert (the_lib_func != NULL);
> +
> +  the_lib_func ();
> +
> +  return 0;
> +}
> +
> +#endif
> diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
> new file mode 100644
> index 000000000000..f2083e4e9c2c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
> @@ -0,0 +1,53 @@
> +# Copyright 2022 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 that placing a line breakpoint outside a function results in a pending
> +# breakpoint.  More importantly, that it does "drift" and place a
> +# breakpoint on the next function.
> +#
> +# See the .c file for more details.
> +
> +standard_testfile
> +
> +set shlib_path [standard_output_file ${testfile}-lib.so]
> +
> +if { [gdb_compile_shlib $srcdir/$subdir/$srcfile $shlib_path {debug}] != "" } {
> +    return
> +}
> +
> +set opts [list debug shlib_load additional_flags=-DTHE_LIB_PATH="${shlib_path}"]
> +if { [build_executable "failed to prepare"  ${testfile} ${srcfile} $opts] } {
> +    return
> +}
> +
> +proc do_test {} {
> +    clean_restart $::binfile
> +
> +    # To make things easier, just so we don't have to deal with the question.
> +    gdb_test_no_output "set breakpoint pending on"
> +
> +    set lineno [gdb_get_line_number "break here"]
> +    gdb_test "break $lineno" \
> +	"No compiled code for line $lineno in the current file.\r\nBreakpoint 1 \\($lineno\\) pending."
> +
> +    gdb_run_cmd
> +    gdb_test_multiple "" "stop on lib function breakpoint" {
> +	-re -wrap "Breakpoint 1, the_lib_func .*29.*x\\+\\+.*" {
> +	    pass $gdb_test_name
> +	}
> +    }
> +}
> +
> +do_test
> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
> index a53c133d5acc..a8a95f3d8254 100644
> --- a/gdb/testsuite/gdb.linespec/ls-errs.c
> +++ b/gdb/testsuite/gdb.linespec/ls-errs.c
> @@ -21,6 +21,16 @@ myfunction (int aa)
>    int i;
>  
>    i = aa + 42;
> +
> +  /* These lines are intentionally left blank such that the tests trying
> +     to place breakpoints at line -10 relative to the "set.breakpoint.here"
> +     line below land on a valid breakpoint location, inside the function.  */
> +
> +
> +
> +
> +
> +
>    return i;    /* set breakpoint here */
>  }
>  
> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
> index ef01bbe85602..3837cffd7d0a 100644
> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
> @@ -71,8 +71,8 @@ proc do_test {lang} {
>  	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
>  	invalid_label "No label \"%s\" defined in function \"%s\"."
>  	invalid_parm "invalid linespec argument, \"%s\""
> -	invalid_offset "No line %d in the current file."
> -	invalid_offset_f "No line %d in file \"%s\"."
> +	invalid_offset "No compiled code for line %d in the current file."
> +	invalid_offset_f "No compiled code for line %d in file \"%s\"."
>  	malformed_line_offset "malformed line offset: \"%s\""
>  	source_incomplete \
>  	    "Source filename requires function, label, or line offset."
> @@ -135,14 +135,14 @@ proc do_test {lang} {
>  
>      foreach x {1 +1 +100 -10} {
>  	test_break "3 $x" unexpected_opt "number" $x
> -	test_break "-line 3 $x" garbage $x
> +	test_break "-line 34 $x" garbage $x
>  	test_break "+10 $x" unexpected_opt "number" $x
>  	test_break "-line +10 $x" garbage $x
>  	test_break "-10 $x" unexpected_opt "number" $x
>  	test_break "-line -10 $x" garbage $x
>      }
>  
> -    foreach x {3 +10 -10} {
> +    foreach x {34 +10 -10} {
>  	test_break "$x foo" unexpected_opt "string" "foo"
>  	test_break "-line $x foo" garbage "foo"
>      }
> @@ -207,12 +207,12 @@ proc do_test {lang} {
>  
>      test_break "${srcfile}::" invalid_function "${srcfile}::"
>      test_break "$srcfile:3 1" unexpected_opt "number" "1"
> -    test_break "-source $srcfile -line 3 1" garbage "1"
> +    test_break "-source $srcfile -line 34 1" garbage "1"
>      test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
> -    test_break "-source $srcfile -line 3 +100" garbage "+100"
> +    test_break "-source $srcfile -line 34 +100" garbage "+100"
>      test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
>      test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
> -    test_break "-source $srcfile -line 3 foo" garbage "foo"
> +    test_break "-source $srcfile -line 34 foo" garbage "foo"
>  
>      foreach x $invalid_offsets {
>  	test_break "$srcfile:$x" invalid_offset_f $x $srcfile
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index 58b1af3a0daf..f8e13a085aae 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -627,7 +627,7 @@ proc_with_prefix test_bkpt_explicit_loc {} {
>  	"No source file named foo.*" \
>  	"set invalid explicit breakpoint by missing source and line"
>      gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"900\")" \
> -	"No line 900 in file \"$srcfile\".*" \
> +	"No compiled code for line 900 in file \"$srcfile\".*" \
>  	"set invalid explicit breakpoint by source and invalid line"
>      gdb_test "python bp1 = gdb.Breakpoint (function=\"blah\")" \
>  	"Function \"blah\" not defined.*" \
> diff --git a/gdb/testsuite/gdb.trace/tfind.exp b/gdb/testsuite/gdb.trace/tfind.exp
> index c987ab14e4df..c45458f4dc96 100644
> --- a/gdb/testsuite/gdb.trace/tfind.exp
> +++ b/gdb/testsuite/gdb.trace/tfind.exp
> @@ -351,10 +351,10 @@ gdb_test "disassemble gdb_c_test" \
>      "8.36: trace disassembly"
>  
>  gdb_test "tfind line 0" \
> -	"out of range.*|failed to find.*|No line 0 in .*" \
> +	"out of range.*|failed to find.*|No compiled code for line 0 in .*" \
>  	"8.18: tfind line 0"
>  gdb_test "tfind line 32767" \
> -	"out of range.*|failed to find.*|No line 32767 in .*" \
> +	"out of range.*|failed to find.*|No compiled code for line 32767 in .*" \
>  	"8.27: tfind line 32767"
>  gdb_test "tfind line NoSuChFiLe.c:$baseline" \
>  	"No source file named.*" \
> diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
> index c2ec95a7a4eb..395ad8429b7f 100644
> --- a/gdb/testsuite/gdb.trace/tracecmd.exp
> +++ b/gdb/testsuite/gdb.trace/tracecmd.exp
> @@ -73,7 +73,7 @@ gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2.
>  # 1.2 trace invalid source line
>  gdb_delete_tracepoints
>  gdb_test_no_output "set breakpoint pending off"
> -gdb_test "trace $srcfile:99999" "No line 99999 in file \".*$srcfile\"." \
> +gdb_test "trace $srcfile:99999" "No compiled code for line 99999 in file \".*$srcfile\"." \
>  	"1.2a: trace invalid line in sourcefile"
>  gdb_test "info trace" "No tracepoints.*" \
>  	"1.2b: reject invalid line in srcfile"
> -- 
> 2.26.2
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] gdb: reject inserting breakpoints between functions

[Andrew Burgess]

Klaus Gerlicher <klaus.gerlicher@intel.com> writes:

> Hi Simon,
>
> I verified that your patch addresses many of the issues we would also
> like to solve in this area.
>
> It appears this has not published yet and it seems to be more than 2
> years old.
>
> Could you please tell me if there are any plans to commit this?

Given I reviewed this once upon a time, I still had my review branch
kicking around.

I rebased onto something close to HEAD of master.  I addressed the minor
nits I pointed out in my review.  I'd be happy to see this merged once
my full regression run (still on going) has completed.

I think I'd like Simon to give a +1 before I pushed this though.

Thanks,
Andrew

---

commit 245b55f774512a997139c15c43f068c55e0c532c
Author: Simon Marchi <Simon.Marchi@amd.com>
Date:   Fri Apr 8 16:05:36 2022 -0400

    gdb: reject inserting breakpoints between functions
    
    In the downstream ROCm-GDB port (to debug AMD GPUs), you can have code
    like this:
    
    Consider the following code:
    
      __global__ void kernel ()
      {
        ...
        // break here
        ...
      }
    
      int main ()
      {
        // Code to call `kernel`
      }
    
    ... where kernel is a function compiled to execute on the GPU.  It does
    not exist in the host x86-64 program that runs the main function, and
    GDB doesn't know about that function until it is called, at which point
    the runtime loads the corresponding code object and GDB learns about the
    "kernel" symbol.  Before the GPU code object is loaded, from the point
    of view of GDB, you might as well have blank lines instead of the
    "kernel" function.  The DWARF in the host program doesn't describe
    anything at these lines.
    
    So, a common problem that users face is:
    
     - Start GDB with the host binary
     - Place a breakpoint by line number at the "break here" line
     - At this point, GDB only knows about the host code, the lines of the
       `kernel` function are a big void.
     - GDB finds no code mapped to the "break here" line, searches for the
       first following line that has code mapped to it.
     - GDB finds that the line with the opening bracket of the `main`
       function (or around there) has code mapped to it, places breakpoint
       there.
     - User runs the program.
     - The programs hits the breakpoint at the start of main.
     - User is confused, because they didn't ask for a breakpoint in main.
    
    If they continue, the code object eventually gets loaded, GDB reads the
    debug info from it, re-evaluates the breakpoint locations, and at this
    point the breakpoint is placed at the expected location.
    
    The goal of this patch is to get rid of this annoyance.
    
    A case similar to the one shown above can actually be simulated without
    GPU-specific code: using a single source file used to generate a library
    and an executable loading that library (see the new test
    gdb.linespec/line-breakpoint-outside-function.c).  Before the library is
    loaded, trying to place a breakpoint in the library code results in the
    breakpoint "drifting" down to the main function.
    
    To address this problem, I suggest making it so that when a user
    requests a breakpoint outside a function, GDB makes a pending
    breakpoint, rather than placing a breakpoint at the next line with code,
    which happens to be in the next function.  When the GPU kernel or shared
    library gets loaded, the breakpoint resolves to a location in the kernel
    or library.
    
    Note that we still want breakpoints placed inside a function to
    "drift" down to the next line with code.  For example, here:
    
       9
      10 void foo()
      11 {
      12   int x;
      13
      14   x++;
    
    There is probably no code associated to lines 10, 12 and 13, but the
    user can still reasonably expect to be able to put a breakpoint there.
    In my experience, GCC maps the function prologue to the line with the
    opening curly bracket, so the user will be able to place a breakpoint
    there anyway (line 11 in the example).  But I don't really see a use
    case to put a breakpoint above line 10 and expect to get a breakpoint in
    foo.  So I think that is a reasonable behavior change for GDB.
    
    This is implemented using the following heuristic:
    
     - If a breakpoint is requested at line L but there is no code mapped to
       L, search for a following line with associated code (this already
       exists today).
     - However, if:
    
         1. the found location falls in a function symbol's block
         2. the found location's address is equal the entry PC of that
            function
         3. the found location's line is greater that the requested line
    
       ... then we don't place a breakpoint at the found location, we will
       end up with a pending breakpoint.
    
    Change the message "No line X in file..." to "No compiled code for line
    X in file...".  There is clearly a line 9 in the example above, so it
    would be weird to say "No line 9 in file...".  What we mean is that
    there is no code associated to line 9.
    
    All the regressions that I found this patch to cause were:
    
     1. tests specifically this behavior where placing a breakpoint before
        a function results in a breakpoint on that function, in which case I
        removed the tests or changed them to expect a pending breakpoint
     2. linespec tests expecting things like "break -line N garbage" to
        error out because of the following garbage, but we now got a
        different error because line N now doesn't resolve to something
        anymore.  For example, before:
    
          (gdb) break -line 3 if foofoofoo == 1
          No symbol "foofoofoo" in current context.
    
        became
    
          (gdb) break -line 3 if foofoofoo == 1
          No line 3 in the current file.
    
        These tests were modified to refer to a valid line with code, so
        that we can still test what we intended to test.
    
    Notes:
    
     - The CUDA compiler "solves" this problem by adding dummy function
       symbols between functions, that are never called.  So when you try to
       insert a breakpoint in the not-yet-loaded kernel, the breakpoint
       still drifts, but is placed on some dummy symbol.  For reasons that
       would be too long to explain here, the ROCm compiler does not do
       that, and it is not a desirable option.
    
     - You can have constructs like this:
    
       void host_function()
       {
         struct foo
         {
           static void __global__ kernel ()
           {
             // Place breakpoint here
           }
         };
    
         // Host code that calls `kernel`
       }
    
       The heuristic won't work then, as the breakpoint will drift somewhere
       inside the enclosing function, but won't be at the start of that
       function.  So a bogus breakpoint location will be created on the host
       side.  I don't think that people are going to use this kind of
       construct often though, so we can probably ignore it.
    
       ROCm doesn't support passing a lambda kernel function to
       hipLaunchKernelGGL (the function used to launch kernels on the
       device), but if it eventually does, there will be the same
       problem.
    
       I think that to properly support this, we will need some DWARF
       improvements to be able to say "there is really nothing at these
       lines" in the line table.
    
    Change-Id: I310b79af3009354e50d5a298b5ae32f90b72b9a3

diff --git a/gdb/linespec.c b/gdb/linespec.c
index ca154d2dcba..7caf89d4589 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2081,12 +2081,19 @@ create_sals_line_offset (struct linespec_state *self,
       const linetable_entry *best_entry = NULL;
       int i, j;
 
+      /* True if the provided line gave an exact match.  False if we had to
+	 search for the next following line with code.  */
+      bool was_exact = true;
+
       std::vector<symtab_and_line> intermediate_results
 	= decode_digits_ordinary (self, ls, val.line, &best_entry);
       if (intermediate_results.empty () && best_entry != NULL)
-	intermediate_results = decode_digits_ordinary (self, ls,
-						       best_entry->line,
-						       &best_entry);
+	{
+	  was_exact = false;
+	  intermediate_results = decode_digits_ordinary (self, ls,
+							 best_entry->line,
+							 &best_entry);
+	}
 
       /* For optimized code, the compiler can scatter one source line
 	 across disjoint ranges of PC values, even when no duplicate
@@ -2129,11 +2136,45 @@ create_sals_line_offset (struct linespec_state *self,
 	    struct symbol *sym = (blocks[i]
 				  ? blocks[i]->containing_function ()
 				  : NULL);
+	    symtab_and_line *sal = &intermediate_results[i];
+
+	    /* Don't consider a match if:
+
+	       - the provided line did not give an exact match (so we
+		 started looking for lines below until we found one with
+		 code associated to it)
+	       - the found location is exactly the start of a function
+	       - the provided line is above the declaration line of the
+		 function
+
+	       Consider the following source:
+
+	       10 } // end of a previous function
+	       11
+	       12 int
+	       13 main (void)
+	       14 {
+	       15   int i = 1;
+	       16
+	       17   return 0;
+	       18 }
+
+	       The intent of this heuristic is that a breakpoint requested on
+	       line 11 and 12 will not result in a breakpoint on main, but a
+	       breakpoint on line 13 will.  A breakpoint requested on the empty
+	       line 16 will also result in a breakpoint in main, at line 17.  */
+	    if (!was_exact
+		&& sym != nullptr
+		&& sym->aclass () == LOC_BLOCK
+		&& sal->pc == sym->value_block ()->entry_pc ()
+		&& val.line < sym->line ())
+	      continue;
 
 	    if (self->funfirstline)
-	      skip_prologue_sal (&intermediate_results[i]);
-	    intermediate_results[i].symbol = sym;
-	    add_sal_to_sals (self, &values, &intermediate_results[i],
+	      skip_prologue_sal (sal);
+
+	    sal->symbol = sym;
+	    add_sal_to_sals (self, &values, sal,
 			     sym ? sym->natural_name () : NULL, 0);
 	  }
     }
@@ -2141,10 +2182,12 @@ create_sals_line_offset (struct linespec_state *self,
   if (values.empty ())
     {
       if (ls->explicit_loc.source_filename)
-	throw_error (NOT_FOUND_ERROR, _("No line %d in file \"%s\"."),
+	throw_error (NOT_FOUND_ERROR,
+		     _("No compiled code for line %d in file \"%s\"."),
 		     val.line, ls->explicit_loc.source_filename.get ());
       else
-	throw_error (NOT_FOUND_ERROR, _("No line %d in the current file."),
+	throw_error (NOT_FOUND_ERROR,
+		     _("No compiled code for line %d in the current file."),
 		     val.line);
     }
 
diff --git a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
index 613c9dc47e8..86572635d4c 100644
--- a/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
+++ b/gdb/testsuite/gdb.base/break-on-linker-gcd-function.exp
@@ -40,10 +40,13 @@ proc set_breakpoint_on_gcd_function {} {
     # Single hex digit
     set xd {[0-9a-f]}
 
-    # This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
-    # but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
-    gdb_test "b [gdb_get_line_number "gdb break here"]" \
-	"Breakpoint \[0-9\] at 0x${xd}${xd}+: .*"
+    set lineno [gdb_get_line_number "gdb break here"]
+    gdb_test "set breakpoint pending on"
+    gdb_test "b $lineno" \
+	[multi_line \
+	     "^No compiled code for line $lineno in the current file\\." \
+	     "Breakpoint $::decimal \\($lineno\\) pending\\."] \
+	"break on line in garbage collected function"
 }
 
 set_breakpoint_on_gcd_function
diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
index cdb4c22a034..34ac21982ea 100644
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -476,9 +476,6 @@ proc_with_prefix test_no_break_on_catchpoint {} {
 
 test_no_break_on_catchpoint
 
-# Verify that GDB responds gracefully when asked to set a breakpoint
-# on a nonexistent source line.
-
 proc_with_prefix test_break_nonexistent_line {} {
     clean_restart break
 
@@ -486,9 +483,11 @@ proc_with_prefix test_break_nonexistent_line {} {
 	return
     }
 
+    # Verify that GDB responds gracefully when asked to set a
+    # breakpoint on a nonexistent source line.
     gdb_test_no_output "set breakpoint pending off"
     gdb_test "break 999" \
-	"No line 999 in the current file." \
+	"^No compiled code for line 999 in the current file\\." \
 	"break on non-existent source line"
 }
 
diff --git a/gdb/testsuite/gdb.base/ending-run.exp b/gdb/testsuite/gdb.base/ending-run.exp
index b9a72b0e70e..90359fd8733 100644
--- a/gdb/testsuite/gdb.base/ending-run.exp
+++ b/gdb/testsuite/gdb.base/ending-run.exp
@@ -32,24 +32,15 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile $flags] } {
 }
 remote_exec build "rm -f core"
 
-# CHFts23469: Test that you can "clear" a bp set at
-# a line _before_ the routine (which will default to the
-# first line in the routine, which turns out to correspond
-# to the prolog--that's another bug...)
-#
-
-gdb_test "b ending-run.c:1" ".*Breakpoint.*ending-run.c, line 1.*" \
-	"bpt at line before routine"
-
 set break1_line [gdb_get_line_number "-break1-"]
 gdb_test "b ending-run.c:$break1_line" \
-	".*Note.*also.*Breakpoint 2.*ending-run.c, line $break1_line.*" \
+	"Breakpoint 1 at ${::hex}.*" \
 	"b ending-run.c:$break1_line, one"
 
 # Set up to go to the next-to-last line of the program
 #
 set break2_line [gdb_get_line_number "-break2-"]
-gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $break2_line.*"
+gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 2.*ending-run.c, line $break2_line.*"
 
 # Expect to hit the bp at line "1", but symbolize this
 # as line "13".  Then try to clear it--this should work.
@@ -57,29 +48,28 @@ gdb_test "b ending-run.c:$break2_line" ".*Breakpoint 3.*ending-run.c, line $brea
 gdb_run_cmd
 gdb_test "" ".*Breakpoint.*1.*callee.*$break1_line.*" "run"
 
-gdb_test "cle" ".*Deleted breakpoints 1 2.*" "clear worked"
-gdb_test_multiple "i b" "cleared bp at line before routine" {
-    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" { 
-	fail "cleared bp at line before routine" 
+gdb_test "cle" "Deleted breakpoint 1 " "clear worked"
+gdb_test_multiple "i b" "cleared bp at stopped line" {
+    -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
+	fail $gdb_test_name
     }
-    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
-	pass "cleared bp at line before routine" 
+    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
+	pass $gdb_test_name
     }
 }
 
 # Test some other "clear" combinations
 #
-gdb_test "b ending-run.c:1" ".*Breakpoint.*4.*"
-gdb_test "b ending-run.c:$break1_line" ".*Note.*also.*Breakpoint.*5.*" "b ending-run.c:$break1_line, two"
+gdb_test "b ending-run.c:$break1_line" "Breakpoint 3 at ${::hex}.*" "b ending-run.c:$break1_line, two"
 gdb_test "cle ending-run.c:$break1_line" \
-	".*Deleted breakpoints 4 5.*" "Cleared 2 by line"
+	"Deleted breakpoint 3 " "Cleared 2 by line"
 
 gdb_test_multiple "info line ending-run.c:$break1_line" "" {
     -re ".*address (0x\[0-9a-fA-F]*).*$gdb_prompt $" {
         set line_nine $expect_out(1,string)
-        gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 6.*ending-run.c, line $break1_line.*"
-        gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 7.*" "breakpoint 7 at *ending-run.c:$break1_line"
-        gdb_test "cle" ".*Deleted breakpoints 6 7.*" "clear 2 by default"
+	gdb_test "b ending-run.c:$break1_line" ".*Breakpoint 4.*ending-run.c, line $break1_line.*"
+	gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 5.*" "breakpoint 7 at *ending-run.c:$break1_line"
+	gdb_test "cle" "Deleted breakpoints 4 5 " "clear 2 by default"
     }
     -re ".*$gdb_prompt $" {
         fail "need to fix test for new compile outcome"
@@ -90,7 +80,7 @@ gdb_test_multiple "i b" "all set to continue" {
     -re ".* breakpoint .* breakpoint .*$gdb_prompt $" {
         fail "all set to continue (didn't clear bps)" 
     }
-    -re ".*3.*main.*$break2_line.*$gdb_prompt $" {
+    -re ".*2.*main.*$break2_line.*$gdb_prompt $" {
         pass "all set to continue"
     }
     -re ".*$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.base/foll-exec-mode.exp b/gdb/testsuite/gdb.base/foll-exec-mode.exp
index 65054b530b3..56a2ffc723b 100644
--- a/gdb/testsuite/gdb.base/foll-exec-mode.exp
+++ b/gdb/testsuite/gdb.base/foll-exec-mode.exp
@@ -127,7 +127,7 @@ proc do_follow_exec_mode_tests { mode cmd infswitch } {
 	# past it.
 	#
 	if {$cmd == "continue"} {
-	    gdb_breakpoint "$execd_line"
+	    gdb_breakpoint "$execd_line" "allow-pending"
 	}
 
 	# Execute past the exec call.
diff --git a/gdb/testsuite/gdb.base/hbreak2.exp b/gdb/testsuite/gdb.base/hbreak2.exp
index 8f5735b790f..9bd5e2a0b41 100644
--- a/gdb/testsuite/gdb.base/hbreak2.exp
+++ b/gdb/testsuite/gdb.base/hbreak2.exp
@@ -296,7 +296,7 @@ if {![runto_main]} {
 #
 gdb_test_no_output "set breakpoint pending off"
 gdb_test "hbreak 999" \
-    "No line 999 in the current file." \
+    "^No compiled code for line 999 in the current file\\." \
     "hardware break on non-existent source line"
 
 # Run to the desired default location.  If not positioned here, the
diff --git a/gdb/testsuite/gdb.base/sepdebug.exp b/gdb/testsuite/gdb.base/sepdebug.exp
index ee9bea2045a..eb3515b84ae 100644
--- a/gdb/testsuite/gdb.base/sepdebug.exp
+++ b/gdb/testsuite/gdb.base/sepdebug.exp
@@ -296,7 +296,7 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
 #
 
 gdb_test_no_output "set breakpoint pending off"
-gdb_test "break 999" "No line 999 in the current file." \
+gdb_test "break 999" "^No compiled code for line 999 in the current file\\." \
     "break on non-existent source line"
 
 # Run to the desired default location. If not positioned here, the
diff --git a/gdb/testsuite/gdb.linespec/cpexplicit.exp b/gdb/testsuite/gdb.linespec/cpexplicit.exp
index 5c93c34ab4a..62033d5dac9 100644
--- a/gdb/testsuite/gdb.linespec/cpexplicit.exp
+++ b/gdb/testsuite/gdb.linespec/cpexplicit.exp
@@ -80,7 +80,7 @@ namespace eval $testfile {
     add linespecs "-function myclass::myfunction -line 3" $location(normal)
     add linespecs "-function myclass::myfunction -label top -line 3" \
 	$location(top)
-    add linespecs "-line 3" $location(normal)
+    add linespecs "-line 25" $location(normal)
     add linespecs "-function myclass::operator," $location(operator)
     add linespecs "-function 'myclass::operator,'" $location(operator)
     add linespecs "-function \"myclass::operator,\"" $location(operator)
diff --git a/gdb/testsuite/gdb.linespec/explicit.exp b/gdb/testsuite/gdb.linespec/explicit.exp
index 625f9cee0fc..60183e98e1e 100644
--- a/gdb/testsuite/gdb.linespec/explicit.exp
+++ b/gdb/testsuite/gdb.linespec/explicit.exp
@@ -86,7 +86,7 @@ namespace eval $testfile {
     # These are also not yet supported; -line is silently ignored.
     add linespecs "-function myfunction -line 3" $location(normal)
     add linespecs "-function myfunction -label top -line 3" $location(top)
-    add linespecs "-line 3" $location(normal)
+    add linespecs "-line 25" $location(normal)
 
     # Fire up gdb.
     if {![runto_main]} {
diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
new file mode 100644
index 00000000000..93c43838312
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.c
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022-2024 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/>.  */
+
+/* The section where THE_LIB_PATH is not defined is compiled as a shared
+   library.  The rest is compiled as the main executable (which loads the
+   shared library.  */
+
+#if !defined(THE_LIB_PATH)
+
+void
+the_lib_func (void)
+{
+  static int x;
+  /* break here */
+  x++;
+}
+
+#else
+#include <dlfcn.h>
+#include <assert.h>
+#include <stdlib.h>
+
+int
+main (void)
+{
+  void *lib = dlopen (THE_LIB_PATH, RTLD_NOW);
+  assert (lib != NULL);
+
+  void (*the_lib_func) (void) = dlsym (lib, "the_lib_func");
+  assert (the_lib_func != NULL);
+
+  the_lib_func ();
+
+  return 0;
+}
+
+#endif
diff --git a/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
new file mode 100644
index 00000000000..946304af84d
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/line-breakpoint-outside-function.exp
@@ -0,0 +1,55 @@
+# Copyright 2022-2024 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 that placing a line breakpoint outside a function results in a pending
+# breakpoint.  More importantly, that it does "drift" and place a
+# breakpoint on the next function.
+#
+# See the .c file for more details.
+
+standard_testfile
+
+set shlib_path [standard_output_file ${testfile}-lib.so]
+if {[build_executable "build shlib" $shlib_path $srcfile {debug shlib}]} {
+    return
+}
+
+set opts [list debug shlib_load additional_flags=-DTHE_LIB_PATH="${shlib_path}"]
+if {[build_executable "failed to prepare" ${testfile} ${srcfile} $opts]} {
+    return
+}
+
+proc do_test {} {
+    clean_restart $::binfile
+
+    # To make things easier, just so we don't have to deal with the question.
+    gdb_test_no_output "set breakpoint pending on"
+
+    set lineno [gdb_get_line_number "break here"]
+    gdb_test "break $lineno" \
+	[multi_line \
+	     "No compiled code for line $lineno in the current file\\." \
+	     "Breakpoint 1 \\($lineno\\) pending\\."] \
+	"breakpoint on a line outside any function"
+
+    gdb_run_cmd
+    gdb_test_multiple "" "stop on lib function breakpoint" {
+	-re -wrap "Breakpoint 1, the_lib_func .*29.*x\\+\\+.*" {
+	    pass $gdb_test_name
+	}
+    }
+}
+
+do_test
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.c b/gdb/testsuite/gdb.linespec/ls-errs.c
index 73b06fc7876..1dfccab2863 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.c
+++ b/gdb/testsuite/gdb.linespec/ls-errs.c
@@ -21,6 +21,16 @@ myfunction (int aa)
   int i;
 
   i = aa + 42;
+
+  /* These lines are intentionally left blank such that the tests trying
+     to place breakpoints at line -10 relative to the "set.breakpoint.here"
+     line below land on a valid breakpoint location, inside the function.  */
+
+
+
+
+
+
   return i;    /* set breakpoint here */
 }
 
diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
index 48c8a5ff056..58125f3626c 100644
--- a/gdb/testsuite/gdb.linespec/ls-errs.exp
+++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
@@ -71,8 +71,8 @@ proc do_test {lang} {
 	    "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
 	invalid_label "No label \"%s\" defined in function \"%s\"."
 	invalid_parm "invalid linespec argument, \"%s\""
-	invalid_offset "No line %d in the current file."
-	invalid_offset_f "No line %d in file \"%s\"."
+	invalid_offset "No compiled code for line %d in the current file."
+	invalid_offset_f "No compiled code for line %d in file \"%s\"."
 	malformed_line_offset "malformed line offset: \"%s\""
 	source_incomplete \
 	    "Source filename requires function, label, or line offset."
@@ -135,14 +135,14 @@ proc do_test {lang} {
 
     foreach x {1 +1 +100 -10} {
 	test_break "3 $x" unexpected_opt "number" $x
-	test_break "-line 3 $x" garbage $x
+	test_break "-line 34 $x" garbage $x
 	test_break "+10 $x" unexpected_opt "number" $x
 	test_break "-line +10 $x" garbage $x
 	test_break "-10 $x" unexpected_opt "number" $x
 	test_break "-line -10 $x" garbage $x
     }
 
-    foreach x {3 +10 -10} {
+    foreach x {34 +10 -10} {
 	test_break "$x foo" unexpected_opt "string" "foo"
 	test_break "-line $x foo" garbage "foo"
     }
@@ -207,12 +207,12 @@ proc do_test {lang} {
 
     test_break "${srcfile}::" invalid_function "${srcfile}::"
     test_break "$srcfile:3 1" unexpected_opt "number" "1"
-    test_break "-source $srcfile -line 3 1" garbage "1"
+    test_break "-source $srcfile -line 34 1" garbage "1"
     test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
-    test_break "-source $srcfile -line 3 +100" garbage "+100"
+    test_break "-source $srcfile -line 34 +100" garbage "+100"
     test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
     test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
-    test_break "-source $srcfile -line 3 foo" garbage "foo"
+    test_break "-source $srcfile -line 34 foo" garbage "foo"
 
     foreach x $invalid_offsets {
 	test_break "$srcfile:$x" invalid_offset_f $x $srcfile
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index c44477c326a..934690db2a1 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -743,7 +743,9 @@ proc_with_prefix test_bkpt_explicit_loc {} {
 	"No source file named foo.*" \
 	"set invalid explicit breakpoint by missing source and line"
     gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"900\")" \
-	"No line 900 in file \"$srcfile\".*" \
+	[multi_line \
+	     "^No compiled code for line 900 in file \"$srcfile\"\\." \
+	     "Breakpoint $::decimal \[^\r\n\]+ pending\\."] \
 	"set invalid explicit breakpoint by source and invalid line"
     gdb_test "python bp1 = gdb.Breakpoint (function=\"blah\")" \
 	"Function \"blah\" not defined.*" \
diff --git a/gdb/testsuite/gdb.trace/tfind.exp b/gdb/testsuite/gdb.trace/tfind.exp
index 9d1f6e32426..c6ac80bfdb5 100644
--- a/gdb/testsuite/gdb.trace/tfind.exp
+++ b/gdb/testsuite/gdb.trace/tfind.exp
@@ -342,10 +342,10 @@ gdb_test "disassemble gdb_c_test" \
     "8.36: trace disassembly"
 
 gdb_test "tfind line 0" \
-	"out of range.*|failed to find.*|No line 0 in .*" \
+	"out of range.*|failed to find.*|No compiled code for line 0 in .*" \
 	"8.18: tfind line 0"
 gdb_test "tfind line 32767" \
-	"out of range.*|failed to find.*|No line 32767 in .*" \
+	"out of range.*|failed to find.*|No compiled code for line 32767 in .*" \
 	"8.27: tfind line 32767"
 gdb_test "tfind line NoSuChFiLe.c:$baseline" \
 	"No source file named.*" \
diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
index 96596ea0833..688980c78f7 100644
--- a/gdb/testsuite/gdb.trace/tracecmd.exp
+++ b/gdb/testsuite/gdb.trace/tracecmd.exp
@@ -64,7 +64,8 @@ gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2.
 # 1.2 trace invalid source line
 gdb_delete_tracepoints
 gdb_test_no_output "set breakpoint pending off"
-gdb_test "trace $srcfile:99999" "No line 99999 in file \".*$srcfile\"." \
+gdb_test "trace $srcfile:99999" \
+	"No compiled code for line 99999 in file \".*$srcfile\"\\." \
 	"1.2a: trace invalid line in sourcefile"
 gdb_test "info trace" "No tracepoints.*" \
 	"1.2b: reject invalid line in srcfile"

Re: [PATCH] gdb: reject inserting breakpoints between functions

[Simon Marchi]

On 2024-05-01 05:47, Andrew Burgess wrote:
> Klaus Gerlicher <klaus.gerlicher@intel.com> writes:
> 
>> Hi Simon,
>>
>> I verified that your patch addresses many of the issues we would also
>> like to solve in this area.
>>
>> It appears this has not published yet and it seems to be more than 2
>> years old.
>>
>> Could you please tell me if there are any plans to commit this?
> 
> Given I reviewed this once upon a time, I still had my review branch
> kicking around.
> 
> I rebased onto something close to HEAD of master.  I addressed the minor
> nits I pointed out in my review.  I'd be happy to see this merged once
> my full regression run (still on going) has completed.
> 
> I think I'd like Simon to give a +1 before I pushed this though.
> 
> Thanks,
> Andrew

Thanks for reminding me about this, it clearly fell through the cracks.
And thanks Andrew for following up on it.  What you posted looks good to
me.  We discussed it internally, Pedro suggested to maybe wait after the
GDB 15 branch is created before pushing this one, to give it more
testing time in master before it reaches a release.

Simon

Re: [PATCH] gdb: reject inserting breakpoints between functions

[Andrew Burgess]

Simon Marchi <simark@simark.ca> writes:

> On 2024-05-01 05:47, Andrew Burgess wrote:
>> Klaus Gerlicher <klaus.gerlicher@intel.com> writes:
>> 
>>> Hi Simon,
>>>
>>> I verified that your patch addresses many of the issues we would also
>>> like to solve in this area.
>>>
>>> It appears this has not published yet and it seems to be more than 2
>>> years old.
>>>
>>> Could you please tell me if there are any plans to commit this?
>> 
>> Given I reviewed this once upon a time, I still had my review branch
>> kicking around.
>> 
>> I rebased onto something close to HEAD of master.  I addressed the minor
>> nits I pointed out in my review.  I'd be happy to see this merged once
>> my full regression run (still on going) has completed.
>> 
>> I think I'd like Simon to give a +1 before I pushed this though.
>> 
>> Thanks,
>> Andrew
>
> Thanks for reminding me about this, it clearly fell through the cracks.
> And thanks Andrew for following up on it.  What you posted looks good to
> me.  We discussed it internally, Pedro suggested to maybe wait after the
> GDB 15 branch is created before pushing this one, to give it more
> testing time in master before it reaches a release.

Fine with me.  Feel free to pick this back up yourself if it's still of
interest to you.  I'll add it to my "todo" pile and revisit in a month
or so if I don't see any activity.

Thanks,
Andrew