[llvm-branch-commits] [llvm] release/23.x: [llvm-objcopy][COFF] Keep COMDAT section definition symbols when stripping (#228783) (PR #229026)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: llvmbot

<details>
<summary>Changes</summary>

Backport cf40a83cb398897d21bb54c5a7a2eecb8d0633b3

Requested by: @<!-- -->mstorsjo

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


2 Files Affected:

- (modified) llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp (+16) 
- (added) llvm/test/tools/llvm-objcopy/COFF/strip-unneeded-comdat.test (+94) 


``````````diff
diff --git a/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp b/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp
index 48c488826c47d59..96de61737d78fbb 100644
--- a/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp
@@ -32,6 +32,19 @@ static bool isDebugSection(const Section &Sec) {
   return Sec.Name.starts_with(".debug");
 }
 
+// The COMDAT selection is stored in the section definition symbol's aux
+// record, so it must be kept even if no relocation references it.
+static bool isComdatSectionDefinition(const Object &Obj, const Symbol &Sym) {
+  if (Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC && Sym.Sym.Value == 0 &&
+      Sym.Sym.NumberOfAuxSymbols != 0 && Sym.TargetSectionId > 0) {
+    // Looks like a section definition symbol, check if it is a COMDAT.
+    const Section *Sec = Obj.findSection(Sym.TargetSectionId);
+    return Sec && Sec->Name == Sym.Name &&
+           (Sec->Header.Characteristics & IMAGE_SCN_LNK_COMDAT);
+  }
+  return false;
+}
+
 static uint64_t getNextRVA(const Object &Obj) {
   if (Obj.getSections().empty())
     return 0;
@@ -233,6 +246,9 @@ static Error handleArgs(const CommonConfig &Config,
       return true;
     }
 
+    if (isComdatSectionDefinition(Obj, Sym))
+      return false;
+
     if (!Sym.Referenced) {
       // With --strip-unneeded, GNU objcopy removes all unreferenced local
       // symbols, and any unreferenced undefined external.
diff --git a/llvm/test/tools/llvm-objcopy/COFF/strip-unneeded-comdat.test b/llvm/test/tools/llvm-objcopy/COFF/strip-unneeded-comdat.test
new file mode 100644
index 000000000000000..8af57d031ee87b3
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/COFF/strip-unneeded-comdat.test
@@ -0,0 +1,94 @@
+## Check that --strip-unneeded and --discard-all keep the section definition
+## symbol of a COMDAT section, even if no relocation references it. It carries
+## the COMDAT selection, without it the linker discards the section.
+
+# RUN: yaml2obj %s -o %t.in.o
+# RUN: llvm-objcopy --strip-unneeded %t.in.o %t.strip-unneeded.o
+# RUN: llvm-readobj --symbols %t.strip-unneeded.o | FileCheck %s
+# RUN: llvm-objcopy --discard-all %t.in.o %t.discard-all.o
+# RUN: llvm-readobj --symbols %t.discard-all.o | FileCheck %s
+
+# CHECK:      Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: .rdata$.refptr.foo
+# CHECK-NEXT:     Value: 0
+# CHECK-NEXT:     Section: .rdata$.refptr.foo (2)
+# CHECK-NEXT:     BaseType: Null (0x0)
+# CHECK-NEXT:     ComplexType: Null (0x0)
+# CHECK-NEXT:     StorageClass: Static (0x3)
+# CHECK-NEXT:     AuxSymbolCount: 1
+# CHECK-NEXT:     AuxSectionDef {
+# CHECK-NEXT:       Length: 8
+# CHECK-NEXT:       RelocationCount: 1
+# CHECK-NEXT:       LineNumberCount: 0
+# CHECK-NEXT:       Checksum: 0x0
+# CHECK-NEXT:       Number: 2
+# CHECK-NEXT:       Selection: Any (0x2)
+# CHECK-NEXT:     }
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: .refptr.foo
+# CHECK:        Symbol {
+# CHECK-NEXT:     Name: foo
+# CHECK-NOT:      Name: .text
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     488B0500000000C3
+    Relocations:
+      - VirtualAddress:  3
+        SymbolName:      .refptr.foo
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            '.rdata$.refptr.foo'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_READ ]
+    Alignment:       8
+    SectionData:     '0000000000000000'
+    Relocations:
+      - VirtualAddress:  0
+        SymbolName:      foo
+        Type:            IMAGE_REL_AMD64_ADDR64
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          8
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          1
+  - Name:            '.rdata$.refptr.foo'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          8
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          2
+      Selection:       IMAGE_COMDAT_SELECT_ANY
+  - Name:            .refptr.foo
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            foo
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

``````````

</details>


https://github.com/llvm/llvm-project/pull/229026