This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj/elf] - Add a testing for --stackmap and refine the implementation.
ClosedPublic

Authored by grimar on Aug 4 2020, 6:22 AM.

Details

Summary

Currently, we only test the --stackmap option here:
https://github.com/llvm/llvm-project/blob/master/llvm/test/Object/stackmap-dump.test
it uses a precompiled MachO binary currently and I've found no tests for this option for ELF.

The implementation also has issues. For example, it might assert on a wrong version
of the .llvm-stackmaps section. Or it might crash on an empty or truncated section.

This patch introduces a new tools/llvm-readobj/ELF test file as well as implements a few
basic checks to catch simple crashes/issues

It also eliminates unwrapOrError calls in printStackMap().

Diff Detail

Event Timeline

grimar created this revision.Aug 4 2020, 6:22 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar requested review of this revision.Aug 4 2020, 6:22 AM
MaskRay accepted this revision.Aug 4 2020, 2:18 PM

Looks great!

llvm/tools/llvm-readobj/COFFDumper.cpp
63

This seems an unrelated change.

This revision is now accepted and ready to land.Aug 4 2020, 2:18 PM
jhenderson added inline comments.Aug 5 2020, 12:52 AM
llvm/include/llvm/Object/StackMapParser.h
322
328

minimal -> minimum

llvm/test/tools/llvm-readobj/ELF/stackmap.test
31
48

You probably want a line break in this comment now too.

llvm/tools/llvm-readobj/ELFDumper.cpp
3430–3447

Is this clang-formatted correctly? That && looks in the wrong place to me.

3438–3441

Is there a test for this case failing?

grimar updated this revision to Diff 283176.Aug 5 2020, 2:53 AM
grimar marked 7 inline comments as done.
  • Addressed review comments.
llvm/tools/llvm-readobj/COFFDumper.cpp
63

It is related: I had to include the ELF.h to StackMapParser.h (to use object::createError) and the former contains the equal createError implementation. So I had to remove this one to fix compilation error.

aside: noticable, that under MSVS it compiles fine without early inclusion of the ELF.h to StackMapParser.h . But it is just because MSVS has relaxed rules about the order of static methods I believe. E.g. you can define a static helper at line 100 and use if from the line 90.

llvm/tools/llvm-readobj/ELFDumper.cpp
3430–3447

Fixed.

3438–3441

Oh, it was not tested. Thanks!