[llvm-objcopy] --[de]compress-debug-sections: don't compress SHF_ALLOC sections, only decompress .debug sections

[MaskRay]

Simplify --[de]compress-debug-sections to make it easier to add custom section [de]compression.
Change the following two behaviors to match GNU objcopy.

• --compress-debug-sections compresses SHF_ALLOC sections while GNU
  doesn't.
• --decompress-debug-sections decompresses non-debug sections while GNU
  doesn't.

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: Fangrui Song (MaskRay)

Changes

Make it easier to add custom section [de]compression. In GNU ld,
--compress-debug-sections doesn't compress SHF_ALLOC sections. Match its
behavior.

---

Full diff: https://github.com/llvm/llvm-project/pull/84885.diff

4 Files Affected:

• (modified) llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp (+25-40)
• (modified) llvm/lib/ObjCopy/ELF/ELFObject.h (+1)
• (modified) llvm/test/tools/llvm-objcopy/ELF/Inputs/compress-debug-sections.yaml (+4)
• (modified) llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zlib.test (+2)

diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index f52bcb74938d15..36826e02a62712 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -214,33 +214,34 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
                            SecName.str().c_str());
 }
 
-static bool isCompressable(const SectionBase &Sec) {
-  return !(Sec.Flags & ELF::SHF_COMPRESSED) &&
-         StringRef(Sec.Name).starts_with(".debug");
-}
-
-static Error replaceDebugSections(
-    Object &Obj, function_ref<bool(const SectionBase &)> ShouldReplace,
-    function_ref<Expected<SectionBase *>(const SectionBase *)> AddSection) {
+Error Object::compressOrDecompressSections(const CommonConfig &Config) {
   // Build a list of the debug sections we are going to replace.
   // We can't call `AddSection` while iterating over sections,
   // because it would mutate the sections array.
-  SmallVector<SectionBase *, 13> ToReplace;
-  for (auto &Sec : Obj.sections())
-    if (ShouldReplace(Sec))
-      ToReplace.push_back(&Sec);
-
-  // Build a mapping from original section to a new one.
-  DenseMap<SectionBase *, SectionBase *> FromTo;
-  for (SectionBase *S : ToReplace) {
-    Expected<SectionBase *> NewSection = AddSection(S);
-    if (!NewSection)
-      return NewSection.takeError();
-
-    FromTo[S] = *NewSection;
+  SmallVector<std::pair<SectionBase *, std::function<SectionBase *()>>, 0>
+      ToReplace;
+  for (SectionBase &Sec : sections()) {
+    if (!StringRef(Sec.Name).starts_with(".debug"))
+      continue;
+    if (auto *CS = dyn_cast<CompressedSection>(&Sec)) {
+      if (Config.DecompressDebugSections) {
+        ToReplace.emplace_back(
+            &Sec, [=] { return &addSection<DecompressedSection>(*CS); });
+      }
+    } else if (!(Sec.Flags & SHF_ALLOC) &&
+               Config.CompressionType != DebugCompressionType::None) {
+      auto *S = &Sec;
+      ToReplace.emplace_back(S, [=] {
+        return &addSection<CompressedSection>(
+            CompressedSection(*S, Config.CompressionType, Is64Bits));
+      });
+    }
   }
 
-  return Obj.replaceSections(FromTo);
+  DenseMap<SectionBase *, SectionBase *> FromTo;
+  for (auto [S, Func] : ToReplace)
+    FromTo[S] = Func();
+  return replaceSections(FromTo);
 }
 
 static bool isAArch64MappingSymbol(const Symbol &Sym) {
@@ -534,24 +535,8 @@ 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, 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);
-              return &Obj.addSection<DecompressedSection>(*CS);
-            }))
-      return Err;
-  }
+  if (Error E = Obj.compressOrDecompressSections(Config))
+    return E;
 
   return Error::success();
 }
diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.h b/llvm/lib/ObjCopy/ELF/ELFObject.h
index 7a2e20d82d1150..f72c109b6009e8 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObject.h
+++ b/llvm/lib/ObjCopy/ELF/ELFObject.h
@@ -1210,6 +1210,7 @@ class Object {
 
   Error removeSections(bool AllowBrokenLinks,
                        std::function<bool(const SectionBase &)> ToRemove);
+  Error compressOrDecompressSections(const CommonConfig &Config);
   Error replaceSections(const DenseMap<SectionBase *, SectionBase *> &FromTo);
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   template <class T, class... Ts> T &addSection(Ts &&...Args) {
diff --git a/llvm/test/tools/llvm-objcopy/ELF/Inputs/compress-debug-sections.yaml b/llvm/test/tools/llvm-objcopy/ELF/Inputs/compress-debug-sections.yaml
index 67d8435fa486c1..e2dfee9163a2b8 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/Inputs/compress-debug-sections.yaml
+++ b/llvm/test/tools/llvm-objcopy/ELF/Inputs/compress-debug-sections.yaml
@@ -43,6 +43,10 @@ Sections:
     Type:            SHT_PROGBITS
     Flags:           [ SHF_GROUP ]
     Content:         '00'
+  - Name:            .debug_alloc
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f
 Symbols:
   - Type:    STT_SECTION
     Section: .debug_foo
diff --git a/llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zlib.test b/llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zlib.test
index e1ebeed8d4fcb0..056ae84ce4915f 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zlib.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/compress-debug-sections-zlib.test
@@ -12,8 +12,10 @@
 # CHECK:             Name              Type            Address          Off    Size   ES Flg Lk Inf Al
 # COMPRESSED:        .debug_foo        PROGBITS        0000000000000000 000040 {{.*}} 00   C  0   0  8
 # COMPRESSED-NEXT:   .notdebug_foo     PROGBITS        0000000000000000 {{.*}} 000008 00      0   0  0
+# COMPRESSED:        .debug_alloc      PROGBITS        0000000000000000 {{.*}} 000040 00   A  0   0  0
 # UNCOMPRESSED:      .debug_foo        PROGBITS        0000000000000000 000040 000008 00      0   0  0
 # UNCOMPRESSED-NEXT: .notdebug_foo     PROGBITS        0000000000000000 {{.*}} 000008 00      0   0  0
+# UNCOMPRESSED:      .debug_alloc      PROGBITS        0000000000000000 {{.*}} 000040 00   A  0   0  0
 
 ## Relocations do not change.
 # CHECK:             Relocation section '.rela.debug_foo' at offset {{.*}} contains 2 entries:

[jh7370]

Looks basically fine to me.

[jh7370]

You presumably need changes to the zstd version of this test too?

[MaskRay]

Changed

[jh7370]

One test bug. Otherwise LGTM.

[jh7370]

Wrong pattern?

[MaskRay]

Thanks. Fixed