wrong line number in breakpoint location
authorJoel Brobecker <brobecker@adacore.com>
Mon, 22 Jan 2018 04:14:50 +0000 (23:14 -0500)
committerJoel Brobecker <brobecker@adacore.com>
Mon, 22 Jan 2018 04:14:50 +0000 (23:14 -0500)
Consider the following situation, where we have one file containing...

    $ cat -n body.inc
         1  i = i + 1;

... we include that file from some code, like so:

    $ cat -n cat -n small.c
        [...]
        17  int
        18  next (int i)
        19  {
        20  #include "body.inc"
        21    return i;
        22  }

When trying to insert a breakpoint on line 18, for instance:

    (gdb) b small.c:18
    Breakpoint 1 at 0x40049f: file body.inc, line 18.
                                                  ^^
                                                  ||

Here, the issue is that GDB reports the breakpoint to be in file
body.inc, which is true, but with the line number that corresponding
to the user-requested location, which is not correct.

Although the simple reproducer may look slightly artificial,
the above is simply one way to reproduce the same issue observed
when trying to insert a breakpoint on a function provided in
a .h files and then subsequently inlined in a C file.

What happens is the following:

  1. We resolve the small.c:18 linespec into a symtab_and_line which
     has "small.c" and 18 as the symtab and line number.

  2. Next, we call skip_prologue_sal, which calculates the PC
     past the prologue, and updates the symtab_and_line: PC,
     but also symtab (now body.inc) and the new line (now 1).

  3. However, right after that, we do:

            /* Make sure the line matches the request, not what was
               found.  */
            intermediate_results.sals[i].line = val.line;

We should either restore both symtab and line, or leave the actual
line to match the actual symtab.  This patch chose the latter.
This introduces a few changes in a few tests, which required some
updates, but looking at those change, I believe them to be expected.

gdb/ChangeLog:

        * linespec.c (create_sals_line_offset): Remove code that preserved
        the symtab_and_line's line number.

gdb/testsuite/ChangeLog:

        * gdb.base/break-include.c, gdb.base/break-include.inc,
        gdb.base/break-include.exp: New files.
        * gdb.base/ending-run.exp: Minor adaptations due to the breakpoint's
        line number now being the actual line number where the breakpoint
        was inserted.
        * gdb.mi/mi-break.exp: Likewise.
        * gdb.mi/mi-reverse.exp: Likewise.
        * gdb.mi/mi-simplerun.exp: Ditto.

Tested on x86_64-linux.

gdb/ChangeLog
gdb/linespec.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/break-include.c [new file with mode: 0644]
gdb/testsuite/gdb.base/break-include.exp [new file with mode: 0644]
gdb/testsuite/gdb.base/break-include.inc [new file with mode: 0644]
gdb/testsuite/gdb.base/ending-run.exp
gdb/testsuite/gdb.mi/mi-break.exp
gdb/testsuite/gdb.mi/mi-reverse.exp
gdb/testsuite/gdb.mi/mi-simplerun.exp

index 0661218bfcd5286e27bdebcdbdb3a801dbad9eb1..095f6576bf427f1c85e44298caca9d470f7610a7 100644 (file)
@@ -1,3 +1,8 @@
+2018-01-22  Joel Brobecker  <brobecker@adacore.com>
+
+       * linespec.c (create_sals_line_offset): Remove code that preserved
+       the symtab_and_line's line number.
+
 2018-01-21  Andrew Burgess  <andrew.burgess@embecosm.com>
 
        * varobj.c (varobj_create): Don't set valid_block when creating a
index 85f412766c8f93f036bb87ef5bdaf642cf86a9c5..1236b3f4754af8a90ecbb1190a7de6f359c60211 100644 (file)
@@ -2246,9 +2246,6 @@ create_sals_line_offset (struct linespec_state *self,
 
            if (self->funfirstline)
              skip_prologue_sal (&intermediate_results[i]);
-           /* Make sure the line matches the request, not what was
-              found.  */
-           intermediate_results[i].line = val.line;
            add_sal_to_sals (self, &values, &intermediate_results[i],
                             sym ? SYMBOL_NATURAL_NAME (sym) : NULL, 0);
          }
index 72e1596731b0e75bc8d2d55af5c19ffdce948567..f9fe3ab02dcbfa558816ea3cc1176993f09d00a7 100644 (file)
@@ -1,3 +1,14 @@
+2018-01-22  Joel Brobecker  <brobecker@adacore.com>
+
+       * gdb.base/break-include.c, gdb.base/break-include.inc,
+       gdb.base/break-include.exp: New files.
+       * gdb.base/ending-run.exp: Minor adaptations due to the breakpoint's
+       line number now being the actual line number where the breakpoint
+       was inserted.
+       * gdb.mi/mi-break.exp: Likewise.
+       * gdb.mi/mi-reverse.exp: Likewise.
+       * gdb.mi/mi-simplerun.exp: Ditto.
+
 2018-01-21  Andrew Burgess  <andrew.burgess@embecosm.com>
 
        * gdb.python/py-mi.exp: Don't expect a thread-id for floating
diff --git a/gdb/testsuite/gdb.base/break-include.c b/gdb/testsuite/gdb.base/break-include.c
new file mode 100644 (file)
index 0000000..bcf6cce
--- /dev/null
@@ -0,0 +1,57 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016-2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int next (int i);
+
+int
+main (void)
+{
+  int result = -1;
+
+  result = next (result);
+  return result;
+}
+
+/* The following function's implementation starts by including a file
+   (break-include.inc) which contains a copyright header followed by
+   a single C statement.  When we place a breakpoint on the line where
+   the function name is declared, we expect GDB to skip the function's
+   prologue, and insert the breakpoint on the first line of "user" code
+   for that function, which we have set up to be that single statement
+   break-include.inc provides.
+
+   The purpose of this testcase is to verify that, when we insert
+   that breakpoint, GDB reports the location as being in that include
+   file, but also using the correct line number inside that include
+   file -- NOT the line number we originally used to insert the
+   breakpoint, nor the location where the file is included from.
+   In order to verify that GDB shows the right line number, we must
+   be careful that this first statement located in break-include.inc
+   and our function are not on the same line number.  Otherwise,
+   we could potentially have a false PASS.
+
+   This is why we implement the following function as far away
+   from the start of this file as possible, as we know that
+   break-include.inc is a fairly short file (copyright header
+   and single statement only).  */
+
+int
+next (int i)  /* break here */
+{
+#include "break-include.inc"
+  return i;
+}
diff --git a/gdb/testsuite/gdb.base/break-include.exp b/gdb/testsuite/gdb.base/break-include.exp
new file mode 100644 (file)
index 0000000..bc54c29
--- /dev/null
@@ -0,0 +1,34 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2016-2018 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp $testfile] } {
+    return -1
+}
+
+set bp_line [gdb_get_line_number "break here" ${testfile}.c]
+set bp_line_actual [gdb_get_line_number "ANCHOR" ${testfile}.inc]
+
+gdb_test "break $testfile.c:$bp_line" \
+    ".*Breakpoint.*$testfile.inc, line $bp_line_actual\\."
+
+# Might as well verify that breaking on function "next" gives
+# the same result...
+
+gdb_test "break next" \
+    ".*Breakpoint.*$testfile.inc, line $bp_line_actual\\."
diff --git a/gdb/testsuite/gdb.base/break-include.inc b/gdb/testsuite/gdb.base/break-include.inc
new file mode 100644 (file)
index 0000000..37122d7
--- /dev/null
@@ -0,0 +1,18 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016-2018 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/>.  */
+
+i = i + 1;  /* ANCHOR */
index 32a67fadb731ea5fbf848569925d9d4334751160..27c080166a651dcc725867bf806109f44e2b43ba 100644 (file)
@@ -68,14 +68,14 @@ gdb_test_multiple "i b" "cleared bp at line before routine" {
 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 "cle ending-run.c:$break1_line" \
-       ".*Deleted breakpoint 5.*" "Cleared 2 by line"
+       ".*Deleted breakpoints 4 5.*" "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 "cle" ".*Deleted breakpoints 6 7.*" "clear 2 by default"
     }
     -re ".*$gdb_prompt $" {
         fail "need to fix test for new compile outcome"
index a49e83b93e6505a2eb70b3077e433e82987d8877..920a2bf2c5c4f1fad9c29907fb491d5b9cacfe92 100644 (file)
@@ -49,7 +49,9 @@ set fullname "fullname=\"${fullname_syntax}${srcfile}\""
 proc test_tbreak_creation_and_listing {} {
     global srcfile
     global line_callee4_head
+    global line_callee4_body
     global line_callee3_head
+    global line_callee3_body
     global line_callee2_body
     global line_main_body
 
@@ -75,7 +77,7 @@ proc test_tbreak_creation_and_listing {} {
     lappend bps [mi_create_breakpoint "-t basics.c:$line_callee3_head" \
                     "insert temp breakpoint at basics.c:\$line_callee3_head" \
                     -number 3 -disp del -func callee3 -file ".*basics.c" \
-                    -line $line_callee3_head]
+                    -line $line_callee3_body]
 
     # Getting the quoting right is tricky.
     # That is "\"<file>\":$line_callee4_head"
@@ -83,7 +85,7 @@ proc test_tbreak_creation_and_listing {} {
                     "-t \"\\\"${srcfile}\\\":$line_callee4_head\"" \
                     "insert temp breakpoint at \"<fullfilename>\":\$line_callee4_head" \
                     -number 4 -disp del -func callee4 -file ".*basics.c" \
-                    -line $line_callee4_head]
+                    -line $line_callee4_body]
 
     mi_gdb_test "666-break-list" \
        "666\\\^done,[mi_make_breakpoint_table $bps]" \
@@ -319,6 +321,7 @@ proc test_breakpoint_commands {} {
 proc test_explicit_breakpoints {} {
     global srcfile
     global line_callee3_head line_callee4_head
+    global line_callee3_body line_callee4_body
     global line_callee2_body line_main_body
 
     mi_delete_breakpoints
@@ -349,13 +352,13 @@ proc test_explicit_breakpoints {} {
     lappend bps \
        [mi_create_breakpoint "-t --source $srcfile --line $line_callee3_head" \
             "insert temp explicit breakpoint at $srcfile:$line_callee3_head" \
-            -func callee3 -file ".*$srcfile" -line $line_callee3_head]
+            -func callee3 -file ".*$srcfile" -line $line_callee3_body]
 
     lappend bps \
        [mi_create_breakpoint \
             "-t --source \"$srcfile\" --line  $line_callee4_head" \
             "insert temp explicit breakpoint at \"$srcfile\":$line_callee4_head" \
-            -func callee4 -file ".*$srcfile" -line $line_callee4_head]
+            -func callee4 -file ".*$srcfile" -line $line_callee4_body]
 
     mi_gdb_test "-break-list" "\\^done,[mi_make_breakpoint_table $bps]" \
        "list of explicit breakpoints"
index 3276c5efa54b26f15f9fb1bc2f97175e4ac9cae9..6677b84908625c5f206efdae29bd619d8971504a 100644 (file)
@@ -153,7 +153,7 @@ proc test_controlled_execution_reverse {} {
     mi_create_breakpoint "-t basics.c:$line_callee3_head" \
        "insert temp breakpoint at basics.c:$line_callee3_head" \
        -number 3 -disp del -func callee3 -file ".*basics.c" \
-       -line $line_callee3_head
+       -line $line_callee3_body
 
     mi_execute_to "exec-continue --reverse" \
         "breakpoint-hit" "callee3" \
index 561c7d92b2da56a3e777cc71e4ebf97fdda64df7..782ebde66f2169e517dccd32672d4dd432b99f83 100644 (file)
@@ -78,13 +78,13 @@ proc test_breakpoints_creation_and_listing {} {
     lappend bps [mi_create_breakpoint "basics.c:$line_callee3_head" \
                     "insert breakpoint at basics.c:\$line_callee3_head" \
                     -number 3 -func callee3 -file ".*basics.c" \
-                    -line $line_callee3_head]
+                    -line $line_callee3_body]
 
     lappend bps [mi_create_breakpoint \
                     "\"\\\"${srcfile}\\\":$line_callee4_head\"" \
                     "insert breakpoint at \"<fullfilename>\":\$line_callee4_head" \
                     -number 4 -func callee4 -file ".*basics.c" \
-                    -line $line_callee4_head]
+                    -line $line_callee4_body]
 
     mi_gdb_test "204-break-list" \
        "204\\^done,[mi_make_breakpoint_table $bps]" \