This is an archive of the discontinued LLVM Phabricator instance.

[DWARF] Allow empty address range tables.
ClosedPublic

Authored by ikudrin on Dec 27 2019, 8:01 AM.

Details

Summary

Empty address range tables are allowed by the DWARF standard; moreover, generating them is recommended as a best practice.

Diff Detail

Event Timeline

ikudrin created this revision.Dec 27 2019, 8:01 AM
dblaikie added inline comments.Dec 27 2019, 10:55 AM
llvm/test/DebugInfo/X86/dwarfdump-debug-aranges.s
22

Might be better to hardcode the value here (since you're hardcoding the '7' anyway) & include a comment about what this is padding out to?

(& just for amusement I looked at the DWARF spec & I find no mention of this padding - though clearly GCC and LLVM emit & parse/expect such padding)

ikudrin updated this revision to Diff 237003.Jan 9 2020, 3:24 AM
ikudrin edited the summary of this revision. (Show Details)
  • Add a macro and an explanation comment for the padding wizardry in the test. Hope it is more clear now.
dblaikie accepted this revision.Jan 9 2020, 2:27 PM

Padding stuff seems like overkill to me, hardcoding the actual value seems like it'd probably be fine, but I don't mind so much.

This revision is now accepted and ready to land.Jan 9 2020, 2:27 PM
ikudrin updated this revision to Diff 237650.Jan 13 2020, 6:09 AM
  • Use hardcoded padding.

It looks like I was not very convincing about the macro for calculating the padding. Moreover, as I am adding gtest-based unit tests in other patches, and they anyway use hardcoded paddings, it makes less sense to have that intricate calculation anymore.

aprantl added inline comments.Jan 13 2020, 9:35 AM
llvm/lib/DebugInfo/DWARF/DWARFDebugArangeSet.cpp
93

FYI, the LLVM coding style prefers putting the comment on a line of its own before the statement.

ikudrin marked 2 inline comments as done.Jan 14 2020, 3:15 AM
ikudrin added inline comments.
llvm/lib/DebugInfo/DWARF/DWARFDebugArangeSet.cpp
93

I am unable to find such a recommendation in https://llvm.org/docs/CodingStandards.html. Could you point that out?

Anyway, these lines are temporary and are going to be removed in the next patch of the stack, D71875.

This revision was automatically updated to reflect the committed changes.