This is an archive of the discontinued LLVM Phabricator instance.

llvm-dwarfdump: implement --find=<name>
ClosedPublic

Authored by aprantl on Sep 26 2017, 10:10 AM.

Details

Summary

This patch implements the dwarfdump option --find=<name>.
This option looks for a DIE in the accelerator tables and dumps it if found.
This initial patch only adds support for .apple_names to keep the review small, adding the other sections and pubnames support should be trivial though.

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl created this revision.Sep 26 2017, 10:10 AM
aprantl updated this revision to Diff 116682.Sep 26 2017, 10:16 AM

s/NAME/name/

JDevlieghere added inline comments.Sep 26 2017, 2:11 PM
include/llvm/DebugInfo/DWARF/DWARFContext.h
242 ↗(On Diff #116682)

s/pointer/reference/

lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
198 ↗(On Diff #116682)

Why not Offset?

241 ↗(On Diff #116682)

Not clang-formatted?

tools/llvm-dwarfdump/llvm-dwarfdump.cpp
254 ↗(On Diff #116682)

Would it be worthwhile to stop iterating once found?

aprantl updated this revision to Diff 116837.Sep 27 2017, 10:01 AM
aprantl marked 3 inline comments as done.

Address review feedback form Jonas.

aprantl marked an inline comment as done.Sep 27 2017, 10:02 AM
probinson added inline comments.
include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
23 ↗(On Diff #116837)

Forward declaration is redundant with the above #include.

test/tools/llvm-dwarfdump/X86/find.test
11 ↗(On Diff #116837)

Maybe another CHECK-NOT to prove the name is associated with this subprogram? Might be overkill but we usually do something like that. Similarly for the MULTI cases below.

aprantl updated this revision to Diff 116897.Sep 27 2017, 3:54 PM

Address review feedback from Paul.

aprantl marked 2 inline comments as done.Sep 27 2017, 3:54 PM
JDevlieghere accepted this revision.Sep 28 2017, 10:33 AM

To the extent of my knowledge about accelerator tables, this LGTM.

This revision is now accepted and ready to land.Sep 28 2017, 10:33 AM
This revision was automatically updated to reflect the committed changes.
dblaikie added inline comments.Sep 28 2017, 11:27 AM
include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
69 ↗(On Diff #116897)

default?

71 ↗(On Diff #116897)

Should this be const /ref/ return? (or non-const value, though iterators that return by value are problematic)

& what about operator->?

Should this class use one of the iterator facade helpers from llvm/ADT/iterator.h?

76 ↗(On Diff #116897)

Prefer direct init over copy init:

X y = z;

rather than:

X y(z);

where possible.