Page MenuHomePhabricator

[ObjectYAML][DWARF] Support emitting .debug_ranges section in ELFYAML.
ClosedPublic

Authored by Higuoxing on Jun 4 2020, 8:18 PM.

Details

Summary

This patch enables yaml2elf to emit the .debug_ranges section.

Diff Detail

Event Timeline

Higuoxing created this revision.Jun 4 2020, 8:18 PM
Herald added a project: Restricted Project. · View Herald Transcript
Higuoxing updated this revision to Diff 268659.Jun 4 2020, 9:09 PM

Remove one blank line.

Thanks. A number of relatively minor test comments only from me.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-ranges.yaml
12–23

Nit: you need an extra space to make these line up with the CONTENT checks.

24

You can make this DWARF-LE-CONTENT-NEXT since it should follow the EntrySize (or you can add extra directives to make it do so). Same goes for the big endian version.

163

Nit: It doesn't matter much, but I'd make AddrSize: 0x8 from here on, since it is the natural address size of X86_64

165–166

I'd avoid using 0 for both high and low, since that is a special meaning for .debug_ranges. Perhaps 1 and 2 would be better. Also, perhaps worth an extra case showing that a) 0, 0 can be used explicitly, and that b) 0xffffffffffffffff, <something> can be used too, since those are both special kinds of entries in the table. You could put them in the original pair of tests if you want, or in a new case, whichever you prefer.

215–216

Nit: don't pad your numbers with leading zeros unless necessary (I don't think it is necessary here). Same goes throughout.

Higuoxing updated this revision to Diff 268725.Jun 5 2020, 2:55 AM
Higuoxing marked 5 inline comments as done.

Address comments:

  • Remove unneeded padding zeros.
  • Add two special entries (0xffffffff, Base Address), (0x00000000, 0x00000000) to test (a).
jhenderson accepted this revision.Jun 5 2020, 3:25 AM

LGTM, with one remaining suggestion.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-ranges.yaml
216–217

Up to you, but the "don't pad unnecessarily" comment also applies to these addresse and the AddrSize field in this and the other examples. I don't mind if you think it's useful to have them there though.

This revision is now accepted and ready to land.Jun 5 2020, 3:25 AM
This revision was automatically updated to reflect the committed changes.