wrong line number in breakpoint location

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.
This commit is contained in:
Joel Brobecker
2018-01-21 23:14:50 -05:00
parent 3b76a4a069
commit a9e408182d
10 changed files with 137 additions and 12 deletions

View 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

View 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);
}

View 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

View File

@ -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;
}

View File

@ -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\\."

View File

@ -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 */

View 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 4 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"

View 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"

View 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" \

View 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]" \