This patch fixes .debug_names verification for split DWARF with no type
units. It will print out an error for any name entries where we can't
locate the .dwo file. It finds the non skeleton unit and correctly
figures out the DIE offset in the .dwo file. If the non skeleton unit is
found and yet the skeleton unit has a DWO ID, an error will be emitted
showing we couldn't access the non-skeleton compile unit.
This patch adds support for verifying local type units in .debug_names
section. It adds a test to test if the TU index is valid, and a test
that tests that an error is found inside the name entry for a type unit.
We don't need to test all other errors in the name entry because these
are essentially identical to compile unit entries, they just use a
different DWARF unit offset index.
The result of the function cannot be correctly interpreted without
knowing the precise form type (a type signature needs to be looked up
very differently from a supplementary debug info reference). The
function sort of worked because the two reference types (unit-relative
and section-relative) that can be handled uniformly are also the most
common types of references, but this setup made it easy to write code
which does not support other kinds of reference (and if one tried to
support them, the result didn't look pretty --
https://github.com/llvm/llvm-project/pull/97423/files#r1676217081).
The split is based on the reference type classification from DWARFv5
(Section 7.5.5 Classes and Forms), and it should enable uniform (if
slightly more verbose) hadling. Note that this only affects users which
want more control of how (or if) the references are resolved. Users
which just want to access the referenced DIE can use the higher level
API (DWARFDie::GetAttributeValueAsReferencedDie) which returns (or will
return after #97423 is merged) the correct die for all reference types
(except for supplementary references, which we don't support right now).
In order to make tooling around dwarf health easier, I've added an `--verify-json` option to `llvm-dwarfdump --verify` that will spit out error summary data with counts to a JSON file.
I've added the same capability to `llvm-gsymutil` in a [different PR.](https://github.com/llvm/llvm-project/pull/81763)
The format of the json is:
``` json
{
"error-categories": {
"<first category description>": {"count": 1234},
"<next category description>": {"count":4321}
},
"error-count": 5555
}
```
for a clean run:
``` json
{
"error-categories": {},
"error-count": 0
}
```
---------
Co-authored-by: Kevin Frei <freik@meta.com>
The DWARF 5 debug_str_offsets section starts with a header, which must
be skipped in order to access the underlying `strp`s.
However, the verifier supports some pre-standardization version of this
section (with the same section name), which does not have a header. In
this case, the offsets start on the first byte of the section. More in
[1] and [2] about this legacy section.
How does The DWARF verifier figure out which version to use? It manually
reads the **first** header in debug_info and uses that. This is wrong
when multiple debug_str_offset sections have been linked together, in
particular it is wrong in the following two cases:
1. A standard DWARF 4 object file (i.e. no debug_str_offsets) linked
with a standard DWARF 5 object file.
2. A non-standard DWARF 4 object file (i.e. containing the header-less
debug_str_offsets section) linked with a standard DWARF 5 object file.
Based on discussions in https://github.com/llvm/llvm-project/pull/81210,
the legacy version is only possible with dwo files, and dwo files cannot
mix the legacy version with the dwarf 5 version. As such, we change the
verifier to only check the debug_info header in the case of dwo files.
If it sees a dwarf 4 version, it handles it the legacy way.
Note: the modified test was technically testing an unsupported
combination of dwarf version + non-dwo sections. To see why, simply note
that the test contained no `debug_info.dwo` sections, so the call to
DWARFObject::forEachInfoDWOSections was doing nothing. We were finding
the error through the "standard version", which shouldn't happen.
[1]: https://gcc.gnu.org/wiki/DebugFission
[2]: https://gcc.gnu.org/wiki/DebugFissionDWP
A line table whose sole entry is an end sequence should not have the
entry's file index verified, as that value corresponds to the initial
value of the state machine, not to a real file index. In DWARF 5, this
is particularly problematic as it uses 0-based indexing, and the state
machine specifies a starting index of 1; in other words, you'd need to
have _two_ files before such index became legal "by default".
A previous attempt to fix this problem was done [1], but it was too
specific in its condition, and did not capture all possible cases where
this issue can happen.
[1]: https://github.com/llvm/llvm-project/pull/77004
The amount and format of output from `llvm-dwarfdump --verify` makes it
quite difficult to know if a change to a tool that produces or modifies
DWARF is causing new problems, or is fixing existing problems. This diff
adds a categorized summary of issues found by the DWARF verifier, on by
default, at the bottom of the error output.
The change includes a new `--error-display` option with 4 settings:
* `--error-display=quiet`: Only display if errors occurred, but no
details or summary are printed.
* `--error-display=summary`: Only display the aggregated summary of
errors with no error detail.
* `--error-display=details`: Only display the detailed error messages
with no summary (previous behavior)
* `--error-display=full`: Display both the detailed error messages and
the aggregated summary of errors (the default)
I changed a handful of tests that were failing due to new output, adding
the flag to use the old behavior for all but a couple. For those two I
added the new aggregated output to the expected output of the test.
The `OutputCategoryAggregator` is a pretty simple little class that
@clayborg suggested to allow code to only be run to dump detail if it's
enabled, while still collating counts of the category. Knowing that the
lambda passed in is only conditionally executed is pretty important
(handling errors has to be done *outside* the lambda). I'm happy to move
this somewhere else (and change/improve it) to be more broadly useful if
folks would like.
---------
Co-authored-by: Kevin Frei <freik@meta.com>
This implements the ideas discussed in [1].
To summarize, this commit changes AsmPrinter so that it outputs
DW_IDX_parent information for debug_name entries. It will enable
debuggers to speed up queries for fully qualified types (based on a
DWARFDeclContext) significantly, as debuggers will no longer need to
parse the entire CU in order to inspect the parent chain of a DIE.
Instead, a debugger can simply take the parent DIE offset from the
accelerator table and peek at its name in the debug_info/debug_str
sections.
The implementation uses two types of DW_FORM for the DW_IDX_parent
attribute:
1. DW_FORM_ref4, which points to the accelerator table entry for the
parent.
2. DW_FORM_flag_present, when the entry has a parent that is not in the
table (that is, the parent doesn't have a name, or isn't allowed to be
in the table as per the DWARF spec). This is space-efficient, since it
takes 0 bytes.
The implementation works by:
1. Changing how abbreviations are encoded (so that they encode which
form, if
any, was used to encode IDX_Parent)
2. Creating an MCLabel per accelerator table entry, so that they may be
referred by IDX_parent references.
When all patches related to this are merged, we are able to show that
evaluating an expression such as:
```
lldb --batch -o 'b CodeGenFunction::GenerateCode' -o run -o 'expr Fn' -- \
clang++ -c -g test.cpp -o /dev/null
```
is far faster: from ~5000 ms to ~1500ms.
Building llvm-project + clang with and without this patch, and looking
at its impact on object file size:
```
ls -la $(find build_stage2_Debug_idx_parent_assert_dwarf5 -name \*.cpp.o) | awk '{s+=$5} END {printf "%\047d\n", s}'
11,507,327,592
-la $(find build_stage2_Debug_no_idx_parent_assert_dwarf5 -name \*.cpp.o) | awk '{s+=$5} END {printf "%\047d\n", s}'
11,436,446,616
```
That is, an increase of 0.62% in total object file size.
Looking only at debug_names:
```
$stage1_build/bin/llvm-objdump --section-headers $(find build_stage2_Debug_idx_parent_assert_dwarf5 -name \*.cpp.o) | grep __debug_names | awk '{s+="0x"$3} END {printf "%\047d\n", s}'
440,772,348
$stage1_build/bin/llvm-objdump --section-headers $(find build_stage2_Debug_no_idx_parent_assert_dwarf5 -name \*.cpp.o) | grep __debug_names | awk '{s+="0x"$3} END {printf "%\047d\n", s}'
369,867,920
```
That is an increase of 19%.
DWARF Linkers need to be changed in order to support this. This commit
already brings support to "base" linker, but it does not attempt to
modify the parallel linker. Accelerator entries refer to the
corresponding DIE offset, and this patch also requires the parent DIE
offset -- it's not clear how the parallel linker can access this. It may
be obvious to someone familiar with it, but it would be nice to get help
from its authors.
[1]:
https://discourse.llvm.org/t/rfc-improve-dwarf-5-debug-names-type-lookup-parsing-speed/74151/
If there is no file name in the prologue of a line table, the verifier
will try to verify the file index, which will be set to 1 by default.
This will cause the DWARF verifier to throw an error even if there is no
error.
rdar://114476503
rdar://114343624
ObjectiveC has its own extra accelerator table entries that are helpful for the
debugger. This patch relaxes the DWARFVerifier so that it accepts those in DWARF
5's debug_names.
Differential Revision: https://reviews.llvm.org/D159471
D155723 changed the return type of getNames to a SmallVector of a different
size. However, it failed to also update the declaration of the variable that is
returned in such function.
Differential Revision: https://reviews.llvm.org/D157881
LLDB can benefit from having the base name of functions (i.e. without any
template parameters) as an entry into accelerator tables pointing back in the
DIE for the corresponding function specialization. In fact, some LLDB
functionality is only possible when those entries are present.
The DWARFLinker has been adding such entries for a while now, both with
apple_names and with debug_names. However, this has two side effects:
1. Some LLDB functionality is only possible when dsym bundles are present (i.e.
the linker touched the debug info).
2. The DWARFVerifier doesn't accept debug_name sections created by the linker,
as such names are (usually) neither the AT_name nor the AT_linkage_name of the
DIE.
Based on recent discussion [1], and because the DWARF 5 spec says that:
> A producer may choose to implement additional rules for what names are placed
> in the index
This patch relaxes the checks on the verifier to allow for simplified template
names in the accelerator table. To do so, we move some helper functions from
DWARFLinker into the core lib debug info. This addresses the point 2) above.
This patch also enables addressing point 1) in the future, since the helper
function is now visible to other parts of LLVM.
[1]: https://github.com/llvm/llvm-project/issues/58362
Differential Revision: https://reviews.llvm.org/D155723
This gives us more meaningful information when
`getAbbreviationDeclarationSet` fails. Right now only
`verifyAbbrevSection` actually uses the error that it returns, but the
other call sites could be rewritten to take advantage of the returned error.
Differential Revision: https://reviews.llvm.org/D153459
This commit aims to improve error handling in the
DWARFAbbreviationDeclarationSet class. Specifically, we change the return type
of DWARFAbbreviationDeclarationSet::extract to an llvm::Error. In doing
so, we propagate the error from DWARFAbbreviationDeclaration::extract
another layer upward.
I have built on the previous unittest for DWARFDebugAbbrev that I
wrote a few days prior.
Namely, I am verifying that the following should give an error:
- An invalid tag following a non-null code
- An invalid attribute with a valid form
- A valid attribute with an invalid form
- An incorrectly terminated DWARFAbbreviationDeclaration
Additionally, I uncovered some invalid DWARF in an unrelated dsymutil
test. Namely the last Abbreviation Decl was missing a code.
This test has been updated accordingly. However, this commit does
not fix the underlying issue: llvm-dwarfdump does not correctly
verify the debug abbreviation section to catch these kinds of
mistakes. I have updated DWARFVerifier to not dereference a
pointer without first checking it and left a FIXME for future
contributors.
Differential Revision: https://reviews.llvm.org/D151353
Had a couple of issues lately causing corrupted strings due to
problematic str_offsets (overflow due to >4GB .debug_str.dwo section in
a dwp and the dwp tool silently overflowing the 32 bit offsets updated
in the .debug_str_offsets.dwo section, and then more recently two CUs in
a dwo caused the dwp tool to reapply the offset adjustment twice
corrupting str_offsets.dwo as well) - so let's check that the offsets
are valid.
This assumes no suffix merging - if anyone implements that, then this
checking should just be removed for the most part (we could still check
the offsets are within the bounds of .debug_str[.dwo], but nothing more
- any offset in the range would be valid, the offsets wouldn't have to
land at the start of a string)
DWARF 5 uses a 0-based index while previous versions use a 1-based
index. Fix the verifier and add a test.
Differential revision: https://reviews.llvm.org/D147202
DW_FORM_line_strp reads from the .debug_line_str section, but
previously the out-of-bounds error reported the .debug_line section.
This incorrect error message showed up when debugging an issue with
some invalid DWARF5 data.
The verify_string.s test has now been extended to check this (which
required a small change to the DWARF verifier to also look at
DW_FORM_line_strp).
Differential Revision: https://reviews.llvm.org/D146539
Changed contribution data structure to 64 bit. I added the 32bit and 64bit
accessors to make it explicit where we use 32bit and where we use 64bit. Also to
make sure sure we catch all the cases where this data structure is used.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D139379
Changed contribution data structure to 64 bit. I added the 32bit and 64bit
accessors to make it explicit where we use 32bit and where we use 64bit. Also to
make sure sure we catch all the cases where this data structure is used.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D139379
Use deduction guides instead of helper functions.
The only non-automatic changes have been:
1. ArrayRef(some_uint8_pointer, 0) needs to be changed into ArrayRef(some_uint8_pointer, (size_t)0) to avoid an ambiguous call with ArrayRef((uint8_t*), (uint8_t*))
2. CVSymbol sym(makeArrayRef(symStorage)); needed to be rewritten as CVSymbol sym{ArrayRef(symStorage)}; otherwise the compiler is confused and thinks we have a (bad) function prototype. There was a few similar situation across the codebase.
3. ADL doesn't seem to work the same for deduction-guides and functions, so at some point the llvm namespace must be explicitly stated.
4. The "reference mode" of makeArrayRef(ArrayRef<T> &) that acts as no-op is not supported (a constructor cannot achieve that).
Per reviewers' comment, some useless makeArrayRef have been removed in the process.
This is a follow-up to https://reviews.llvm.org/D140896 that introduced
the deduction guides.
Differential Revision: https://reviews.llvm.org/D140955
Summary:
Changed contribution data structure to 64 bit. I added the 32bit and 64bit
accessors to make it explicit where we use 32bit and where we use 64bit. Also to
make sure sure we catch all the cases where this data structure is used.
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
The DWARF spec says:
Any debugging information entry representing the declaration of an object,
module, subprogram or type may have DW_AT_decl_file, DW_AT_decl_line and
DW_AT_decl_column attributes, each of whose value is an unsigned integer
^^^^^^^^
constant.
If however, a producer happens to emit DW_AT_decl_file /
DW_AT_decl_line using a signed integer form, llvm-dwarfdump crashes,
like so:
(... snip ...)
0x000000b4: DW_TAG_structure_type
DW_AT_name ("test_struct")
DW_AT_byte_size (136)
DW_AT_decl_file (llvm-dwarfdump: (... snip ...)/llvm/include/llvm/ADT/Optional.h:197: T& llvm::optional_detail::OptionalStorage<T, true>::getValue() &
[with T = long unsigned int]: Assertion `hasVal' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0. Program arguments: /opt/rocm/llvm/bin/llvm-dwarfdump ./testsuite/outputs/gdb.rocm/lane-pc-vega20/lane-pc-vega20-kernel.so
#0 0x000055cc8e78315f PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
#1 0x000055cc8e780d3d SignalHandler(int) Signals.cpp:0:0
#2 0x00007f8f2cae8420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
#3 0x00007f8f2c58d00b raise /build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
#4 0x00007f8f2c56c859 abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:81:7
#5 0x00007f8f2c56c729 get_sysdep_segment_value /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:509:8
#6 0x00007f8f2c56c729 _nl_load_domain /build/glibc-SzIz7B/glibc-2.31/intl/loadmsgcat.c:970:34
#7 0x00007f8f2c57dfd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
#8 0x000055cc8e58ceb9 llvm::DWARFDie::dump(llvm::raw_ostream&, unsigned int, llvm::DIDumpOptions) const (/opt/rocm/llvm/bin/llvm-dwarfdump+0x2e0eb9)
#9 0x000055cc8e58bec3 llvm::DWARFDie::dump(llvm::raw_ostream&, unsigned int, llvm::DIDumpOptions) const (/opt/rocm/llvm/bin/llvm-dwarfdump+0x2dfec3)
#10 0x000055cc8e5b28a3 llvm::DWARFCompileUnit::dump(llvm::raw_ostream&, llvm::DIDumpOptions) (.part.21) DWARFCompileUnit.cpp:0:0
Likewise with DW_AT_call_file / DW_AT_call_line.
The problem is that the code in llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
dumping these attributes assumes that
FormValue.getAsUnsignedConstant() returns an armed optional. If in
debug mode, we get an assertion line the above. If in release mode,
and asserts are compiled out, then we proceed as if the optional had a
value, running into undefined behavior, printing whatever random
value.
Fix this by checking whether the optional returned by
FormValue.getAsUnsignedConstant() has a value, like done in other
places.
In addition, DWARFVerifier.cpp is validating DW_AT_call_file /
DW_AT_decl_file, but not AT_call_line / DW_AT_decl_line. This commit
fixes that too.
The llvm-dwarfdump/X86/verify_file_encoding.yaml testcase is extended
to cover these cases. Current llvm-dwarfdump crashes running the
newly-extended test.
"make check-llvm-tools-llvm-dwarfdump" shows no regressions, on x86-64
GNU/Linux.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D129392
Discovered in a large object that would need a 64 bit index (but the
cu/tu index format doesn't include a 64 bit offset/length mode in
DWARF64 - a spec bug) but instead binutils dwp overflowed the offsets
causing overlapping regions.
As usual with that header cleanup series, some implicit dependencies now need to
be explicit:
llvm/DebugInfo/DWARF/DWARFContext.h no longer includes:
- "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
- "llvm/DebugInfo/DWARF/DWARFCompileUnit.h"
- "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h"
- "llvm/DebugInfo/DWARF/DWARFDebugAranges.h"
- "llvm/DebugInfo/DWARF/DWARFDebugFrame.h"
- "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
- "llvm/DebugInfo/DWARF/DWARFDebugMacro.h"
- "llvm/DebugInfo/DWARF/DWARFGdbIndex.h"
- "llvm/DebugInfo/DWARF/DWARFSection.h"
- "llvm/DebugInfo/DWARF/DWARFTypeUnit.h"
- "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
Plus llvm/Support/Errc.h not included by a bunch of llvm/DebugInfo/DWARF/DWARF*.h files
Preprocessed lines to build llvm on my setup:
after: 1065629059
before: 1066621848
Which is a great diff!
Discourse thread: https://discourse.llvm.org/t/include-what-you-use-include-cleanup
Differential Revision: https://reviews.llvm.org/D119723
When reading location lists in dwo files the addresses cannot be
resolved, but that's not a problem.
Long term this probably should be fixed with a different API that
exposes location expressions without the need to resolve the address
ranges, since that's all the verifier (in its current state) requires.
(though the verifier should probably also eventually verify the address
ranges in location lists are a subset of the enclosing scope's address
range)
When verifying dwo files address ranges won't be able to be resolved due
to missing debug_addr (or missing debug_ranges in the case of DWARFv4
Split DWARF).
Since they refer to the debug_line in the skeleton unit, they can't be
resolved from the dwo CU.
But they can be resolved for split TUs, since those refer to
.debug_line.dwo, which is available in the dwo file.
Avoid duplicating the string decoding - improve the error messages down
in form parsing (& produce an Expected<const char*> instead of
Optional<const char*> to communicate the extra error details)
Introduced/discussed in https://reviews.llvm.org/D38719
The header validation logic was also explicitly building the DWARFUnits
to validate. But then other calls, like "Units.getUnitForOffset" creates
the DWARFUnits again in the DWARFContext proper - so, let's avoid
creating the DWARFUnits twice by walking the DWARFContext's units rather
than building a new list explicitly.
This does reduce some verifier power - it means that any unit with a
header parsing failure won't get further validation, whereas the
verifier-created units were getting some further validation despite
invalid headers. I don't think this is a great loss/seems "right" in
some ways to me that if the header's invalid we should stop there.
Exposing the raw DWARFUnitVectors from DWARFContext feels a bit
sub-optimal, but gave simple access to the getUnitForOffset to keep the
rest of the code fairly similar.
3d1d8c767be5537eb5510ee0522e2f3475fe7c04 made
DWARFExpression::iterator's Operation member `mutable`. After a few prep
commits, the iterator can instead be made a `const` iterator since no
caller can change the Operation.
Differential Revision: https://reviews.llvm.org/D113958
Clang will encode names that should be able to be simplified as
"_STNname|<template, args>" (eg: "_STNt1|<int>") - this verification
mode will detect these names, decode them, create the original name
("t1<int>") and the simple name ("t1") - letting the simple name run
through the usual rebuilding logic - then compare the two sources of the
full name - the rebuilt and the _STN encoding.
This helps ensure that -gsimple-template-names is lossless.
