[llvm-objcopy] --compress-debug-sections: remove tail padding for ELFCLASS32

[MaskRay]

Summary

For an ELFCLASS32 object, a compressed section due to --compress-debug-sections={zlib,zstd} has a
tail padding of 12 zero bytes. zlib happily allows this while zstd is rigid and
reports error: '{{.*}}': failed to decompress section '.debug_foo': Src size is incorrect.

Cole Kissane reported the problem to me.

[phosek]

LGTM

[jhenderson]

LGTM too!

[jhenderson]

llvm/lib/ObjCopy/ELF/ELFObject.cpp

538–539

Actually one question: this seems to be assuming that BE and LE Elf_Chdr_Impl are the same size. I assume that's intentional, and it's fine if it is, but perhaps it deserves a static_assert or similar to protect us in case something odd changes in the future?

[MaskRay]

llvm/lib/ObjCopy/ELF/ELFObject.cpp

538–539

Thanks for the quick review. ELF has ELFCLASS32 and ELFCLASS64 distinction and endianness does not affect the structures. LLVMObject defines Elf_Chdr_Impl to provide ergonomic access for cross-endianness situations but I feel static_assert is rather unnecessary here.

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

static Error replaceAndRemoveSections(const CommonConfig &Config,
   if (Error E = Obj.removeSections(ELFConfig.AllowBrokenLinks, RemovePred))
     return E;
 
   if (Config.CompressionType != DebugCompressionType::None) {
     if (Error Err = replaceDebugSections(
             Obj, isCompressable,
             [&Config, &Obj](const SectionBase *S) -> Expected<SectionBase *> {
               return &Obj.addSection<CompressedSection>(
-                  CompressedSection(*S, Config.CompressionType));
+                  CompressedSection(*S, Config.CompressionType, Obj.Is64Bits));
             }))
       return Err;
   } else if (Config.DecompressDebugSections) {
     if (Error Err = replaceDebugSections(
             Obj,
             [](const SectionBase &S) { return isa<CompressedSection>(&S); },
             [&Obj](const SectionBase *S) {
               const CompressedSection *CS = cast<CompressedSection>(S);

llvm/lib/ObjCopy/ELF/ELFObject.h

class CompressedSection : public SectionBase {
   uint32_t ChType = 0;
   DebugCompressionType CompressionType;
   uint64_t DecompressedSize;
   uint64_t DecompressedAlign;
   SmallVector<uint8_t, 128> CompressedData;
 
 public:
   CompressedSection(const SectionBase &Sec,
-                    DebugCompressionType CompressionType);
+    DebugCompressionType CompressionType, bool Is64Bits);
   CompressedSection(ArrayRef<uint8_t> CompressedData, uint32_t ChType,
                     uint64_t DecompressedSize, uint64_t DecompressedAlign);
 
   uint64_t getDecompressedSize() const { return DecompressedSize; }
   uint64_t getDecompressedAlign() const { return DecompressedAlign; }
   uint64_t getChType() const { return ChType; }
 
   Error accept(SectionVisitor &Visitor) const override;
public:
   // not present in any segment. This could be a problem during file layout,
   // because other segments may get assigned an offset where either of the
   // two should reside, which will effectively corrupt the resulting binary.
   // Other than that we use these segments to track program header offsets
   // when they may not follow the ELF header.
   Segment ElfHdrSegment;
   Segment ProgramHdrSegment;
 
+  bool Is64Bits;
   uint8_t OSABI;
   uint8_t ABIVersion;
   uint64_t Entry;
   uint64_t SHOff;
   uint32_t Type;
   uint32_t Machine;
   uint32_t Version;
   uint32_t Flags;

llvm/lib/ObjCopy/ELF/ELFObject.cpp

Error ELFSectionWriter<ELFT>::visit(const CompressedSection &Sec) {
   memcpy(Buf, &Chdr, sizeof(Chdr));
   Buf += sizeof(Chdr);
 
   std::copy(Sec.CompressedData.begin(), Sec.CompressedData.end(), Buf);
   return Error::success();
 }
 
 CompressedSection::CompressedSection(const SectionBase &Sec,
-                                     DebugCompressionType CompressionType)
+                                     DebugCompressionType CompressionType,
+                                     bool Is64Bits)
     : SectionBase(Sec), CompressionType(CompressionType),
       DecompressedSize(Sec.OriginalData.size()), DecompressedAlign(Sec.Align) {
   compression::compress(compression::Params(CompressionType), OriginalData,
                         CompressedData);
 
   Flags |= ELF::SHF_COMPRESSED;
-  size_t ChdrSize =
-      std::max(std::max(sizeof(object::Elf_Chdr_Impl<object::ELF64LE>),
-                        sizeof(object::Elf_Chdr_Impl<object::ELF64BE>)),
-               std::max(sizeof(object::Elf_Chdr_Impl<object::ELF32LE>),
-                        sizeof(object::Elf_Chdr_Impl<object::ELF32BE>)));
+  size_t ChdrSize = Is64Bits ? sizeof(object::Elf_Chdr_Impl<object::ELF64LE>)
+                             : sizeof(object::Elf_Chdr_Impl<object::ELF32LE>);
   Size = ChdrSize + CompressedData.size();
   Align = 8;
 }
 
 CompressedSection::CompressedSection(ArrayRef<uint8_t> CompressedData,
                                      uint32_t ChType, uint64_t DecompressedSize,
                                      uint64_t DecompressedAlign)
     : ChType(ChType), CompressionType(DebugCompressionType::None),
template <class ELFT> Error ELFBuilder<ELFT>::build(bool EnsureSymtab) {
   // output file. Normally the same as ElfFile, but if we're extracting a
   // loadable partition it will point to the partition's headers.
   Expected<ELFFile<ELFT>> HeadersFile = ELFFile<ELFT>::create(toStringRef(
       {ElfFile.base() + EhdrOffset, ElfFile.getBufSize() - EhdrOffset}));
   if (!HeadersFile)
     return HeadersFile.takeError();
 
   const typename ELFFile<ELFT>::Elf_Ehdr &Ehdr = HeadersFile->getHeader();
+  Obj.Is64Bits = Ehdr.e_ident[EI_CLASS] == ELFCLASS64;
   Obj.OSABI = Ehdr.e_ident[EI_OSABI];
   Obj.ABIVersion = Ehdr.e_ident[EI_ABIVERSION];
   Obj.Type = Ehdr.e_type;
   Obj.Machine = Ehdr.e_machine;
   Obj.Version = Ehdr.e_version;
   Obj.Entry = Ehdr.e_entry;
   Obj.Flags = Ehdr.e_flags;
 

llvm/test/tools/llvm-objcopy/ELF/Inputs/compress-debug-sections.yaml

 --- !ELF
 FileHeader:
-  Class:           ELFCLASS64
+  Class:           ELFCLASS[[BITS=64]]
   Data:            ELFDATA2LSB
   Type:            ET_REL
   Machine:         EM_X86_64
 Sections:
   - Name:            .debug_foo
     Type:            SHT_PROGBITS
     Content:         0000000000000000
   - Name:            .notdebug_foo

llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zstd.test

 # COMPRESSED-NEXT:   0x00000010 00000000 00000000 {{.*}}
 
 ## --compress-debug-sections does not update a compressed section. Its compression
 ## type does not change.
 # RUN: llvm-objcopy --compress-debug-sections=zstd %t-zstd %t-zstd-zstd
 # RUN: cmp %t-zstd %t-zstd-zstd
 # RUN: %if zlib %{ llvm-objcopy --compress-debug-sections=zlib %t-zstd %t-zstd-zlib && cmp %t-zstd %t-zstd-zlib %}
 
+## Test roundtrip for ELFCLASS32.
+# RUN: yaml2obj %p/Inputs/compress-debug-sections.yaml -o %t32 && llvm-objcopy %t32
+# RUN: llvm-objcopy --compress-debug-sections=zstd %t32 %t32-zstd
+# RUN: llvm-objcopy --decompress-debug-sections %t32-zstd %t32-de
+# RUN: cmp %t32 %t32-de
+
 # RUN: yaml2obj %s -o %t-corrupted
 # RUN: not llvm-objcopy --decompress-debug-sections %t-corrupted /dev/null 2>&1 | FileCheck %s -DFILE=%t-corrupted --check-prefix=ERR
 
 # ERR: error: '[[FILE]]': failed to decompress section '.debug_info': Src size is incorrect
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64