This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy] Remove support for legacy .zdebug sections
ClosedPublic

Authored by MaskRay on Jun 27 2022, 4:29 PM.

Details

Summary

clang 14 removed -gz=zlib-gnu support and ld.lld removed linker input support
for zlib-gnu in D126793. Now let's remove zlib-gnu from llvm-objcopy.

  • .zdebug* sections are no longer recognized as debug sections. --strip* don't remove them. They are copied like other opaque sections
  • --decompress-debug-sections does not uncompress .zdebug* sections
  • --compress-debug-sections=zlib-gnu is not supported

It is very rare but in case a user has object files using .zdebug . They can use
llvm-objcopy<15 or GNU objcopy for uncompression.
--compress-debug-sections=zlib-gnu is unlikely ever used by anyone, so I do not
add a custom diagnostic.

Diff Detail

Event Timeline

MaskRay created this revision.Jun 27 2022, 4:29 PM
MaskRay requested review of this revision.Jun 27 2022, 4:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 27 2022, 4:29 PM

@plotfi, you were the one who originally added compression support, including zlib-gnu support. Do you know of a use-case for zlib-gnu support? If not, I think we can drop it, given that GNU has. (Looking back at https://reviews.llvm.org/D49678#1189341 which was a comment I made on the review to add support, @plotfi originally had this marked as deprecated).

llvm/lib/ObjCopy/ELF/ELFObject.cpp
522

I wonder if this should be a switch statement, so that we can benefit from compiler diagnostics if not all DebugCompressionType values are handled. Thoughts?

MaskRay added inline comments.Jun 28 2022, 1:27 AM
llvm/lib/ObjCopy/ELF/ELFObject.cpp
522

I placed assert(Sec.CompressionType == DebugCompressionType::Z); below. When zstd is added, the assert will fire :)

MaskRay marked an inline comment as done.Jun 28 2022, 1:27 AM
jhenderson added inline comments.Jun 28 2022, 2:33 AM
llvm/lib/ObjCopy/ELF/ELFObject.cpp
522

My thinking was that it would turn the runtime failure into a potential build-time one, which improves developer experience. We'd still want an llvm_unreachable or similar after the switch of course.

MaskRay updated this revision to Diff 440739.Jun 28 2022, 1:13 PM
MaskRay marked an inline comment as done.

Change ELFSectionWriter<ELFT>::visit to a switch to catch new DebugCompressionType values.

llvm/lib/ObjCopy/ELF/ELFObject.cpp
522

The build-time warning/error idea LG. I switched to a switch statement.

This revision is now accepted and ready to land.Jun 29 2022, 1:18 AM

@plotfi Do you have zlib-gnu usage?

@plotfi Do you have zlib-gnu usage?

Hi all thanks for reaching out. I will confirm with some folks who I wrote this zlib-gnu support for to confirm it is not needed.

@plotfi Do you have zlib-gnu usage?

Hi all thanks for reaching out. I will confirm with some folks who I wrote this zlib-gnu support for to confirm it is not needed.

We don't need zlib-gnu anymore.

@plotfi Do you have zlib-gnu usage?

Hi all thanks for reaching out. I will confirm with some folks who I wrote this zlib-gnu support for to confirm it is not needed.

We don't need zlib-gnu anymore.

Thanks!

@plotfi Do you have zlib-gnu usage?

Hi all thanks for reaching out. I will confirm with some folks who I wrote this zlib-gnu support for to confirm it is not needed.

We don't need zlib-gnu anymore.

Thanks for chiming in @smeenai. Thanks for reaching out @MaskRay @jhenderson

This revision was landed with ongoing or failed builds.Jun 29 2022, 10:43 AM
This revision was automatically updated to reflect the committed changes.

Hi, the new test, zdebug.yaml, fails on AIX with error: LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress.
https://lab.llvm.org/buildbot/#/builders/214/builds/2061/steps/6/logs/FAIL__LLVM__zdebug_yaml

I think the test needs # REQUIRES: zlib ?

Hi, the new test, zdebug.yaml, fails on AIX with error: LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress.
https://lab.llvm.org/buildbot/#/builders/214/builds/2061/steps/6/logs/FAIL__LLVM__zdebug_yaml

I think the test needs # REQUIRES: zlib ?

Yes! Done in 5530e5552128e9360dccf5e4813dd691d8b4c503

llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-invalid-format.test