[MC] Support writing ELFCOMPRESS_ZSTD compressed debug info sections

[MaskRay]

Summary

and add --compress-debug-sections=zstd to llvm-mc for testing.

[MaskRay]

@ckissane This also serves as an example splitting a big change like D130667 and adding accompanying tests:)

(Note: I've gradually cleaned up code and removed zlib-gnu stuff over the previous months to make the code more ready for zstd.)

[dblaikie]

llvm/lib/MC/ELFObjectWriter.cpp

861–879

Proliferation of these sort of switch statements is why I'd like to resolve the design discussions before further patches are committed. They're not hugely expensive to fix/change later, but neither are we in a rush to add this functionality, nor is the design discussion especially hard to resolve.
Code something like:

if (CompressionAlgorithm *C = getCompressionAlgorithm(MAI->compressDebugSections()) {
  ChType = C->getELFType(); // alternatively have an array to lookup ELF type from the enum type
  C->compress(Uncompressed, Compressed);
}

Seems like it'd be quite achievable without great complexity & simplify consumers.

Possibly even having 'compressDebugSections' carry the algorithm pointer directly? (since the checking's already done up-front, and the pointer would be to an immutable/thread safe/singleton/eternal object anyway)

llvm/tools/llvm-mc/llvm-mc.cpp

404–424

This could potentially be replaced by something like:

CompressionAlgorithm *C = getCompressionAlgorithm(CompressDebugSections);
if (C && !C->isAvailable()) {
  WithColor::error(errs(), ProgName) << "build LLVM with " << C->getBuildFlag() << " to enable " << CompressDebugSections.is_there_some_way_to_query_the_opts_full_spelling();
  C = nullptr;
}
MAI->setCompressDebugSections(C);

I guess the desire to get the build flag name might be enough to justify getCompressionAlgorithm returning non-null even for non-available algorithms, so it can provide that info? (unlike my previous comment/example above that implies getCompressionAlgorithm returns null for non-available algorithms)

[MaskRay]

llvm/lib/MC/ELFObjectWriter.cpp

861–879

I wish that 15.0.0 would have the llvm-objcopy change. This helps distributions which install llvm binary utilities in /usr/bin which may use newer userland llvm utilities.
MC/clang -gz support certainly can go into 16.0.0.

After this MC change, the only remaining place using DebugCompressionType is clang -gz. We don't have many ELF object producers, and the object-oriented CompressionAlgorithm design kinda makes me feel some over-engineering.

Note: in many (among the few DebugCompressionType uses) use cases isAvailable and compress are called apart.

[dblaikie]

llvm/lib/MC/ELFObjectWriter.cpp

861–879

I don't think there's any need to rush to get this into any particular LLVM release (compression isn't enabled by default in any case & I don't think this is a pressing issue, is it?).

I don't think the complexity needed to implement the sort of framing above should be too great (the current patches add a bunch of other things that I think are overly complicating it & making the comparison weight differently - which is why I'm trying to start with the syntax and work backwards to the implementation necessary to provide that interface and not more than that).

It'd be nice if they didn't have to be so separate (between isAvailable and compress) - so the invariants were expressed at the API level rather than by assertions that something was checked somewhere else.

[MaskRay]

llvm/lib/MC/ELFObjectWriter.cpp

861–879

Say, someone maintains XXXBSD and has /usr/bin llvm binary utilities (base image) from an older release. If I get a binary built from a newer machine with newer clang, I'd hope that I have a way to decompress the debug sections with /usr/bin/llvm-objcopy . Having zstd decompression support at 15.0.0 will help such a use case. Whether LLVM_ENABLE_ZSTD is enabled or not by default in the upstream doesn't really matter. A distribution typically enables many LLVM/CLANG/etc CMake variables to build a toolchain customized to their needs.

As mentioned on another thread, the proposed object-oriented design seems to have too much boilerplate. Free functions seem to work quite well. (This is just the classical expression problem and I think different people may have different opinions.)

[dblaikie]

llvm/lib/MC/ELFObjectWriter.cpp

861–879

Oh, sorry, by not enabled by default, I was thinking about clang -gz not being the default. So I guess there's 3 (maybe 4) levels of "this is not the default" - LLVM_ENABLE_ZSTD isn't enabled by default, -gz isn't clang's default, and -gz=zstd isn't the default even when compression is requested (& the 4th - -g isn't the default either, but probably has enough users that we don't count that case).

If someone gets a new LLVM-based tool that isn't a full LLVM release, it won't compress by default and it won't use zstd by default - I think it's OK if their system-installed tools don't handle that novel case (similarly we implemented -gdwarf-5 long before system debuggers could handle it, we even changed to `-gdwarf-5 by default before system gdb's could handle it, I think).

I don't think there's any need to rush this.

[MaskRay]

llvm/lib/MC/ELFObjectWriter.cpp

861–879

-gz: There are Linux distributions defaulting to -gz (probably by patching gcc driver).

-g: Many Linux distributions build packages with -g and ship the non-debug part and debug symbols in separate packages. -g is likely never a default but this argument does not affect distribution usage.

[MaskRay]

rebase

[MaskRay]

rebase on D130458 (llvm-objcopy)

[MaskRay]

llvm/lib/MC/ELFObjectWriter.cpp

861–879

Resolving all comments since this does not catch 15.0.0 and the latest API design discussion happened in other places (D130506 and alternative patches).

I think the switch is useful as it makes clear the code intentionally supports the related compression formats.

[dblaikie]

Sounds good

[DavidSpickett]

FYI one of the tests is failing on at least this bot:
https://lab.llvm.org/buildbot/#/builders/42/builds/7040

[dyung]

In D130724#3776791, @DavidSpickett wrote:

FYI one of the tests is failing on at least this bot:
https://lab.llvm.org/buildbot/#/builders/42/builds/7040

The PS5 Windows bot is also failing for the same reason:
https://lab.llvm.org/buildbot/#/builders/216/builds/9486

The error message on Windows seems to be different from what is expected.

[thakis]

This breaks tests on Windows when building without zlib: http://45.33.8.238/win/65893/step_11.txt

Please take a look and revert for now if it takes a while to fix.

llvm/lib/MC/ELFObjectWriter.cpp

void ELFWriter::writeSectionData(const MCAssembler &Asm, MCSection &Sec,
 
   bool CompressionEnabled =
       MAI->compressDebugSections() != DebugCompressionType::None;
   if (!CompressionEnabled || !SectionName.startswith(".debug_")) {
     Asm.writeSectionData(W.OS, &Section, Layout);
     return;
   }
 
-  assert(MAI->compressDebugSections() == DebugCompressionType::Z &&
-         "expected zlib style compression");
-
   SmallVector<char, 128> UncompressedData;
   raw_svector_ostream VecOS(UncompressedData);
   Asm.writeSectionData(VecOS, &Section, Layout);
+  ArrayRef<uint8_t> Uncompressed =
+      makeArrayRef(reinterpret_cast<uint8_t *>(UncompressedData.data()),
+                   UncompressedData.size());
 
   SmallVector<uint8_t, 128> Compressed;
-  const uint32_t ChType = ELF::ELFCOMPRESS_ZLIB;
-  compression::zlib::compress(
-      makeArrayRef(reinterpret_cast<uint8_t *>(UncompressedData.data()),
-                   UncompressedData.size()),
-      Compressed);
+  uint32_t ChType;
+  switch (MAI->compressDebugSections()) {
+  case DebugCompressionType::None:
+    llvm_unreachable("has been handled");
+  case DebugCompressionType::Z:
+    ChType = ELF::ELFCOMPRESS_ZLIB;
+    compression::zlib::compress(Uncompressed, Compressed);
+    break;
+  case DebugCompressionType::Zstd:
+    ChType = ELF::ELFCOMPRESS_ZSTD;
+    compression::zstd::compress(Uncompressed, Compressed);
+    break;
+  }
 
   if (!maybeWriteCompression(ChType, UncompressedData.size(), Compressed,
                              Sec.getAlignment())) {
     W.OS << UncompressedData;
     return;
   }
 
   Section.setFlags(Section.getFlags() | ELF::SHF_COMPRESSED);
   // Alignment field should reflect the requirements of

llvm/test/MC/ELF/compress-debug-sections-zlib-unavailable.s

 // REQUIRES: !zlib
 // RUN: not llvm-mc -filetype=obj -compress-debug-sections=zlib -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
 
-// CHECK: llvm-mc{{[^:]*}}: error: build tools with zlib to enable -compress-debug-sections
+// CHECK:     llvm-mc{{[^:]*}}: error: build tools with LLVM_ENABLE_ZLIB to enable --compress-debug-sections=zlib
+// CHECK-NOT: {{.}}

llvm/test/MC/ELF/compress-debug-sections-zstd-unavailable.s

+# UNSUPPORTED: zstd
+# RUN: not llvm-mc -filetype=obj -compress-debug-sections=zstd -triple x86_64 %s 2>&1 | FileCheck %s
+
+# CHECK:     error: build LLVM with LLVM_ENABLE_ZSTD to enable --compress-debug-sections=zstd
+# CHECK-NOT: {{.}}

llvm/test/MC/ELF/compress-debug-sections-zstd.s

+# REQUIRES: zstd
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 -compress-debug-sections=zstd %s -o %t
+# RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=SEC
+# RUN: llvm-objdump -s %t | FileCheck %s
+
+## Check that the large debug sections .debug_line and .debug_frame are compressed
+## and have the SHF_COMPRESSED flag.
+# SEC: .nonalloc     PROGBITS [[#%x,]] [[#%x,]] [[#%x,]] 00     0 0  1
+# SEC: .debug_line   PROGBITS [[#%x,]] [[#%x,]] [[#%x,]] 00   C 0 0  8
+# SEC: .debug_abbrev PROGBITS [[#%x,]] [[#%x,]] [[#%x,]] 00     0 0  1
+# SEC: .debug_info   PROGBITS [[#%x,]] [[#%x,]] [[#%x,]] 00     0 0  1
+# SEC: .debug_str    PROGBITS [[#%x,]] [[#%x,]] [[#%x,]] 01 MSC 0 0  8
+# SEC: .debug_frame  PROGBITS [[#%x,]] [[#%x,]] [[#%x,]] 00   C 0 0  8
+
+# CHECK:      Contents of section .debug_line
+## ch_type == ELFCOMPRESS_ZSTD (2)
+# CHECK-NEXT: 0000 02000000 00000000 55010000 00000000
+# CHECK-NEXT: 0010 01000000 00000000 {{.*}}
+
+## The compress/decompress round trip should be identical to the uncompressed output.
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.uncom
+# RUN: llvm-objcopy --decompress-debug-sections %t %t.decom
+# RUN: cmp %t.uncom %t.decom
+
+## Don't compress a section not named .debug_*.
+        .section .nonalloc,"",@progbits
+.rept 50
+.asciz "aaaaaaaaaa"
+.endr
+
+  .section  .debug_line,"",@progbits
+
+  .section  .debug_abbrev,"",@progbits
+.Lsection_abbrev:
+  .byte 1                       # Abbreviation Code
+  .byte 17                      # DW_TAG_compile_unit
+  .byte 0                       # DW_CHILDREN_no
+  .byte 27                      # DW_AT_comp_dir
+  .byte 14                      # DW_FORM_strp
+  .byte 0                       # EOM(1)
+  .byte 0                       # EOM(2)
+
+  .section  .debug_info,"",@progbits
+  .long 12                      # Length of Unit
+  .short  4                       # DWARF version number
+  .long .Lsection_abbrev        # Offset Into Abbrev. Section
+  .byte 8                       # Address Size (in bytes)
+  .byte 1                       # Abbrev [1] DW_TAG_compile_unit
+  .long .Linfo_string0          # DW_AT_comp_dir
+
+  .text
+foo:
+  .cfi_startproc
+  .file 1 "Driver.ii"
+
+.rept 3
+.ifdef I386
+  pushl %ebp
+  .cfi_def_cfa_offset 8
+  .cfi_offset %ebp, -8
+  movl  %esp, %ebp
+  .cfi_def_cfa_register %ebp
+  pushl %ebx
+  pushl %edi
+  pushl %esi
+  .cfi_offset %esi, -20
+  .cfi_offset %edi, -16
+  .cfi_offset %ebx, -12
+  .loc  1 1 1 prologue_end
+  popl  %esi
+  popl  %edi
+  popl  %ebx
+  popl  %ebp
+  .cfi_def_cfa %esp, 4
+.else
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+  pushq %r15
+  pushq %r14
+  pushq %r13
+  pushq %r12
+  pushq %rbx
+  .cfi_offset %rbx, -56
+  .cfi_offset %r12, -48
+  .cfi_offset %r13, -40
+  .cfi_offset %r14, -32
+  .cfi_offset %r15, -24
+  .loc  1 1 1 prologue_end
+  popq  %rbx
+  popq  %r12
+  popq  %r13
+  popq  %r14
+  popq  %r15
+  popq  %rbp
+  .cfi_def_cfa %rsp, 8
+.endif
+.endr
+
+# pad out the line table to make sure it's big enough to warrant compression
+  .irpc i, 123456789
+    .irpc j, 0123456789
+      .loc 1 \i\j \j
+      nop
+     .endr
+  .endr
+  .cfi_endproc
+  .cfi_sections .debug_frame
+
+  .section        .debug_str,"MS",@progbits,1
+.Linfo_string0:
+        .asciz  "perfectly compressable data sample *****************************************"

llvm/tools/llvm-mc/llvm-mc.cpp

static cl::opt<bool> RelaxELFRel(
     cl::desc("Emit R_X86_64_GOTPCRELX instead of R_X86_64_GOTPCREL"),
     cl::cat(MCCategory));
 
 static cl::opt<DebugCompressionType> CompressDebugSections(
     "compress-debug-sections", cl::ValueOptional,
     cl::init(DebugCompressionType::None),
     cl::desc("Choose DWARF debug sections compression:"),
     cl::values(clEnumValN(DebugCompressionType::None, "none", "No compression"),
-               clEnumValN(DebugCompressionType::Z, "zlib",
-                          "Use zlib compression")),
+               clEnumValN(DebugCompressionType::Z, "zlib", "Use zlib"),
+               clEnumValN(DebugCompressionType::Zstd, "zstd", "Use zstd")),
     cl::cat(MCCategory));
 
 static cl::opt<bool>
     ShowInst("show-inst", cl::desc("Show internal instruction representation"),
              cl::cat(MCCategory));
 
 static cl::opt<bool>
     ShowInstOperands("show-inst-operands",
int main(int argc, char **argv) {
   std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TripleName));
   assert(MRI && "Unable to create target register info!");
 
   std::unique_ptr<MCAsmInfo> MAI(
       TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
   assert(MAI && "Unable to create target asm info!");
 
   MAI->setRelaxELFRelocations(RelaxELFRel);
+  MAI->setPreserveAsmComments(PreserveComments);
 
-  if (CompressDebugSections != DebugCompressionType::None) {
+  switch (CompressDebugSections) {
+  case DebugCompressionType::None:
+    break;
+  case DebugCompressionType::Z:
     if (!compression::zlib::isAvailable()) {
       WithColor::error(errs(), ProgName)
-          << "build tools with zlib to enable -compress-debug-sections";
+          << "build LLVM with LLVM_ENABLE_ZLIB to enable "
+             "--compress-debug-sections=zlib";
       return 1;
     }
-    MAI->setCompressDebugSections(CompressDebugSections);
+    break;
+  case DebugCompressionType::Zstd:
+    if (!compression::zstd::isAvailable()) {
+      WithColor::error(errs(), ProgName)
+          << "build LLVM with LLVM_ENABLE_ZSTD to enable "
+             "--compress-debug-sections=zstd";
+      return 1;
     }
-  MAI->setPreserveAsmComments(PreserveComments);
+    break;
+  }
+  MAI->setCompressDebugSections(CompressDebugSections);
 
   // Package up features to be passed to target/subtarget
   std::string FeaturesStr;
   if (MAttrs.size()) {
     SubtargetFeatures Features;
     for (unsigned i = 0; i != MAttrs.size(); ++i)
       Features.AddFeature(MAttrs[i]);
     FeaturesStr = Features.getString();