diff options
author | Andrew Burgess <andrew.burgess@embecosm.com> | 2020-03-23 12:40:24 +0000 |
---|---|---|
committer | Andrew Burgess <andrew.burgess@embecosm.com> | 2020-04-02 17:44:03 +0100 |
commit | c90d28ac8903c5781b1a82e487072081383fd7b9 (patch) | |
tree | c736279b1f48a4eb01c1f43d866d84fbfb4ee3b5 /gdb/buildsym.c | |
parent | 34e9a9fa0a18fc544bc8ec3dc9e02096be1e4335 (diff) | |
download | gdb-c90d28ac8903c5781b1a82e487072081383fd7b9.zip gdb-c90d28ac8903c5781b1a82e487072081383fd7b9.tar.gz gdb-c90d28ac8903c5781b1a82e487072081383fd7b9.tar.bz2 |
gdb: Don't remove duplicate entries from the line table
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.
Diffstat (limited to 'gdb/buildsym.c')
-rw-r--r-- | gdb/buildsym.c | 14 |
1 files changed, 0 insertions, 14 deletions
diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 46c5bb1..fe07103 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 >= subfile->line_vector_length) { subfile->line_vector_length *= 2; |