[llvm-objcopy] - Do not crash on object that has relocations but no symbol table.

Georgii Rymar · Georgii Rymar · commit a795bd964544 · 2019-10-30T13:17:22.000+03:00
It was revealed by D69260. Tool crashed when scanned relocations in a object without a symbol table. This patch teaches it either to handle such objects (when relocations does not use symbols we do not need a symbol table to proceed) or to show an appropriate error otherwise. Differential revision: https://reviews.llvm.org/D69304
diff --git a/llvm/test/tools/llvm-objcopy/ELF/no-symbol-relocation.test b/llvm/test/tools/llvm-objcopy/ELF/no-symbol-relocation.test
@@ -1,8 +1,8 @@
-# RUN: yaml2obj %s > %t
-# RUN: llvm-objcopy %t %t2
+# RUN: yaml2obj --docnum=1 %s > %t1
+# RUN: llvm-objcopy %t1 %t2
 # RUN: llvm-readobj --relocations %t2 | FileCheck %s
 
-!ELF
+--- !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
@@ -21,11 +21,50 @@ Sections:
     Relocations:
       - Offset: 0x1000
         Type:   R_X86_64_RELATIVE
-## TODO: llvm-objcopy crashes without the following line.
-Symbols: []
 
 # CHECK:     Relocations [
 # CHECK-NEXT:  Section (2) .rel.text {
 # CHECK-NEXT:    0x1000 R_X86_64_RELATIVE - 0x0
 # CHECK-NEXT:  }
 # CHECK-NEXT:]
+
+## Check we produce a valid output when stripping unneeded symbols from an object that
+## has a symbol table and a relocation with a symbol index of 0.
+
+# RUN: yaml2obj --docnum=2 %s > %t3
+# RUN: llvm-objcopy --strip-unneeded %t3 %t4
+# RUN: llvm-readobj --relocations --sections --symbols %t4 | FileCheck %s --check-prefix=STRIP
+
+# STRIP:      Relocations [
+# STRIP-NEXT:   Section {{.*}} .rel.text {
+# STRIP-NEXT:     0x1000 R_X86_64_NONE - 0x0
+# STRIP-NEXT:   }
+# STRIP-NEXT: ]
+# STRIP-NEXT: Symbols [
+# STRIP-NEXT:   Symbol {
+# STRIP-NEXT:     Name:  (0)
+# STRIP-NEXT:     Value: 0x0
+# STRIP-NEXT:     Size: 0
+# STRIP-NEXT:     Binding: Local (0x0)
+# STRIP-NEXT:     Type: None (0x0)
+# STRIP-NEXT:     Other: 0
+# STRIP-NEXT:     Section: Undefined (0x0)
+# STRIP-NEXT:   }
+# STRIP-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:  .text
+    Type:  SHT_PROGBITS
+  - Name:  .rel.text
+    Type:  SHT_REL
+    Info:  .text
+    Relocations:
+      - Offset: 0x1000
+        Type:   R_X86_64_NONE
+Symbols: []
diff --git a/llvm/test/tools/llvm-objcopy/ELF/relocations-no-symtab.test b/llvm/test/tools/llvm-objcopy/ELF/relocations-no-symtab.test
@@ -0,0 +1,55 @@
+## Check that we can copy an object that has a relocation
+## with a symbol index of 0 even when there is no symbol table.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-objcopy %t1 %t2
+# RUN: llvm-readobj --relocations %t2 | FileCheck %s
+
+# CHECK:     Relocations [
+# CHECK-NEXT:  Section {{.*}} .rel.text {
+# CHECK-NEXT:    0x1000 R_X86_64_RELATIVE - 0x0
+# CHECK-NEXT:  }
+# CHECK-NEXT:]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:  .text
+    Type:  SHT_PROGBITS
+    Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:  .rel.text
+    Type:  SHT_REL
+    Info:  .text
+    Relocations:
+      - Offset: 0x1000
+        Type:   R_X86_64_RELATIVE
+
+## Check that we report an error when a relocation refers to a
+## non-zero symbol index but there is no symbol table.
+
+# RUN: yaml2obj --docnum=2 %s -o %t3
+# RUN: not llvm-objcopy %t3 /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR: error: '.rel.text': relocation references symbol with index 1, but there is no symbol table
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:  .text
+    Type:  SHT_PROGBITS
+    Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:  .rel.text
+    Type:  SHT_REL
+    Info:  .text
+    Relocations:
+      - Offset: 0x1000
+        Symbol: 1
+        Type:   R_X86_64_NONE
diff --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -815,7 +815,8 @@ Error RelocationSection::removeSectionReferences(
   }
 
   for (const Relocation &R : Relocations) {
-    if (!R.RelocSymbol->DefinedIn || !ToRemove(R.RelocSymbol->DefinedIn))
+    if (!R.RelocSymbol || !R.RelocSymbol->DefinedIn ||
+        !ToRemove(R.RelocSymbol->DefinedIn))
       continue;
     return createStringError(llvm::errc::invalid_argument,
                              "section '%s' cannot be removed: (%s+0x%" PRIx64
@@ -868,7 +869,8 @@ static void writeRel(const RelRange &Relocations, T *Buf) {
   for (const auto &Reloc : Relocations) {
     Buf->r_offset = Reloc.Offset;
     setAddend(*Buf, Reloc.Addend);
-    Buf->setSymbolAndType(Reloc.RelocSymbol->Index, Reloc.Type, false);
+    Buf->setSymbolAndType(Reloc.RelocSymbol ? Reloc.RelocSymbol->Index : 0,
+                          Reloc.Type, false);
     ++Buf;
   }
 }
@@ -893,7 +895,7 @@ void RelocationSection::accept(MutableSectionVisitor &Visitor) {
 Error RelocationSection::removeSymbols(
     function_ref<bool(const Symbol &)> ToRemove) {
   for (const Relocation &Reloc : Relocations)
-    if (ToRemove(*Reloc.RelocSymbol))
+    if (Reloc.RelocSymbol && ToRemove(*Reloc.RelocSymbol))
       return createStringError(
           llvm::errc::invalid_argument,
           "not stripping symbol '%s' because it is named in a relocation",
@@ -903,7 +905,8 @@ Error RelocationSection::removeSymbols(
 
 void RelocationSection::markSymbols() {
   for (const Relocation &Reloc : Relocations)
-    Reloc.RelocSymbol->Referenced = true;
+    if (Reloc.RelocSymbol)
+      Reloc.RelocSymbol->Referenced = true;
 }
 
 void RelocationSection::replaceSectionReferences(
@@ -1418,7 +1421,15 @@ static void initRelocations(RelocationSection *Relocs,
     ToAdd.Offset = Rel.r_offset;
     getAddend(ToAdd.Addend, Rel);
     ToAdd.Type = Rel.getType(false);
-    ToAdd.RelocSymbol = SymbolTable->getSymbolByIndex(Rel.getSymbol(false));
+
+    if (uint32_t Sym = Rel.getSymbol(false)) {
+      if (!SymbolTable)
+        error("'" + Relocs->Name +
+              "': relocation references symbol with index " + Twine(Sym) +
+              ", but there is no symbol table");
+      ToAdd.RelocSymbol = SymbolTable->getSymbolByIndex(Sym);
+    }
+
     Relocs->addRelocation(ToAdd);
   }
 }
