This is an archive of the discontinued LLVM Phabricator instance.

llvm-dwarfdump: Replace -debug-dump=sect option with individual options.
ClosedPublic

Authored by aprantl on Sep 11 2017, 2:56 PM.

Details

Summary

As discussed on llvm-dev in http://lists.llvm.org/pipermail/llvm-dev/2017-September/117301.html this changes the command line interface of llvm-dwarfdump to match the one used by the dwarfdump utility shipping on macOS. In addition to being shorter to type this format also has the advantage of allowing more than one section to be specified at the same time.

With this change

$ llvm-dwarfdump --debug-dump=info
$ llvm-dwarfdump --debug-dump=apple-objc

becomes

$ dwarfdump --debug-info --apple-objc
```.

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl created this revision.Sep 11 2017, 2:56 PM
davide added a subscriber: davide.Sep 11 2017, 3:00 PM

The changes to llvm-dwarfdump/llvm-objdump look good to me.
I didn't review all the tests, but that seems like something you did with grep, so I wouldn't be too worried if it passes the suite.

dblaikie accepted this revision.Sep 11 2017, 3:04 PM

Decided it wasn't worth keeping the old syntax? I think that's probably fine (looks like Nico added it inspired by the readelf syntax (which supports exactly that, --debug-dump=foo) - but since it's in a different binary anyway, I doubt anyone's using it for compatibility in scripts, etc).

Worth adding a test for the new "can dump more than one section" functionality?

include/llvm/DebugInfo/DIContext.h
115–124 ↗(On Diff #114713)

Anonymous namespaces in headers are generally a bit suspect - tend to create ODR violations.

This revision is now accepted and ready to land.Sep 11 2017, 3:04 PM
JDevlieghere accepted this revision.Sep 11 2017, 3:15 PM

LGTM!

include/llvm/DebugInfo/DIContext.h
117 ↗(On Diff #114713)

Is this already being used somewhere?

This revision was automatically updated to reflect the committed changes.
probinson edited edge metadata.Sep 11 2017, 4:07 PM

I would kind of like one option to dump whichever of .debug_info or .debug_info.dwo (etc) there is, so I don't have to know up front which section to be asking for. Would also be more user-friendly to people who kind of know DWARF but not at that level of detail.

I would kind of like one option to dump whichever of .debug_info or .debug_info.dwo (etc) there is, so I don't have to know up front which section to be asking for. Would also be more user-friendly to people who kind of know DWARF but not at that level of detail.

Since the DWO and non-DWO variants rarely (never) appear in the same file, is there even a point in having a separate options?
What about supporting both options, but --debug-info also implies --debug-info-dwo?

  • adrian

What about supporting both options, but --debug-info also implies --debug-info-dwo?

WFM.

rnk edited edge metadata.Sep 11 2017, 4:29 PM

+1 for dropping the old names

include/llvm/DebugInfo/DIContext.h
115–124 ↗(On Diff #114713)

+1

Also, think it's worth adding DIDT_ID_Count and a static_assert(DIDT_ID_Count < 64)?

122 ↗(On Diff #114713)

This is redundant with the .def file.

131 ↗(On Diff #114713)

Is 1ULL << necessary to avoid UB here?