[PATCH 3/3] gdb: Don't remove duplicate entries from the line table
Andrew Burgess
andrew.burgess@embecosm.com
Mon Mar 23 17:30:19 GMT 2020
In this commit:
commit 8c95582da858ac981f689a6f599acacb8c5c490f
Date: Mon Dec 30 21:04:51 2019 +0000
gdb: Add support for tracking the DWARF line table is-stmt field
A change was made in buildsym_compunit::record_line to remove
duplicate line table entries in some cases. This was an invalid
change, as these duplicate line table entries are used in _some_ cases
as part of prologue detection (see skip_prologue_using_sal).
It might be possible to identify those line table entries that are
required by skip_prologue_using_sal and only keep those duplicates
around, however, I have not done this here. The original duplicate
removal was done because (a) it was easy to implement, and (b) it
seemed obviously harmless.
As (b) is now known to be false, and implementation would be more
complex, and so (a) is also false. As such, it seems better to keep
all duplicates until an actual reason presents itself for why we
should remove any.
The original regression was spotted on RISC-V, which makes use of
skip_prologue_using_sal as part of riscv_skip_prologue. Originally I
created the test gdb.dwarf2/dw2-inline-small-func.exp, however, this
test will not compile on RISC-V as this target doesn't support
.uleb128 or .sleb128 assembler directives containing complex
expressions. As a result I added the gdb.opt/inline-small-func.exp
test, which exposes the bug on RISC-V, but obviously depends on the
compiler to produce specific DWARF information in order to expose the
bug. Still this test does ensure we always get the desired result,
even if the DWARF changes.
Originally the gdb.dwarf2/dw2-inline-small-func.exp test passed on
x86-64 even with the duplicate line table entries incorrectly
removed. The reason for this is that when a compilation unit doesn't
have a 'producer' string then skip_prologue_using_sal is not used,
instead the prologue is always skipped using analysis of the assembler
code.
However, for Clang on x86-64 skip_prologue_using_sal is used, so I
modified the gdb.dwarf2/dw2-inline-small-func.exp test to include a
'producer' string that names the Clang compiler. With this done the
test would fail on x86-64.
One thing to note is that the gdb.opt/inline-small-func.exp test might
fail on some targets. For example, if we compare sparc to risc-v by
looking at sparc32_skip_prologue we see that this function doesn't use
skip_prologue_using_sal, but instead uses find_pc_partial_function
directly. I don't know the full history behind why the code is like
it is, but it feels like sparc32_skip_prologue is an attempt to
duplicate some of the functionality of skip_prologue_using_sal, but
without all of the special cases. If this is true then the new test
could easily fail on this target, this would suggest that sparc should
consider switching to use skip_prologue_using_sal like risc-v does.
gdb/ChangeLog:
* buildsym.c (buildsym_compunit::record_line): Remove
deduplication code.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/dw2-inline-small-func-lbls.c: New file.
* gdb.dwarf2/dw2-inline-small-func.c: New file.
* gdb.dwarf2/dw2-inline-small-func.exp: New file.
* gdb.dwarf2/dw2-inline-small-func.h: New file.
* gdb.opt/inline-small-func.c: New file.
* gdb.opt/inline-small-func.exp: New file.
* gdb.opt/inline-small-func.h: New file.
---
gdb/ChangeLog | 7 +
gdb/buildsym.c | 14 --
gdb/testsuite/ChangeLog | 10 ++
.../gdb.dwarf2/dw2-inline-small-func-lbls.c | 37 +++++
gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.c | 22 +++
gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp | 158 +++++++++++++++++++++
gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.h | 21 +++
gdb/testsuite/gdb.opt/inline-small-func.c | 22 +++
gdb/testsuite/gdb.opt/inline-small-func.exp | 60 ++++++++
gdb/testsuite/gdb.opt/inline-small-func.h | 21 +++
10 files changed, 358 insertions(+), 14 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-small-func-lbls.c
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.c
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.h
create mode 100644 gdb/testsuite/gdb.opt/inline-small-func.c
create mode 100644 gdb/testsuite/gdb.opt/inline-small-func.exp
create mode 100644 gdb/testsuite/gdb.opt/inline-small-func.h
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 7155db34b08..aea594ebca4 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -681,20 +681,6 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
m_have_line_numbers = true;
}
- if (subfile->line_vector->nitems > 0)
- {
- /* If we have a duplicate for the previous entry then ignore the new
- entry, except, if the new entry is setting the is_stmt flag, then
- ensure the previous entry respects the new setting. */
- e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
- if (e->line == line && e->pc == pc)
- {
- if (is_stmt && !e->is_stmt)
- e->is_stmt = 1;
- return;
- }
- }
-
if (subfile->line_vector->nitems + 1 >= subfile->line_vector_length)
{
subfile->line_vector_length *= 2;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func-lbls.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func-lbls.c
new file mode 100644
index 00000000000..5772131569c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func-lbls.c
@@ -0,0 +1,37 @@
+/* Copyright 2020 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/>. */
+
+/* Used to insert labels with which we can build a fake line table. */
+#define LL(N) asm ("line_label_" #N ": .globl line_label_" #N)
+
+volatile int var;
+volatile int bar;
+
+/* Generate some code to take up some space. */
+#define FILLER do { \
+ var = 99; \
+} while (0)
+
+int
+main ()
+{ /* main prologue */
+ asm ("main_label: .globl main_label");
+ LL (1);
+ FILLER;
+ LL (2);
+ FILLER;
+ LL (3);
+ return 0; /* main end */
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.c b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.c
new file mode 100644
index 00000000000..e2095e36625
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.c
@@ -0,0 +1,22 @@
+/* Copyright 2020 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "dw2-inline-small-func.h"
+
+int
+main ()
+{ /* caller: before call. */
+ callee (); /* caller: the call. */
+} /* caller: after call. */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp
new file mode 100644
index 00000000000..777db062b37
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.exp
@@ -0,0 +1,158 @@
+# Copyright 2020 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/>.
+
+# Check for an issue in GDB where buildsym_compunit::record_line was
+# removing duplicate line table entries, but skip_prologue_using_sal
+# depends on these duplicates to spot the end of the prologue.
+#
+# When the de-duplication was added this regression was not spotted as
+# it requires a particular combination of a (very) small function
+# being inlined into an also very small outer function.
+#
+# This test recreates the exact combination of line table entries that
+# were seen in the original test using the Dejagnu DWARF compiler.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+# The .c files use __attribute__.
+if [get_compiler_info] {
+ return -1
+}
+if !$gcc_compiled {
+ return 0
+}
+
+standard_testfile dw2-inline-small-func-lbls.c dw2-inline-small-func.S \
+ dw2-inline-small-func.c dw2-inline-small-func.h
+
+# Extract the start, length, and end for function called NAME and
+# create suitable variables in the callers scope.
+proc get_func_info { name } {
+ global srcdir subdir srcfile
+
+ upvar 1 "${name}_start" func_start
+ upvar 1 "${name}_len" func_len
+ upvar 1 "${name}_end" func_end
+
+ lassign [function_range ${name} [list ${srcdir}/${subdir}/$srcfile] {debug optimize=-O1}] \
+ func_start func_len
+ set func_end "$func_start + $func_len"
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ global srcdir subdir srcfile srcfile3 srcfile4
+ declare_labels lines_label callee_subprog_label
+
+ get_func_info main
+
+ cu {} {
+ # It is important that the producer here be 'clang' as, at the
+ # time of writing this, GCC for x86-64 doesn't make use of
+ # skip_prologue_using_sal, while clang does.
+ compile_unit {
+ {producer "clang xxxx" }
+ {language @DW_LANG_C}
+ {name ${srcfile3}}
+ {low_pc 0 addr}
+ {stmt_list ${lines_label} DW_FORM_sec_offset}
+ } {
+ callee_subprog_label: subprogram {
+ {external 1 flag}
+ {name callee}
+ {inline 3 data1}
+ }
+ subprogram {
+ {external 1 flag}
+ {name main}
+ {low_pc $main_start addr}
+ {high_pc "$main_start + $main_len" addr}
+ } {
+ inlined_subroutine {
+ {abstract_origin %$callee_subprog_label}
+ {low_pc line_label_1 addr}
+ {high_pc line_label_2 addr}
+ {call_file 1 data1}
+ {call_line 21 data1}
+ }
+ }
+ }
+ }
+
+ lines {version 2 default_is_stmt 1} lines_label {
+ include_dir "${srcdir}/${subdir}"
+ file_name "$srcfile3" 1
+ file_name "$srcfile4" 1
+
+ set f1_l1 [gdb_get_line_number "caller: before call" $srcfile3]
+ set f1_l2 [gdb_get_line_number "caller: the call" $srcfile3]
+ set f1_l3 [gdb_get_line_number "caller: after call" $srcfile3]
+
+ set f2_l1 [gdb_get_line_number "callee: body" $srcfile4]
+
+ program {
+ {DW_LNE_set_address line_label_1}
+ {DW_LNS_advance_line [expr $f1_l1 - 1]}
+ {DW_LNS_copy}
+
+ {DW_LNS_advance_line [expr ${f1_l2} - ${f1_l1}]}
+ {DW_LNS_copy}
+
+ {DW_LNS_set_file 2}
+ {DW_LNS_advance_line [expr ${f2_l1} - ${f1_l2}]}
+ {DW_LNS_copy}
+
+ {DW_LNS_negate_stmt}
+ {DW_LNS_copy}
+
+ {DW_LNS_set_file 1}
+ {DW_LNE_set_address line_label_2}
+ {DW_LNS_advance_line [expr ${f1_l3} - ${f2_l1}]}
+ {DW_LNS_copy}
+
+ {DW_LNE_set_address line_label_3}
+ {DW_LNS_copy}
+ {DW_LNE_end_sequence}
+ }
+ }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug optimize=-O1}] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+# Delete all breakpoints so that the output of "info breakpoints"
+# below will only contain a single breakpoint.
+delete_breakpoints
+
+# Place a breakpoint within the function in the header file.
+set linenum [gdb_get_line_number "callee: body" $srcfile4]
+gdb_breakpoint "${srcfile4}:${linenum}"
+
+# Check that the breakpoint was placed where we expected. It should
+# appear at the requested line. When the bug in GDB was present the
+# breakpoint would be placed on one of the following lines instead.
+gdb_test "info breakpoints" \
+ ".* in callee at \[^\r\n\]+${srcfile4}:${linenum}\\y.*"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.h b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.h
new file mode 100644
index 00000000000..69c67af6011
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-small-func.h
@@ -0,0 +1,21 @@
+/* Copyright 2020 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 counter = 42;
+
+inline void
+callee () {
+ counter = 0; /* callee: body. */
+}
diff --git a/gdb/testsuite/gdb.opt/inline-small-func.c b/gdb/testsuite/gdb.opt/inline-small-func.c
new file mode 100644
index 00000000000..902674e8773
--- /dev/null
+++ b/gdb/testsuite/gdb.opt/inline-small-func.c
@@ -0,0 +1,22 @@
+/* Copyright 2020 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "inline-small-func.h"
+
+int
+main ()
+{ /* caller: before call. */
+ callee (); /* caller: the call. */
+} /* caller: after call. */
diff --git a/gdb/testsuite/gdb.opt/inline-small-func.exp b/gdb/testsuite/gdb.opt/inline-small-func.exp
new file mode 100644
index 00000000000..dfe046ad723
--- /dev/null
+++ b/gdb/testsuite/gdb.opt/inline-small-func.exp
@@ -0,0 +1,60 @@
+# Copyright 2020 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/>.
+
+# Check for an issue in GDB where buildsym_compunit::record_line was
+# removing duplicate line table entries, but skip_prologue_using_sal
+# depends on these duplicates to spot the end of the prologue.
+#
+# When the de-duplication was added this regression was not spotted as
+# it requires a particular combination of a (very) small function
+# being inlined into an also very small outer function.
+#
+# See also gdb.dwarf/dw2-inline-small-func.exp for a version of this
+# test that makes use of the Dejagnu DWARF compiler.
+#
+# This test simply compiles with optimization and checks that GDB can
+# do something suitable with the compiled binary. Problems with this
+# test are most likely to occur when GDB asks the target specific code
+# to skip the prologue (gdbarch_skip_prologue). Some targets make use
+# of skip_prologue_using_sal, which should be fine, however, some
+# targets make a poor attempt to duplicate parts of
+# skip_prologue_using_sal, these targets could easily fail this test.
+# This is not (necessarily) a problem with this test, but could
+# indicate a weakness with the target in question.
+
+standard_testfile inline-small-func.c inline-small-func.h
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile] {debug optimize=-O1}] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+# Delete all breakpoints so that the output of "info breakpoints"
+# below will only contain a single breakpoint.
+delete_breakpoints
+
+# Place a breakpoint within the function in the header file.
+set linenum [gdb_get_line_number "callee: body" $srcfile2]
+gdb_breakpoint "${srcfile2}:${linenum}"
+
+# Check that the breakpoint was placed where we expected. It should
+# appear at the requested line. When the bug in GDB was present the
+# breakpoint would be placed on one of the following lines instead.
+gdb_test "info breakpoints" \
+ ".* in callee at \[^\r\n\]+${srcfile2}:${linenum}\\y.*"
diff --git a/gdb/testsuite/gdb.opt/inline-small-func.h b/gdb/testsuite/gdb.opt/inline-small-func.h
new file mode 100644
index 00000000000..69c67af6011
--- /dev/null
+++ b/gdb/testsuite/gdb.opt/inline-small-func.h
@@ -0,0 +1,21 @@
+/* Copyright 2020 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 counter = 42;
+
+inline void
+callee () {
+ counter = 0; /* callee: body. */
+}
--
2.14.5
More information about the Gdb-patches
mailing list