[llvm-readobj] - Don't crash when dumping invalid dynamic relocation.

[grimar]

Summary

Currently when we dump dynamic relocation with use of
DT_RELA/DT_RELASZ/DT_RELAENT tags, we crash when a symbol index
is larger than the number of dynamic symbols or
when there is no dynamic symbol table.

This patch adds test cases and fixes the issues.
It is rebased on top of D73484.

[grimar]

• Added a new line at EOF to the test.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test

383–384

Do we have a test case somewhere that shows that relocations without symbols (e.g. R_X86_64_RELATIVE) are handled fine without warning?

428

when the symbol index...

435

larger or equal to -> greater than or equal to

(that's the more common way of saying '>=')

llvm/test/tools/llvm-readobj/ELF/dynamic-reloc-no-section-headers.test

2

May be worth splitting this line into two.

llvm/tools/llvm-readobj/ELFDumper.cpp

4049

don't -> not

[grimar]

• Addressed review comments.

llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test

383–384

Yes, I think the test case is:
https://github.com/llvm/llvm-project/blob/master/llvm/test/tools/llvm-readobj/ELF/reloc-zero-name-or-value.test#L22
We have the R_X86_64_NONE dynamic relocation with no symbol and no warnings.

[jhenderson]

LGTM.

llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test

Entries:
       - Tag:   DT_NULL
         Value: 0x0
 DynamicSymbols: []
 ProgramHeaders:
   - Type:  PT_LOAD
     Sections:
       - Section: .rela.plt
       - Section: .dynamic
+
+## Show we print a warning when dumping dynamic relocations if there is no dynamic symbol table.
+# RUN: yaml2obj --docnum=11 %s -o %t11
+# RUN: llvm-readobj --dyn-relocations %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=LLVM-NO-DYNSYM
+# RUN: llvm-readelf --dyn-relocations %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=GNU-NO-DYNSYM
+
+# LLVM-NO-DYNSYM:      Dynamic Relocations {
+# LLVM-NO-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 0: no dynamic symbol table found
+# LLVM-NO-DYNSYM-NEXT:   0x0 R_X86_64_NONE <corrupt> 0x0
+# LLVM-NO-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 1: no dynamic symbol table found
+# LLVM-NO-DYNSYM-NEXT:   0x0 R_X86_64_NONE <corrupt> 0x0
+# LLVM-NO-DYNSYM-NEXT: }
+
+# GNU-NO-DYNSYM:      'RELA' relocation section at offset 0x78 contains 48 bytes:
+# GNU-NO-DYNSYM-NEXT: Offset            Info             Type             Symbol's Value   Symbol's Name + Addend
+# GNU-NO-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 0: no dynamic symbol table found
+# GNU-NO-DYNSYM-NEXT: 0000000000000000  0000000000000000 R_X86_64_NONE                     <corrupt> + 0
+# GNU-NO-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 1: no dynamic symbol table found
+# GNU-NO-DYNSYM-NEXT: 0000000000000000  0000000100000000 R_X86_64_NONE                     <corrupt> + 0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_RELA
+    Relocations:
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+        Symbol: 0x1
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELA
+        Value: 0x0
+      - Tag:   DT_RELASZ
+        Value: 0x30
+      - Tag:   DT_RELAENT
+        Value: 0x18
+      - Tag:   DT_NULL
+        Value: 0x0
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+
+## Show we print a warning when the symbol index of a dynamic relocation is too
+## large (goes past the end of the dynamic symbol table).
+# RUN: yaml2obj --docnum=12 %s -o %t12
+# RUN: llvm-readobj --dyn-relocations %t12 2>&1 | FileCheck %s -DFILE=%t12 --check-prefix=LLVM-INVALID-DYNSYM
+# RUN: llvm-readelf --dyn-relocations %t12 2>&1 | FileCheck %s -DFILE=%t12 --check-prefix=GNU-INVALID-DYNSYM
+
+# LLVM-INVALID-DYNSYM:      Dynamic Relocations {
+# LLVM-INVALID-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 2: index is greater than or equal to the number of dynamic symbols (2)
+# LLVM-INVALID-DYNSYM-NEXT:   0x0 R_X86_64_NONE <corrupt> 0x0
+# LLVM-INVALID-DYNSYM-NEXT: }
+
+# GNU-INVALID-DYNSYM:      'RELA' relocation section at offset 0x78 contains 24 bytes:
+# GNU-INVALID-DYNSYM-NEXT: Offset            Info             Type             Symbol's Value   Symbol's Name + Addend
+# GNU-INVALID-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 2: index is greater than or equal to the number of dynamic symbols (2)
+# GNU-INVALID-DYNSYM-NEXT: 0000000000000000  0000000200000000 R_X86_64_NONE                     <corrupt> + 0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_RELA
+    Relocations:
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+        Symbol: 0x2
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELA
+        Value: 0x0
+      - Tag:   DT_RELASZ
+        Value: 0x18
+      - Tag:   DT_RELAENT
+        Value: 0x18
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols:
+  - Name: foo
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic

llvm/test/tools/llvm-readobj/ELF/dynamic-reloc-no-section-headers.test

+## Test that we are able to print dynamic relocations with --dyn-relocations
+## even when there are no section headers.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --dyn-relocations %t 2>&1 \
+# RUN:  | FileCheck %s -DFILE=%t --implicit-check-not="warning:" --check-prefix=LLVM-NO-SEC-TABLE
+# RUN: llvm-readelf --dyn-relocations %t 2>&1 \
+# RUN:  | FileCheck %s -DFILE=%t --implicit-check-not="warning:" --check-prefix=GNU-NO-SEC-TABLE
+
+# LLVM-NO-SEC-TABLE:      Dynamic Relocations {
+# LLVM-NO-SEC-TABLE-NEXT:   0x0 R_X86_64_NONE foo 0x0
+# LLVM-NO-SEC-TABLE-NEXT: }
+
+# GNU-NO-SEC-TABLE:      'RELA' relocation section at offset 0xb0 contains 24 bytes:
+# GNU-NO-SEC-TABLE-NEXT: Offset            Info             Type             Symbol's Value   Symbol's Name + Addend
+# GNU-NO-SEC-TABLE-NEXT: 0000000000000000  0000000100000000 R_X86_64_NONE    0000000000000000 foo + 0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+## We simulate no section header table by
+## overriding the ELF header properties.
+  SHOff:   0x0
+  SHNum:   0x0
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_RELA
+    Relocations:
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+        Symbol: 0x1
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELA
+        Value: 0x0
+      - Tag:   DT_RELASZ
+        Value: 0x18
+      - Tag:   DT_RELAENT
+        Value: 0x18
+## Offset of .dynsym is 0x138. Offset of PT_LOAD is 0xb0.
+## 0x138 - 0xb0 == 0x88
+      - Tag:   DT_SYMTAB
+        Value: 0x88
+      - Tag:   DT_STRSZ
+        Value: 0x5
+## Offset of .dynstr is 0x19f. Offset of PT_LOAD is 0xb0.
+## 0x19f - 0xb0 == 0xef
+      - Tag:   DT_STRTAB
+        Value: 0xef
+      - Tag:   DT_NULL
+        Value: 0x0
+  - Name:    .dynsym
+    Type:    SHT_DYNSYM
+DynamicSymbols:
+  - Name: foo
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+        Section: .dynsym
+        Section: .dynstr
+  - Type:  PT_DYNAMIC
+    Sections:
+      - Section: .dynamic

llvm/tools/llvm-readobj/ELFDumper.cpp

template <class ELFT> struct RelSymbol {
   std::string Name;
 };
 
 template <class ELFT>
 RelSymbol<ELFT> getSymbolForReloc(const ELFFile<ELFT> *Obj, StringRef FileName,
                                   const ELFDumper<ELFT> *Dumper,
                                   const typename ELFT::Rela &Reloc) {
   uint32_t SymIndex = Reloc.getSymbol(Obj->isMips64EL());
-  const typename ELFT::Sym *Sym = Dumper->dynamic_symbols().begin() + SymIndex;
-  Expected<StringRef> ErrOrName = Sym->getName(Dumper->getDynamicStringTable());
-
-  std::string Name;
-  if (ErrOrName) {
-    Name = maybeDemangle(*ErrOrName);
-  } else {
+  auto WarnAndReturn = [&](const typename ELFT::Sym *Sym,
+                           const Twine &Reason) -> RelSymbol<ELFT> {
     reportWarning(
         createError("unable to get name of the dynamic symbol with index " +
-                    Twine(SymIndex) + ": " + toString(ErrOrName.takeError())),
+                    Twine(SymIndex) + ": " + Reason),
         FileName);
-    Name = "<corrupt>";
-  }
+    return {Sym, "<corrupt>"};
+  };
+
+  ArrayRef<typename ELFT::Sym> Symbols = Dumper->dynamic_symbols();
+  const typename ELFT::Sym *FirstSym = Symbols.begin();
+  if (!FirstSym)
+    return WarnAndReturn(nullptr, "no dynamic symbol table found");
+
+  // We might have an object without a section header. In this case the size of
+  // Symbols is zero, because there is no way to know the size of the dynamic
+  // table. We should allow this case and not print a warning.
+  if (!Symbols.empty() && SymIndex >= Symbols.size())
+    return WarnAndReturn(
+        nullptr,
+        "index is greater than or equal to the number of dynamic symbols (" +
+            Twine(Symbols.size()) + ")");
+
+  const typename ELFT::Sym *Sym = FirstSym + SymIndex;
+  Expected<StringRef> ErrOrName = Sym->getName(Dumper->getDynamicStringTable());
+  if (!ErrOrName)
+    return WarnAndReturn(Sym, toString(ErrOrName.takeError()));
 
-  return {Sym, std::move(Name)};
+  return {Sym, maybeDemangle(*ErrOrName)};
 }
 } // namespace
 
 template <class ELFT>
 void GNUStyle<ELFT>::printDynamicRelocation(const ELFO *Obj, Elf_Rela R,
                                             bool IsRela) {
   RelSymbol<ELFT> S = getSymbolForReloc(Obj, this->FileName, this->dumper(), R);
   printRelocation(Obj, S.Sym, S.Name, R, IsRela);