Page MenuHomePhabricator

[llvm-readobj] - Stop using `unwrapOrError` in `DumpStyle<ELFT>::getGroups()`
ClosedPublic

Authored by grimar on Nov 20 2020, 6:10 AM.

Details

Summary

With this we are able to diagnose possible issues much better and
don't exit on an error.

Diff Detail

Unit TestsFailed

TimeTest
400 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp

Event Timeline

grimar created this revision.Nov 20 2020, 6:10 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar requested review of this revision.Nov 20 2020, 6:10 AM
jhenderson added inline comments.Nov 23 2020, 12:21 AM
llvm/test/tools/llvm-readobj/ELF/groups.test
135

The string table isn't directly linked to a group section, so I found the origianl message slightly confusing.

186
282
llvm/tools/llvm-readobj/ELFDumper.cpp
3572

I don't like this function name. It implies it is solely for use by errors, which isn't the case. How about getSectionNameOrFallback?

grimar updated this revision to Diff 306983.Nov 23 2020, 1:27 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
3572

Done. 2 more possible names probably are: getSectionNameOrWarn or tryGetSectionName.

jhenderson accepted this revision.Nov 23 2020, 1:31 AM

LGTM.

llvm/test/tools/llvm-readobj/ELF/groups.test
186

Ping this?

llvm/tools/llvm-readobj/ELFDumper.cpp
3572

tryGetSectionName would work for me too. I'm happy with whichever you prefer. I'd probably not use getSectionNameOrWarn.

This revision is now accepted and ready to land.Nov 23 2020, 1:31 AM
grimar marked 2 inline comments as done.Nov 23 2020, 1:32 AM
grimar added inline comments.
llvm/test/tools/llvm-readobj/ELF/groups.test
186

Will fix. Sorry!

This revision was automatically updated to reflect the committed changes.
grimar marked 2 inline comments as done.