This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy] - Remove dead code. NFCI.
ClosedPublic

Authored by grimar on Mar 6 2019, 2:46 AM.

Details

Summary

DecompressedSection can only be created if --decompress-debug-sections is specified.
https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/ELF/ELFObjcopy.cpp#L492

If it is specified when !zlib::isAvailable(), we error out early when parsing the options:
https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/CopyConfig.cpp#L657

What means the code I am removing in this patch is dead.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Mar 6 2019, 2:46 AM
This revision is now accepted and ready to land.Mar 6 2019, 4:30 AM
jhenderson added inline comments.Mar 6 2019, 4:31 AM
tools/llvm-objcopy/ELF/Object.cpp
184 ↗(On Diff #189466)

I wonder if it's worth making zlib::isAvailable() an assertion in this function?

grimar marked an inline comment as done.Mar 6 2019, 4:42 AM
grimar added inline comments.
tools/llvm-objcopy/ELF/Object.cpp
184 ↗(On Diff #189466)

I think it might be better to have an assert in DecompressedSection's constructor then.
This section is not expected to be created in case of !zlib::isAvailable() first of all.

jhenderson added inline comments.Mar 6 2019, 4:49 AM
tools/llvm-objcopy/ELF/Object.cpp
184 ↗(On Diff #189466)

Actually, it's moot, since there is an llvm_unreachable assertion in zlib::uncompress is called when it isn't available, so no need for the assertion at all, I think, although it doesn't hurt to have one in the constructor if you want to add it.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2019, 6:11 AM