New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 46805 - Regression in handling 0-length address range entries
Summary: Regression in handling 0-length address range entries
Status: RESOLVED FIXED
Alias: None
Product: tools
Classification: Unclassified
Component: llvm-dwarfdump (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-22 00:44 PDT by James Henderson
Modified: 2020-10-02 10:34 PDT (History)
6 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description James Henderson 2020-07-22 00:44:41 PDT
This regression was introduced in https://reviews.llvm.org/rGed9851a0a682d1ff288ed749287fbc7682ed1514 (see https://reviews.llvm.org/D71932 for the review).

A .debug_aranges table might contain an entry for a zero-length function or data. This is not forbidden by the standard, and can happen in practice. In such a case, it should not be an error. In up to at least LLVM version 9, this worked fine, but the cited change caused an error to start occurring. Example:

PS C:\Work\TempWork> llvm-dwarfdump.exe --debug-aranges bar.elf
bar.elf:        file format elf64-x86-64

.debug_aranges contents:
error: address range table at offset 0x0 has an invalid tuple (length = 0) at offset 0x10

A trivial asm input might look like:
foo:

.secton .debug_aranges,"",@progbits
... # Some data for the header
.quad foo
.quad 0 # 0 length
.quad 0 # The .debug_aranges terminator
.quad 0

Note that simply removing the error message is not quite the right fix - we still want to emit an error if a terminator is detected before the claimed end of the file, but beware a similar issue to bug 46804, where in an ET_REL file, the address might appear to be zero, but it is relocated.
Comment 1 David Blaikie 2020-07-22 12:34:35 PDT
just a few extra notes:

zero length entries in object files might be better avoided by the producer... maybe? But I don't think we should diagnose these as errors still.

but zero-length ranges in linked objects should certainly be expected - ld.bfd's tombstoning strategy is to resolve all addresses in debug_ranges referring to dead code to 1, making the start/end the same.

So, yeah, we shouldn't verify-fail on common constructs (zero length ranges in linked executables), and even the slightly less common/more quirky case (zero length ranges in object files) still seem reasonable enough to accept (& LLVM does produce these today for "int x() { }" or "void y() { __builtin_unreachable(); }")
Comment 2 James Henderson 2020-07-23 01:13:05 PDT
Igor Kudrin has pointed out in the original review that the spec does explicitly say "non-zero length" for .debug_aranges entries. I wonder if this is a case of overspecification. Of course, ideally the producer would never produce such entries, but it seems like it is unnecessary to impose that restriction on them, even in the presence of 0 as a valid address:

.debug_aranges has two ways to terminate a table: 1) the length field specifies the length of the table, so once that point is reached, it could be said that the table has ended; 2) the spec says that a (0, 0) pair (or (0, 0, 0) tuple) represents a terminator of the table. The latter is redundant. If it were to be removed from the spec in a future revision, the length could be solely used, and the bit about not allowing zero-length entries could be dropped, thus resolving any confusion.

@Paul/Adrian, what are your thoughts on this?

Also, what do people think about a fix here? Technically, according to the v4 and v5 specs it is indeed an error for a zero-length entry to exist, but in practice, there's no particular issue with that. There are real cases where these might have been emitted in the past (at least in our downstream toolchain up until very recently).
Comment 3 Paul Robinson 2020-07-23 13:30:03 PDT
Well, this is interesting.  Both sections 6.1.2 and 7.21 describe the
header and content for a .debug_aranges contribution, and they're not
quite identical.
- Only the 6.1.2 description says "non-zero length"
- Only the 7.21 description specifies alignment of the tuple array

Regarding the argument that having both a unit-length and terminator value
is redundant:
That's not entirely true.  The overall length of the header and tuple array 
is a multiple of the tuple size (segment_selector_size + 2*address_size).
If the segment selector size is non-zero, but you want the overall length
of the .debug_aranges contribution to be nicely aligned, then you'll need
padding at the end of the tuple array.  This suggests that a terminating
tuple value would be helpful, because that way the unit-length is not
required to be an exact multiple of the tuple size.

However, on the principle of "be generous in what you accept" as well as
wanting llvm-dwarfdump to be a useful diagnostic tool, I don't think it
should error out on a tuple with zero length and non-zero address.  I'd 
want it to keep dumping until it reached the end of the section.

Verify mode might want to complain about a zero length with a non-zero
address, though.
Comment 4 James Henderson 2020-08-05 07:45:18 PDT
Review: https://reviews.llvm.org/D85313
Comment 5 James Henderson 2020-08-10 08:04:15 PDT
Fixed in https://reviews.llvm.org/rGcb3a598c87db2db997401b82dfb3f7f80707194e. This removes the error, except in the case of a (0, 0) entry before the expected table end, in which case it is downgraded to a warning, and the list continues to be parsed until the expected end.