Page MenuHomePhabricator

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

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



This patch adds support for dumping the .debug_ranges section to

Diff Detail

Event Timeline

Higuoxing created this revision.Sep 9 2020, 8:18 PM
Herald added a project: Restricted Project. · View Herald Transcript
Higuoxing requested review of this revision.Sep 9 2020, 8:18 PM
jhenderson added inline comments.Sep 10 2020, 12:29 AM
  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.


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).


Add a comment explaining the need for the debug_info bit.


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.


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


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.Sep 10 2020, 2:09 AM
Higuoxing marked 5 inline comments as done.

Address review comments.


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.Sep 10 2020, 2:15 AM

LGTM, with a couple of nits.


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


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


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.Sep 10 2020, 2:15 AM
Higuoxing updated this revision to Diff 291106.Sep 10 2020, 5:45 PM
Higuoxing marked 2 inline comments as done.

Address comments.

Thanks for reviewing!

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