[llvm-readelf/obj] - Improve the error reporting in printStackSize().

[grimar]

Summary

This stops using RelocationRef API in the printStackSize method
and starts using the "regular" API that is used in almost all other places
in ELFDumper.cpp.

This is not only makes the code to be more consistent, but helps to diagnose
issues better, because the ELFObjectFile API, which is used
currently to implement stack sized dumping sometimes has a behavior
that just doesn't work well for broken inputs.

E.g see how it gets the symbol_end iterator. It will just not work
well for a case when the sh_size is broken.

template <class ELFT>
basic_symbol_iterator ELFObjectFile<ELFT>::symbol_end() const {
...
  DataRefImpl Sym = toDRI(SymTab, SymTab->sh_size / sizeof(Elf_Sym));
  return basic_symbol_iterator(SymbolRef(Sym, this));
}

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/stack-sizes.test

397–410

Would --implicit-check-not=warning: be worthwhile?

llvm/tools/llvm-readobj/ELFDumper.cpp

5856–5857

I think you can delete this consumeError since you're calling takeError above, right?

[grimar]

• Addressed review comments.

llvm/test/tools/llvm-readobj/ELF/stack-sizes.test

397–410

Done.

llvm/tools/llvm-readobj/ELFDumper.cpp

5856–5857

Right. Thanks!

[jhenderson]

Looks good. Might be worth giving @MaskRay a chance to comment.

llvm/test/tools/llvm-readobj/ELF/stack-sizes.test

 Symbols:
   - Name:    foo
     Section: .text
     Value:   0x10
     Type:    STT_FUNC
     Binding: STB_GLOBAL
 
 ## Check that we report a warning when a relocation symbol does not belong to a
-## valid section. We expect a stack size entry with an unknown symbol in the 
-## output.
+## valid section or when it has an invalid index. We expect a stack size entry
+## with an unknown symbol in the output.
 
 # RUN: yaml2obj --docnum=7 %s -o %t07
-# RUN: llvm-readelf --stack-sizes %t07 2> %t07-gnu.err | FileCheck %s --check-prefix=BADSECTION-OUT-GNU
-# RUN: FileCheck %s < %t07-gnu.err --check-prefix=BADSECTION-ERR -DFILE=%t07
-# RUN: llvm-readobj --stack-sizes %t07 2> %t07-llvm.err | FileCheck %s --check-prefix=BADSECTION-OUT-LLVM
-# RUN: FileCheck %s < %t07-llvm.err --check-prefix=BADSECTION-ERR -DFILE=%t07
-
-# RUN: llvm-readelf --stack-sizes --demangle %t07 2>&1 | FileCheck %s --check-prefix=BADSECTION-DEMANGLE-ERR -DFILE=%t07
-# RUN: llvm-readobj --stack-sizes --demangle %t07 2>&1 | FileCheck %s --check-prefix=BADSECTION-DEMANGLE-ERR -DFILE=%t07
-
-# BADSECTION-OUT-GNU: Size Function
-# BADSECTION-OUT-GNU:    8 ?
+# RUN: llvm-readelf --stack-sizes %t07 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t07 --check-prefix=BADSECTION-OUT-GNU --implicit-check-not=warning:
+# RUN: llvm-readobj --stack-sizes %t07 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t07 --check-prefix=BADSECTION-OUT-LLVM --implicit-check-not=warning:
+
+# BADSECTION-OUT-GNU:      Stack Sizes:
+# BADSECTION-OUT-GNU-NEXT:          Size     Function
+# BADSECTION-OUT-GNU-NEXT: warning: '[[FILE]]': cannot identify the section for relocation symbol '_Z3foof': invalid section index: 10
+# BADSECTION-OUT-GNU-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry
+# BADSECTION-OUT-GNU-NEXT:             8     ?
+# BADSECTION-OUT-GNU-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 2 in SHT_RELA section with index 3: unable to access section [index 4] data at 0x1880: offset goes past the end of file
+# BADSECTION-OUT-GNU-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry
+# BADSECTION-OUT-GNU-NEXT:             22    ?
 
 # BADSECTION-OUT-LLVM:      StackSizes [
+# BADSECTION-OUT-LLVM-NEXT: warning: '[[FILE]]': cannot identify the section for relocation symbol '_Z3foof': invalid section index: 10
+# BADSECTION-OUT-LLVM-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry
 # BADSECTION-OUT-LLVM-NEXT:   Entry {
 # BADSECTION-OUT-LLVM-NEXT:     Function: ?
 # BADSECTION-OUT-LLVM-NEXT:     Size: 0x8
 # BADSECTION-OUT-LLVM-NEXT:   }
+# BADSECTION-OUT-LLVM-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 2 in SHT_RELA section with index 3: unable to access section [index 4] data at 0x1880: offset goes past the end of file
+# BADSECTION-OUT-LLVM-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry
+# BADSECTION-OUT-LLVM-NEXT:   Entry {
+# BADSECTION-OUT-LLVM-NEXT:     Function: ?
+# BADSECTION-OUT-LLVM-NEXT:     Size: 0x16
+# BADSECTION-OUT-LLVM-NEXT:   }
 # BADSECTION-OUT-LLVM-NEXT: ]
 
-# BADSECTION-ERR: warning: '[[FILE]]': cannot identify the section for relocation symbol '_Z3foof'
+# RUN: llvm-readelf --stack-sizes --demangle %t07 2>&1 | FileCheck %s --check-prefix=BADSECTION-DEMANGLE-ERR -DFILE=%t07
+# RUN: llvm-readobj --stack-sizes --demangle %t07 2>&1 | FileCheck %s --check-prefix=BADSECTION-DEMANGLE-ERR -DFILE=%t07
+
 # BADSECTION-DEMANGLE-ERR: warning: '[[FILE]]': cannot identify the section for relocation symbol 'foo(float)'
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name:    .text
     Type:    SHT_PROGBITS
-    Size:    8
+    Size:    16
   - Name:    .stack_sizes
     Type:    SHT_PROGBITS
     Link:    .text
     Entries:
       - Size: 0x8
+      - Size: 0x16
   - Name:    .rela.stack_sizes
     Type:    SHT_RELA
     Info:    .stack_sizes
     Relocations:
     - Offset: 0
       Symbol: _Z3foof
       Type:   R_X86_64_64
+    - Offset: 9
+## An invalid symbol index.
+      Symbol: 0xff
+      Type:   R_X86_64_64
 Symbols:
   - Name:    _Z3foof
 ## An invalid section index.
     Index:   10
     Type:    STT_FUNC
     Binding: STB_GLOBAL
 
 ## Check that we report a warning when a stack sizes section does not come with

llvm/tools/llvm-readobj/ELFDumper.cpp

public:
   virtual void printELFLinkerOptions() = 0;
   virtual void printStackSizes() = 0;
   void printNonRelocatableStackSizes(std::function<void()> PrintHeader);
   void printRelocatableStackSizes(std::function<void()> PrintHeader);
   void printFunctionStackSize(uint64_t SymValue,
                               Optional<const Elf_Shdr *> FunctionSec,
                               const Elf_Shdr &StackSizeSec, DataExtractor Data,
                               uint64_t *Offset);
-  void printStackSize(RelocationRef Rel, const Elf_Shdr *FunctionSec,
-                      const Elf_Shdr &StackSizeSec,
+  void printStackSize(const Relocation<ELFT> &R, const Elf_Shdr &RelocSec,
+                      unsigned Ndx, const Elf_Shdr *SymTab,
+                      const Elf_Shdr *FunctionSec, const Elf_Shdr &StackSizeSec,
                       const RelocationResolver &Resolver, DataExtractor Data);
   virtual void printStackSizeEntry(uint64_t Size, StringRef FuncName) = 0;
   virtual void printMipsGOT(const MipsGOTParser<ELFT> &Parser) = 0;
   virtual void printMipsPLT(const MipsGOTParser<ELFT> &Parser) = 0;
   virtual void printMipsABIFlags() = 0;
   const ELFDumper<ELFT> &dumper() const { return Dumper; }
 
 protected:

 void GNUStyle<ELFT>::printStackSizeEntry(uint64_t Size, StringRef FuncName) {
   OS.PadToColumn(2);
   OS << format_decimal(Size, 11);
   OS.PadToColumn(18);
   OS << FuncName << "\n";
 }
 
 template <class ELFT>
-void DumpStyle<ELFT>::printStackSize(RelocationRef Reloc,
+void DumpStyle<ELFT>::printStackSize(const Relocation<ELFT> &R,
+                                     const Elf_Shdr &RelocSec, unsigned Ndx,
+                                     const Elf_Shdr *SymTab,
                                      const Elf_Shdr *FunctionSec,
                                      const Elf_Shdr &StackSizeSec,
                                      const RelocationResolver &Resolver,
                                      DataExtractor Data) {
   // This function ignores potentially erroneous input, unless it is directly
   // related to stack size reporting.
-  object::symbol_iterator RelocSym = Reloc.getSymbol();
+  const Elf_Sym *Sym = nullptr;
+  Expected<RelSymbol<ELFT>> TargetOrErr =
+      this->dumper().getRelocationTarget(R, SymTab);
+  if (!TargetOrErr)
+    reportUniqueWarning(
+        createError("unable to get the target of relocation with index " +
+                    Twine(Ndx) + " in " + describe(Obj, RelocSec) + ": " +
+                    toString(TargetOrErr.takeError())));
+  else
+    Sym = TargetOrErr->Sym;
+
   uint64_t RelocSymValue = 0;
-  if (RelocSym != ElfObj.symbol_end()) {
-    // Ensure that the relocation symbol is in the function section, i.e. the
-    // section where the functions whose stack sizes we are reporting are
-    // located.
-    auto SectionOrErr = RelocSym->getSection();
+  if (Sym) {
+    Expected<const Elf_Shdr *> SectionOrErr =
+        this->Obj.getSection(*Sym, SymTab, this->dumper().getShndxTable());
     if (!SectionOrErr) {
-      reportWarning(
-          createError("cannot identify the section for relocation symbol '" +
-                      getSymbolName(*RelocSym) + "'"),
-          FileName);
-      consumeError(SectionOrErr.takeError());
-    } else if (*SectionOrErr != ElfObj.toSectionRef(FunctionSec)) {
-      reportWarning(createError("relocation symbol '" +
-                                getSymbolName(*RelocSym) +
-                                "' is not in the expected section"),
-                    FileName);
+      reportUniqueWarning(createError(
+          "cannot identify the section for relocation symbol '" +
+          (*TargetOrErr).Name + "': " + toString(SectionOrErr.takeError())));
+    } else if (*SectionOrErr != FunctionSec) {
+      reportUniqueWarning(createError("relocation symbol '" +
+                                      (*TargetOrErr).Name +
+                                      "' is not in the expected section"));
       // Pretend that the symbol is in the correct section and report its
       // stack size anyway.
-      FunctionSec = ElfObj.getSection((*SectionOrErr)->getRawDataRefImpl());
+      FunctionSec = *SectionOrErr;
     }
 
-    Expected<uint64_t> RelocSymValueOrErr = RelocSym->getValue();
-    if (RelocSymValueOrErr)
-      RelocSymValue = *RelocSymValueOrErr;
-    else
-      consumeError(RelocSymValueOrErr.takeError());
+    RelocSymValue = Sym->st_value;
   }
 
-  uint64_t Offset = Reloc.getOffset();
+  uint64_t Offset = R.Offset;
   if (!Data.isValidOffsetForDataOfSize(Offset, sizeof(Elf_Addr) + 1)) {
     reportUniqueWarning(createStringError(
         object_error::parse_failed,
         "found invalid relocation offset (0x" + Twine::utohexstr(Offset) +
             ") into " + describe(Obj, StackSizeSec) +
             " while trying to extract a stack size entry"));
     return;
   }
 
-  uint64_t Addend = Data.getAddress(&Offset);
-  uint64_t SymValue = resolveRelocation(Resolver, Reloc, RelocSymValue, Addend);
+  uint64_t SymValue =
+      Resolver(R.Type, Offset, RelocSymValue, Data.getAddress(&Offset),
+               R.Addend.getValueOr(0));
   this->printFunctionStackSize(SymValue, FunctionSec, StackSizeSec, Data,
                                &Offset);
 }
 
 template <class ELFT>
 void DumpStyle<ELFT>::printNonRelocatableStackSizes(
     std::function<void()> PrintHeader) {
   // This function ignores potentially erroneous input, unless it is directly
for (const auto &StackSizeMapEntry : StackSizeRelocMap) {
 
     SupportsRelocation IsSupportedFn;
     RelocationResolver Resolver;
     std::tie(IsSupportedFn, Resolver) = getRelocationResolver(ElfObj);
     ArrayRef<uint8_t> Contents =
         unwrapOrError(this->FileName, Obj.getSectionContents(*StackSizesELFSec));
     DataExtractor Data(Contents, Obj.isLE(), sizeof(Elf_Addr));
 
-    size_t I = 0;
-    for (const RelocationRef &Reloc :
-         ElfObj.toSectionRef(RelocSec).relocations()) {
-      ++I;
-      if (!IsSupportedFn || !IsSupportedFn(Reloc.getType())) {
+    forEachRelocationDo(
+        *RelocSec, /*RawRelr=*/false,
+        [&](const Relocation<ELFT> &R, unsigned Ndx, const Elf_Shdr &Sec,
+            const Elf_Shdr *SymTab) {
+          if (!IsSupportedFn || !IsSupportedFn(R.Type)) {
             reportUniqueWarning(createStringError(
                 object_error::parse_failed,
                 describe(Obj, *RelocSec) +
-                " contains an unsupported relocation with index " + Twine(I) +
-                ": " + Obj.getRelocationTypeName(Reloc.getType())));
-        continue;
-      }
-      this->printStackSize(Reloc, FunctionSec, *StackSizesELFSec, Resolver,
-                           Data);
+                    " contains an unsupported relocation with index " +
+                    Twine(Ndx) + ": " + Obj.getRelocationTypeName(R.Type)));
+            return;
           }
+
+          this->printStackSize(R, *RelocSec, Ndx, SymTab, FunctionSec,
+                               *StackSizesELFSec, Resolver, Data);
+        },
+        [](const Elf_Relr &) {
+          llvm_unreachable("can't get here, because we only support "
+                           "SHT_REL/SHT_RELA sections");
+        });
   }
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printStackSizes() {
   bool HeaderHasBeenPrinted = false;
   auto PrintHeader = [&]() {
     if (HeaderHasBeenPrinted)