[llvm-strip] Remove empty SHT_GROUP sections.

[chestnykh]

Currently llvm-strip in --strip-debug mode doesn't remove such sections. This behavior can lead to incompatibilities with GNU binutils (for examples ld.bfd cannot process the object file contains empty .group section). The ELF object that contains group section
with .debug_* sections inside can be obtained by gcc -g3. Fix #97139

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[chestnykh]

@MaskRay sorry, i've accidentally closed #97141. This PR contains the same changes.

CC: @jh7370

[llvmbot]

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

Author: Dmitriy Chestnykh (chestnykh)

Changes

Currently llvm-strip in --strip-debug mode doesn't remove such sections. This behavior can lead to incompatibilities with GNU binutils (for examples ld.bfd cannot process the object file contains empty .group section). The ELF object that contains group section
with .debug_* sections inside can be obtained by gcc -g3. Fix #97139

---

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

3 Files Affected:

• (modified) llvm/lib/ObjCopy/ELF/ELFObject.cpp (+9-1)
• (modified) llvm/lib/ObjCopy/ELF/ELFObject.h (+2)
• (added) llvm/test/tools/llvm-objcopy/ELF/strip-debug-empty-group.test (+31)

diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.cpp b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
index 02591e6f987c2..44181ccd328bf 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObject.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
@@ -2206,8 +2206,16 @@ Error Object::removeSections(
   // Transfer removed sections into the Object RemovedSections container for use
   // later.
   std::move(Iter, Sections.end(), std::back_inserter(RemovedSections));
-  // Now finally get rid of them all together.
+  // Now get rid of them all together.
   Sections.erase(Iter, std::end(Sections));
+
+  // Finally iterate over all sections and erase empty SHT_GROUP sections.
+  for (auto Iter = Sections.begin(); Iter != Sections.end(); ++Iter) {
+    if (auto GroupSec = dyn_cast<GroupSection>(Iter->get())) {
+      if (GroupSec->getMembersCount() == 0)
+        Sections.erase(Iter);
+    }
+  }
   return Error::success();
 }
 
diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.h b/llvm/lib/ObjCopy/ELF/ELFObject.h
index 2b1895a30b41e..9bbef268017ea 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObject.h
+++ b/llvm/lib/ObjCopy/ELF/ELFObject.h
@@ -963,6 +963,8 @@ class GroupSection : public SectionBase {
       const DenseMap<SectionBase *, SectionBase *> &FromTo) override;
   void onRemove() override;
 
+  size_t getMembersCount() {return GroupMembers.size(); }
+
   static bool classof(const SectionBase *S) {
     return S->OriginalType == ELF::SHT_GROUP;
   }
diff --git a/llvm/test/tools/llvm-objcopy/ELF/strip-debug-empty-group.test b/llvm/test/tools/llvm-objcopy/ELF/strip-debug-empty-group.test
new file mode 100644
index 0000000000000..3e09674e2288d
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/ELF/strip-debug-empty-group.test
@@ -0,0 +1,31 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-strip --strip-debug %t -o %t1
+# RUN: llvm-readelf --section-groups %t1 | FileCheck %s
+# RUN: llvm-strip --strip-unneeded %t -o %t2
+# RUN: llvm-readelf --section-groups %t2 | FileCheck %s
+# RUN: llvm-strip --remove-section=.debug_macro %t -o %t3
+# RUN: llvm-readelf --section-groups %t3 | FileCheck %s
+# RUN: llvm-strip --remove-section=.debug_* %t -o %t4
+# RUN: llvm-readelf --section-groups %t4 | FileCheck %s
+
+--- !ELF
+FileHeader:
+  Class:      ELFCLASS64
+  Data:       ELFDATA2LSB
+  Type:       ET_REL
+  Machine:    EM_X86_64
+Sections:
+  - Name:     .group
+    Type:     SHT_GROUP
+    Info:     foo_grp
+    Members:
+      - SectionOrType:  GRP_COMDAT
+      - SectionOrType:  .debug_macro
+  - Name:     .debug_macro
+    Type:     SHT_PROGBITS
+    Flags:    [ SHF_GROUP ]
+Symbols:
+  - Name:     foo_grp
+    Section:  .group
+
+# CHECK: There are no section groups in this file.

[tschuett]

At least for the for loop, I am not convinced that braces are needed. Furthermore:
https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop

[tschuett]

Or:

llvm::remove_if(Sections, Lambda_Predicate);

[chestnykh]

Removed the braces. The loop cannot be rewritten to auto I = Sections.begin(), E = Sections.end(); I != E; ++I because Sections may change size every time

[chestnykh]

llvm::remove_if as i can see isn't very widely used so i think we can leave for here