[llvm-objcopy][COFF] Update .symidx values after stripping

llvm/lib/ObjCopy/COFF/COFFObject.cpp

@@ -18,6 +18,8 @@ using namespace object;
 void Object::addSymbols(ArrayRef<Symbol> NewSymbols) {
   for (Symbol S : NewSymbols) {
     S.UniqueId = NextSymbolUniqueId++;
+    S.OriginalRawIndex = NextSymbolOriginalIndex;
+    NextSymbolOriginalIndex += 1 + S.Sym.NumberOfAuxSymbols;
     Symbols.emplace_back(S);
   }
   updateSymbols();

llvm/lib/ObjCopy/COFF/COFFObject.h

@@ -89,6 +89,7 @@ struct Symbol {
   std::optional<size_t> WeakTargetSymbolId;
   size_t UniqueId;
   size_t RawIndex;
+  size_t OriginalRawIndex;
   bool Referenced;
 };
 
@@ -140,6 +141,7 @@ struct Object {
   DenseMap<size_t, Symbol *> SymbolMap;
 
   size_t NextSymbolUniqueId = 0;
+  size_t NextSymbolOriginalIndex = 0;
 
   std::vector<Section> Sections;
   DenseMap<ssize_t, Section *> SectionMap;

llvm/lib/ObjCopy/COFF/COFFWriter.cpp

@@ -12,6 +12,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/COFF.h"
+#include "llvm/Support/CRC.h"
+#include "llvm/Support/Endian.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstddef>
@@ -92,6 +94,76 @@ Error COFFWriter::finalizeSymbolContents() {
   return Error::success();
 }
 
+Error COFFWriter::finalizeSymIdxContents() {
+  // CFGuards shouldn't be present in PE.
+  if (Obj.IsPE)
+    return Error::success();
+
+  // Currently handle only sections consisting only of .symidx.
+  // TODO: other sections such as .impcall and .hybmp$x require more complex
+  // handling as they have more complex layout.
+  auto IsSymIdxSection = [](StringRef Name) {
+    return Name == ".gljmp$y" || Name == ".giats$y" || Name == ".gfids$y" ||
+           Name == ".gehcont$y";
+  };
+
+  DenseMap<size_t, size_t> SymIdMap;
+  SmallDenseMap<ssize_t, coff_aux_section_definition *, 4> SecIdMap;
+  for (Symbol &Sym : Obj.getMutableSymbols()) {
+    SymIdMap[Sym.OriginalRawIndex] = Sym.RawIndex;
+
+    // We collect only definition symbols of the sections to update checksum.
+    if (Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC &&
+        Sym.Sym.NumberOfAuxSymbols == 1 && Sym.Sym.Value == 0 &&
+        IsSymIdxSection(Sym.Name))
+      SecIdMap[Sym.TargetSectionId] =
+          reinterpret_cast<coff_aux_section_definition *>(
+              Sym.AuxData[0].Opaque);
+  }
+
+  for (Section &Sec : Obj.getMutableSections()) {
+    if (!IsSymIdxSection(Sec.Name))
+      continue;
+
+    ArrayRef<uint8_t> RawIds = Sec.getContents();
+    // Nothing to do and also CheckSum will be -1 instead of 0 if we recalculate
+    // it on empty input.
+    if (RawIds.size() == 0)
+      continue;
+
+    auto SecDefIt = SecIdMap.find(Sec.UniqueId);
+    if (SecDefIt == SecIdMap.end())
+      return createStringError(object_error::invalid_symbol_index,
+                               "section '%s' does not have the corresponding "
+                               "symbol or the symbol has unexpected format",
+                               Sec.Name.str().c_str());
+
+    // Create updated content.
+    ArrayRef<support::ulittle32_t> Ids(
+        reinterpret_cast<const support::ulittle32_t *>(RawIds.data()),
+        RawIds.size() / 4);
+    std::vector<support::ulittle32_t> NewIds;
+    for (support::ulittle32_t Id : Ids) {
+      auto SymIdIt = SymIdMap.find(Id);
+      if (SymIdIt == SymIdMap.end())
+        return createStringError(object_error::invalid_symbol_index,
+                                 "section '%s' contains a .symidx (%d) that is "
+                                 "incorrect or was stripped",
+                                 Sec.Name.str().c_str(), Id.value());
+      NewIds.push_back(support::ulittle32_t(SymIdIt->getSecond()));
+    }
+    ArrayRef<uint8_t> NewRawIds(reinterpret_cast<uint8_t *>(NewIds.data()),
+                                RawIds.size());
+    // Update check sum.
+    JamCRC JC(/*Init=*/0);
+    JC.update(NewRawIds);
+    SecDefIt->getSecond()->CheckSum = JC.getCRC();
+    // Set new content.
+    Sec.setOwnedContents(NewRawIds.vec());
+  }
+  return Error::success();
+}
+
 void COFFWriter::layoutSections() {
   for (auto &S : Obj.getMutableSections()) {
     if (S.Header.SizeOfRawData > 0)
@@ -183,6 +255,8 @@ Error COFFWriter::finalize(bool IsBigObj) {
     return E;
   if (Error E = finalizeSymbolContents())
     return E;
+  if (Error E = finalizeSymIdxContents())
+    return E;
 
   size_t SizeOfHeaders = 0;
   FileAlignment = 1;

llvm/lib/ObjCopy/COFF/COFFWriter.h

@@ -34,6 +34,7 @@ class COFFWriter {
   template <class SymbolTy> std::pair<size_t, size_t> finalizeSymbolTable();
   Error finalizeRelocTargets();
   Error finalizeSymbolContents();
+  Error finalizeSymIdxContents();
   void layoutSections();
   Expected<size_t> finalizeStringTable();
 

llvm/test/tools/llvm-objcopy/COFF/strip-invalid-symidx-section-1.test

@@ -0,0 +1,75 @@
+# Bail out if a section consisting of symidx has unexpected format for patching
+# its content and updating a check sum. Basically, the expected format is a
+# definitive symbol with a zero offset that is linked with section.
+# In the case, the symbol .gfids$y is not present at all.
+
+# RUN: yaml2obj %s -o %t.in.o
+# RUN: llvm-readobj -r -s -x '.gfids$y' %t.in.o | FileCheck %s --check-prefix=ORIG
+# RUN: not llvm-objcopy --strip-debug %t.in.o %t.out.o 2>&1 | FileCheck %s --check-prefix=ERROR
+
+# ORIG:      Relocations [
+# ORIG-NEXT:   Section (1) .text {
+# ORIG-NEXT:     0x3 IMAGE_REL_AMD64_REL32 foo (4)
+# ORIG-NEXT:   }
+# ORIG-NEXT: ]
+# ORIG:      Hex dump of section '.gfids$y':
+# ORIG-NEXT: 0x00000000 04000000
+
+# ERROR: section '.gfids$y' does not have the corresponding symbol or the symbol has unexpected format
+
+--- !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:       4
+    SectionData:     488D0500000000488D0D00000000488D0500000000488D0500000000
+    SizeOfRawData:   28
+    Relocations:
+      - VirtualAddress:  3
+        SymbolName:      foo
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     04000000F100000044656275672073656374696F6E20746F20626520737472697070656400
+    SizeOfRawData:   37
+  - Name:            '.gfids$y'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '04000000'
+    SizeOfRawData:   8
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          28
+      NumberOfRelocations: 4
+      NumberOfLinenumbers: 0
+      CheckSum:        3583480811
+      Number:          1
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          37
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        2941632545
+      Number:          4
+  - Name:            foo
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

llvm/test/tools/llvm-objcopy/COFF/strip-invalid-symidx-section-2.test

@@ -0,0 +1,95 @@
+# Bail out if a section consisting of symidx has unexpected format for patching
+# its content and updating a check sum. Basically, the expected format is a
+# definitive symbol with a zero offset that is linked with section.
+# In the case, the symbol .giats$y has a non-zero offset.
+
+# RUN: yaml2obj %s -o %t.in.o
+# RUN: llvm-readobj -r -s -x '.giats$y' %t.in.o | FileCheck %s --check-prefix=ORIG
+# RUN: not llvm-objcopy --strip-debug %t.in.o %t.out.o 2>&1 | FileCheck %s --check-prefix=ERROR
+
+# ORIG:      Relocations [
+# ORIG-NEXT:   Section (1) .text {
+# ORIG-NEXT:     0x3 IMAGE_REL_AMD64_REL32 foo (6)
+# ORIG-NEXT:   }
+# ORIG-NEXT: ]
+# ORIG:      Symbols [
+# ORIG:          Name: .giats$y
+# ORIG:          Section: .giats$y
+# ORIG:          AuxSymbolCount: 1
+# ORIG:          AuxSectionDef {
+# ORIG:            Checksum: 0x459345AD
+# ORIG:          }
+# ORIG:      ]
+# ORIG:      Hex dump of section '.giats$y':
+# ORIG-NEXT: 0x00000000 06000000 10000000                   ........
+
+# ERROR: section '.giats$y' does not have the corresponding symbol or the symbol has unexpected format
+
+--- !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:       4
+    SectionData:     488D0500000000488D0D00000000488D0500000000488D0500000000
+    SizeOfRawData:   28
+    Relocations:
+      - VirtualAddress:  3
+        SymbolName:      foo
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     04000000F100000044656275672073656374696F6E20746F20626520737472697070656400
+    SizeOfRawData:   37
+  - Name:            '.giats$y'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '0600000010000000'
+    SizeOfRawData:   8
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          28
+      NumberOfRelocations: 4
+      NumberOfLinenumbers: 0
+      CheckSum:        3583480811
+      Number:          1
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          37
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        2941632545
+      Number:          4
+  - Name:            '.giats$y'
+    Value:           42
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          8
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        1167279533
+      Number:          5
+  - Name:            foo
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

llvm/test/tools/llvm-objcopy/COFF/strip-invalid-symidx-section-3.test

@@ -0,0 +1,86 @@
+# Bail out if a section consisting of symidx has unexpected format for patching
+# its content and updating a check sum. Basically, the expected format is a
+# definitive symbol with a zero offset that is linked with section.
+# In the case, the symbol .gljmp$y has a non-static storage class.
+
+# RUN: yaml2obj %s -o %t.in.o
+# RUN: llvm-readobj -r -s -x '.gljmp$y' %t.in.o | FileCheck %s --check-prefix=ORIG
+# RUN: not llvm-objcopy --strip-debug %t.in.o %t.out.o 2>&1 | FileCheck %s --check-prefix=ERROR
+
+# ORIG:      Relocations [
+# ORIG-NEXT:   Section (1) .text {
+# ORIG-NEXT:     0x3 IMAGE_REL_AMD64_REL32 foo (5)
+# ORIG-NEXT:   }
+# ORIG-NEXT: ]
+# ORIG:      Symbols [
+# ORIG:          Name: .gljmp$y
+# ORIG:          Section: .gljmp$y
+# ORIG:          AuxSymbolCount: 0
+# ORIG:      ]
+# ORIG:      Hex dump of section '.gljmp$y':
+# ORIG-NEXT: 0x00000000 06000000 10000000                   ........
+
+# ERROR: section '.gljmp$y' does not have the corresponding symbol or the symbol has unexpected format
+
+--- !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:       4
+    SectionData:     488D0500000000488D0D00000000488D0500000000488D0500000000
+    SizeOfRawData:   28
+    Relocations:
+      - VirtualAddress:  3
+        SymbolName:      foo
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     04000000F100000044656275672073656374696F6E20746F20626520737472697070656400
+    SizeOfRawData:   37
+  - Name:            '.gljmp$y'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '0600000010000000'
+    SizeOfRawData:   8
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          28
+      NumberOfRelocations: 4
+      NumberOfLinenumbers: 0
+      CheckSum:        3583480811
+      Number:          1
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          37
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        2941632545
+      Number:          4
+  - Name:            '.gljmp$y'
+    Value:           0
+    SectionNumber:   3
+    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
+...