Page MenuHomePhabricator

[obj2yaml] Add support for dumping the .debug_ranges section.
ClosedPublic

Authored by Higuoxing on Wed, Sep 9, 8:18 PM.

Details

Summary

This patch adds support for dumping the .debug_ranges section to
elf2yaml.

Diff Detail

Event Timeline

Higuoxing created this revision.Wed, Sep 9, 8:18 PM
Herald added a project: Restricted Project. · View Herald Transcript
Higuoxing requested review of this revision.Wed, Sep 9, 8:18 PM
jhenderson added inline comments.Thu, Sep 10, 12:29 AM
llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml
8
  1. See below re. numeric versus string variables.
  2. Reflow the FileCheck command - it's way too long for one line.
  3. I recommend reordering the commands this way, since the first bits don't change at all.
  4. There was a sneaky double space before %s.

Same applies throughout.

47

As you're not doing any arithmetic here, I'd just match the number as a string. Same below with the low/high offsets. This simplifies both the check here and the definition above (i.e. -DOFFSET=0x18 or whatever).

59–63

Add a comment explaining the need for the debug_info bit.

86

This and the following lines probably want to be RAW-SAME, and start with a {{$}} to ensure there's nothing in between the previous match and the checked-for one.

109

Probably should show that there's no Sections: tag here.

123

Same below. Whilst the 80-column width style guide doesn't really apply in tests, it's good to avoid the lines getting too long, in my opinion, as it is easier to read.

Also RANGES and SHDR are always both checked, so combine them to a single prefix (e.g. COMMON).

Higuoxing updated this revision to Diff 290909.Thu, Sep 10, 2:09 AM
Higuoxing marked 5 inline comments as done.

Address review comments.

llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml
86

It looks that FileCheck is not happy when the check line starts with a {{$}}. I think using {{^}} to check that the rest buffer exactly starts with our check string is fine here. What do you think?

jhenderson accepted this revision.Thu, Sep 10, 2:15 AM

LGTM, with a couple of nits.

llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml
86

Sorry, {{^}} was what I meant (I got mixed up between start and end of line regex).

98

No need for the "should". Hard to explain from a grammatical perspective though!

151

This should be less brittle in case any user paths start appearing in the output at some point.

This revision is now accepted and ready to land.Thu, Sep 10, 2:15 AM
Higuoxing updated this revision to Diff 291106.Thu, Sep 10, 5:45 PM
Higuoxing marked 2 inline comments as done.

Address comments.

Thanks for reviewing!

This revision was landed with ongoing or failed builds.Thu, Sep 10, 5:48 PM
This revision was automatically updated to reflect the committed changes.