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

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 24 00:49:28 PST 2020


Author: Georgii Rymar
Date: 2020-11-24T11:49:00+03:00
New Revision: 245052ac3080681915c4da59e871d43ea583debb

URL: https://github.com/llvm/llvm-project/commit/245052ac3080681915c4da59e871d43ea583debb
DIFF: https://github.com/llvm/llvm-project/commit/245052ac3080681915c4da59e871d43ea583debb.diff

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

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));
}
```

Differential revision: https://reviews.llvm.org/D91624

Added: 
    

Modified: 
    llvm/test/tools/llvm-readobj/ELF/stack-sizes.test
    llvm/tools/llvm-readobj/ELFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-readobj/ELF/stack-sizes.test b/llvm/test/tools/llvm-readobj/ELF/stack-sizes.test
index c0fa2c802934..01bdb7231567 100644
--- a/llvm/test/tools/llvm-readobj/ELF/stack-sizes.test
+++ b/llvm/test/tools/llvm-readobj/ELF/stack-sizes.test
@@ -390,29 +390,42 @@ Symbols:
     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
@@ -424,12 +437,13 @@ FileHeader:
 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
@@ -437,6 +451,10 @@ Sections:
     - 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.

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 34be3aa0a350..cdfbd1f20b2b 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -794,8 +794,9 @@ template <typename ELFT> class DumpStyle {
                               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;
@@ -5877,44 +5878,47 @@ void GNUStyle<ELFT>::printStackSizeEntry(uint64_t Size, StringRef FuncName) {
 }
 
 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,
@@ -5924,8 +5928,9 @@ void DumpStyle<ELFT>::printStackSize(RelocationRef Reloc,
     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);
 }
@@ -6031,21 +6036,26 @@ void DumpStyle<ELFT>::printRelocatableStackSizes(
         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())) {
-        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);
-    }
+    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(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");
+        });
   }
 }
 


        


More information about the llvm-commits mailing list