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

llvm/lib/ObjCopy/COFF/COFFWriter.cpp

@@ -13,6 +13,7 @@
 #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>
@@ -93,59 +94,75 @@ Error COFFWriter::finalizeSymbolContents() {
   return Error::success();
 }
 
-Error COFFWriter::finalizeCFGuardContents() {
+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;
   bool NeedUpdate = false;
-  for (Symbol &Sym : Obj.getMutableSymbols()) {
+  for (auto &Sym : Obj.getMutableSymbols()) {
     NeedUpdate |= Sym.OriginalRawIndex == Sym.RawIndex;
     SymIdMap[Sym.OriginalRawIndex] = Sym.RawIndex;
+
+    // We collect only definition symbols of the sections to update checksum
+    if (Sym.Sym.NumberOfAuxSymbols == 1 &&
+        Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC && Sym.Sym.Value == 0 &&
+        IsSymIdxSection(Sym.Name))
+      SecIdMap[Sym.TargetSectionId] =
+          reinterpret_cast<coff_aux_section_definition *>(
+              Sym.AuxData[0].Opaque);
   }
 
   if (!NeedUpdate)
     return Error::success();
 
-  for (auto &Sym : Obj.getMutableSymbols()) {
-    if (Sym.Name != ".gljmp$y" && Sym.Name != ".giats$y" &&
-        Sym.Name != ".gfids$y")
+  for (auto &Sec : Obj.getMutableSections()) {
+    if (!IsSymIdxSection(Sec.Name))
       continue;
 
-    auto Sec = find_if(Obj.getMutableSections(),
-                       [&Sym](Section &S) { return S.Name == Sym.Name; });
-
-    if (Sec == Obj.getMutableSections().end() ||
-        Sec->UniqueId != Sym.TargetSectionId)
-      return createStringError(object_error::invalid_symbol_index,
-                               "symbol '%s' is missing its section",
-                               Sym.Name.str().c_str());
-
-    if (Sym.Sym.NumberOfAuxSymbols != 1 ||
-        Sym.Sym.StorageClass != IMAGE_SYM_CLASS_STATIC)
-      return createStringError(object_error::invalid_symbol_index,
-                               "symbol '%s' has unexpected section format",
-                               Sym.Name.str().c_str());
-
-    ArrayRef<uint8_t> RawIds = Sec->getContents();
+    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)
-      return Error::success();
+      continue;
+
+    if (!SecIdMap.contains(Sec.UniqueId))
+      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<uint32_t> Ids(reinterpret_cast<const uint32_t *>(RawIds.data()),
-                           RawIds.size() / 4);
-    std::vector<uint32_t> NewIds;
-    for (auto Id : Ids)
-      NewIds.push_back(SymIdMap[Id]);
+    ArrayRef<support::ulittle32_t> Ids(
+        reinterpret_cast<const support::ulittle32_t *>(RawIds.data()),
+        RawIds.size() / 4);
+    std::vector<support::ulittle32_t> NewIds;
+    for (auto Id : Ids) {
+      if (!SymIdMap.contains(Id))
+        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(SymIdMap[Id]));
+    }
     ArrayRef<uint8_t> NewRawIds(reinterpret_cast<uint8_t *>(NewIds.data()),
                                 RawIds.size());
     // Update check sum
     JamCRC JC(/*Init=*/0);
     JC.update(NewRawIds);
-    coff_aux_section_definition *SD =
-        reinterpret_cast<coff_aux_section_definition *>(Sym.AuxData[0].Opaque);
-    SD->CheckSum = JC.getCRC();
+    SecIdMap[Sec.UniqueId]->CheckSum = JC.getCRC();
     // Set new content
-    Sec->setOwnedContents(NewRawIds);
+    Sec.setOwnedContents(NewRawIds.vec());
   }
   return Error::success();
 }
@@ -241,7 +258,7 @@ Error COFFWriter::finalize(bool IsBigObj) {
     return E;
   if (Error E = finalizeSymbolContents())
     return E;
-  if (Error E = finalizeCFGuardContents())
+  if (Error E = finalizeSymIdxContents())
     return E;
 
   size_t SizeOfHeaders = 0;

llvm/lib/ObjCopy/COFF/COFFWriter.h

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

llvm/test/tools/llvm-objcopy/COFF/strip-update-ehcont.test

@@ -0,0 +1,201 @@
+# RUN: yaml2obj %s -o %t.in.o
+
+# RUN: llvm-readobj -r -s  -x '.gehcont$y' %t.in.o | FileCheck %s --check-prefix=ORIG
+# RUN: llvm-objcopy --strip-debug %t.in.o %t.out.o
+# RUN: llvm-readobj -r -s  -x '.gehcont$y' %t.out.o | FileCheck %s --check-prefix=STRIP
+
+# ORIG:      Symbols [
+# ORIG:          Name: .text
+# ORIG:          Name: .data
+# ORIG:          Name: .bss
+# ORIG:          Name: .text
+# ORIG:          Name: ?foo@@YAXXZ
+# ORIG:          Name: .data
+# ORIG:          Name: .drectve
+# ORIG:          Name: .debug$S
+# ORIG:          Name: .gehcont$y
+# ORIG:          Name: ?foo2@@YAXXZ
+# ORIG:          Name: $ehgcr_0_1
+# ORIG:      Hex dump of section '.gehcont$y':
+# ORIG-NEXT: 0x00000000 12000000 12000000 12000000          ............
+
+# .debug$S is going to be stripped and $ehgcr_0_1 index is decreased by 2
+
+# STRIP:      Symbols [
+# STRIP:          Name: .text
+# STRIP:          Name: .data
+# STRIP:          Name: .bss
+# STRIP:          Name: .text
+# STRIP:          Name: ?foo@@YAXXZ
+# STRIP:          Name: .data
+# STRIP:          Name: .drectve
+# STRIP:          Name: .gehcont$y
+# STRIP:          Name: ?foo2@@YAXXZ
+# STRIP:          Name: $ehgcr_0_1
+# STRIP:      Hex dump of section '.gehcont$y':
+# STRIP-NEXT: 0x00000000 10000000 10000000 10000000          ............
+
+--- !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:     ''
+  - Name:            .data
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    Alignment:       4
+    SectionData:     ''
+  - Name:            .bss
+    Characteristics: [ IMAGE_SCN_CNT_UNINITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    Alignment:       4
+    SectionData:     ''
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     554883EC30488D6C243048C745F0FEFFFFFFE800000000904883C4305DC366904889542410554883EC20488D6A30488D05E2FFFFFF4883C4205DC3
+    SizeOfRawData:   59
+    Relocations:
+      - VirtualAddress:  19
+        SymbolName:      '?foo2@@YAXXZ'
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            .data
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    Alignment:       16
+    SectionData:     '000000000000000000000000000000002E48000000000000'
+    SizeOfRawData:   24
+  - Name:            .drectve
+    Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
+    Alignment:       1
+    SectionData:     202F44454641554C544C49423A6C6962636D742E6C6962202F44454641554C544C49423A6C69626972636D742E6C6962202F44454641554C544C49423A73766D6C5F646973706D742E6C6962202F44454641554C544C49423A6C69626D6D742E6C6962202F44454641554C544C49423A6F6C646E616D65732E6C6962
+    SizeOfRawData:   124
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     04000000F100000044656275672073656374696F6E20746F20626520737472697070656400
+    SizeOfRawData:   37
+  - Name:            '.gehcont$y'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '120000001200000012000000'
+    SizeOfRawData:   12
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          0
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          1
+  - Name:            .data
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          0
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          2
+  - Name:            .bss
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          0
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          3
+  - Name:            .text
+    Value:           0
+    SectionNumber:   4
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          59
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        517419950
+      Number:          4
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            '?foo@@YAXXZ'
+    Value:           0
+    SectionNumber:   4
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            .data
+    Value:           0
+    SectionNumber:   5
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          24
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        2602060666
+      Number:          5
+      Selection:       IMAGE_COMDAT_SELECT_ANY
+  - Name:            .drectve
+    Value:           0
+    SectionNumber:   6
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          124
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        1122646683
+      Number:          6
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   7
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          37
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        820498156
+      Number:          7
+  - Name:            '.gehcont$y'
+    Value:           0
+    SectionNumber:   8
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          12
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        820498156
+      Number:          8
+  - Name:            '?foo2@@YAXXZ'
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            '$ehgcr_0_1'
+    Value:           23
+    SectionNumber:   4
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+...