[llvm-readobj/elf] - Refine the implementation of "printFunctionStackSize".

[grimar]

Summary

This rewrites the logic to get rid of "ELFSymbolRef" API where possible.
This allowed to handle possible errors better, improve warnings reported and add new ones.
Also 'reportWarning' was replaced with 'reportUniqueWarning'

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

5804–5805

You've added several new warnings with this refactor. Does this comment still apply?

5834–5839

Should this not just be cantFail?

5867–5868

Are you planning on further improving these messages in another change? The offset would be useful here, for example.

(Suggested fix could be deferred too, if it causes too many test changes)

[grimar]

• Addressed review comments.

llvm/tools/llvm-readobj/ELFDumper.cpp

5804–5805

Probably we can remove it.

5834–5839

I am not sure.

cantFail looks like we give a guarantee for this condition to be always true. But this is not our intention.
This error currently can't be triggered because Obj.getSection API
is called somewhere inside ElfObj.toSymbolRef(SymTab, Index).getAddress(). But we don't give a guarantee here as
things are probably too brittle, unobvious and might change easily.

5867–5868

Are you planning on further improving these messages in another change? The offset would be useful here, for example.

I'll do it.

[jhenderson]

LGTM.

llvm/include/llvm/Object/ELFObjectFile.h

public:
   using Elf_Rel = typename ELFT::Rel;
   using Elf_Rela = typename ELFT::Rela;
   using Elf_Dyn = typename ELFT::Dyn;
 
   SectionRef toSectionRef(const Elf_Shdr *Sec) const {
     return SectionRef(toDRI(Sec), this);
   }
 
+  ELFSymbolRef toSymbolRef(const Elf_Shdr *SymTable, unsigned SymbolNum) const {
+    return ELFSymbolRef({toDRI(SymTable, SymbolNum), this});
+  }
+
   bool IsContentValid() const { return ContentValid; }
 
 private:
   ELFObjectFile(MemoryBufferRef Object, ELFFile<ELFT> EF,
                 const Elf_Shdr *DotDynSymSec, const Elf_Shdr *DotSymtabSec,
                 const Elf_Shdr *DotSymtabShndxSec);
 
   bool ContentValid = false;

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

## A section relative reference.
         Type:   R_X86_64_64
   - Name:   '.rela.stack_sizes (1)'
     Type:   SHT_RELA
     Info:   '.stack_sizes (1)'
     Relocations:
       - Offset: 0
         Symbol: separate_text_section_baz
         Type:   R_X86_64_64
+  - Name:     .symtab
+    Type:     SHT_SYMTAB
+    ShOffset: [[SYMTABOFFSET=<none>]]
 Symbols:
   - Name:    separate_text_section_baz
-    Section: .text.baz
+    Section: [[SEC1=.text.baz]]
     Type:    STT_FUNC
+    Index:   [[SEC1INDEX=<none>]]
   - Name:    .text
     Section: .text
     Type:    STT_SECTION
   - Name:    referenced_by_symbol_foo
     Section: .text
     Type:    STT_FUNC
     Binding: STB_GLOBAL
   - Name:    referenced_via_section_bar
     Section: .text
     Value:   0x10
     Type:    STT_FUNC
     Binding: STB_GLOBAL
 
+## Check that we report a warning when we are unable to read
+## the symbol table when dumping stack sizes.
+
+# RUN: yaml2obj --docnum=1 %s -DSYMTABOFFSET=0xffffeeee -o %t01.broken.symtab
+# RUN: llvm-readelf --stack-sizes %t01.broken.symtab 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t01.broken.symtab --check-prefix=SYMTAB-GNU --implicit-check-not=warning:
+# RUN: llvm-readobj --stack-sizes %t01.broken.symtab 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t01.broken.symtab --check-prefix=SYMTAB-LLVM --implicit-check-not=warning:
+
+# SYMTAB-GNU:      Stack Sizes:
+# SYMTAB-GNU-NEXT:          Size     Function
+# SYMTAB-GNU-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 1 in SHT_RELA section with index 5: unable to access section [index 7] data at 0xffffef36: offset goes past the end of file
+# SYMTAB-GNU-NEXT: warning: '[[FILE]]': unable to read the symbol table: section [index 7] has a sh_offset (0xffffeeee) + sh_size (0x78) that is greater than the file size (0x450)
+# SYMTAB-GNU-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 3
+# SYMTAB-GNU-NEXT:            16     ?
+# SYMTAB-GNU-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 2 in SHT_RELA section with index 5: unable to access section [index 7] data at 0xffffef1e: offset goes past the end of file
+# SYMTAB-GNU-NEXT:            32     ?
+# SYMTAB-GNU-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 1 in SHT_RELA section with index 6: unable to access section [index 7] data at 0xffffef06: offset goes past the end of file
+# SYMTAB-GNU-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 4
+# SYMTAB-GNU-NEXT:             8     ?
+
+# SYMTAB-LLVM:      StackSizes [
+# SYMTAB-LLVM-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 1 in SHT_RELA section with index 5: unable to access section [index 7] data at 0xffffef36: offset goes past the end of file
+# SYMTAB-LLVM-NEXT: warning: '[[FILE]]': unable to read the symbol table: section [index 7] has a sh_offset (0xffffeeee) + sh_size (0x78) that is greater than the file size (0x450)
+# SYMTAB-LLVM-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 3
+# SYMTAB-LLVM-NEXT:   Entry {
+# SYMTAB-LLVM-NEXT:     Function: ?
+# SYMTAB-LLVM-NEXT:     Size: 0x10
+# SYMTAB-LLVM-NEXT:   }
+# SYMTAB-LLVM-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 2 in SHT_RELA section with index 5: unable to access section [index 7] data at 0xffffef1e: offset goes past the end of file
+# SYMTAB-LLVM-NEXT:   Entry {
+# SYMTAB-LLVM-NEXT:     Function: ?
+# SYMTAB-LLVM-NEXT:     Size: 0x20
+# SYMTAB-LLVM-NEXT:   }
+# SYMTAB-LLVM-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 1 in SHT_RELA section with index 6: unable to access section [index 7] data at 0xffffef06: offset goes past the end of file
+# SYMTAB-LLVM-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 4
+# SYMTAB-LLVM-NEXT:   Entry {
+# SYMTAB-LLVM-NEXT:     Function: ?
+# SYMTAB-LLVM-NEXT:     Size: 0x8
+# SYMTAB-LLVM-NEXT:   }
+# SYMTAB-LLVM-NEXT: ]
+
+## In this case we have a function symbol with an invalid section index.
+## Document what we dump.
+
+# RUN: yaml2obj --docnum=1 %s -DSEC1="<none>" -DSEC1INDEX=0xFF -o %t01.broken.sym
+# RUN: llvm-readelf --stack-sizes %t01.broken.sym 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t01.broken.sym --check-prefix=SYM-GNU --implicit-check-not=warning:
+# RUN: llvm-readobj --stack-sizes %t01.broken.sym 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t01.broken.sym --check-prefix=SYM-LLVM --implicit-check-not=warning:
+
+# SYM-GNU:      Stack Sizes:
+# SYM-GNU-NEXT:          Size     Function
+# SYM-GNU-NEXT: warning: '[[FILE]]': unable to get address of symbol 'separate_text_section_baz': invalid section index: 255
+# SYM-GNU-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 3
+# SYM-GNU-NEXT:            16     ?
+# SYM-GNU-NEXT:            32     ?
+# SYM-GNU-NEXT: warning: '[[FILE]]': cannot identify the section for relocation symbol 'separate_text_section_baz': invalid section index: 255
+# SYM-GNU-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 4
+# SYM-GNU-NEXT:             8     ?
+
+# SYM-LLVM:      StackSizes [
+# SYM-LLVM-NEXT: warning: '[[FILE]]': unable to get address of symbol 'separate_text_section_baz': invalid section index: 255
+# SYM-LLVM-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 3
+# SYM-LLVM-NEXT:   Entry {
+# SYM-LLVM-NEXT:     Function: ?
+# SYM-LLVM-NEXT:     Size: 0x10
+# SYM-LLVM-NEXT:   }
+# SYM-LLVM-NEXT:   Entry {
+# SYM-LLVM-NEXT:     Function: ?
+# SYM-LLVM-NEXT:     Size: 0x20
+# SYM-LLVM-NEXT:   }
+# SYM-LLVM-NEXT: warning: '[[FILE]]': cannot identify the section for relocation symbol 'separate_text_section_baz': invalid section index: 255
+# SYM-LLVM-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 4
+# SYM-LLVM-NEXT:   Entry {
+# SYM-LLVM-NEXT:     Function: ?
+# SYM-LLVM-NEXT:     Size: 0x8
+# SYM-LLVM-NEXT:   }
+# SYM-LLVM-NEXT: ]
+
 ## Check that we correctly report the stack sizes in an executable (non-relocatable)
 ## object file. This also shows that the sh_link field is ignored in this situation
 ## without warning.
 
 # RUN: yaml2obj --docnum=2 %s -o %t02
 # RUN: llvm-readelf --stack-sizes %t02 2>&1 \
 # RUN:   | FileCheck %s --check-prefix=EXEC-GNU --strict-whitespace \
 # RUN:                  --match-full-lines --implicit-check-not=warning:

 
 ## Check that we report an invalid stack size, which is represented by a ULEB that
 ## ends in a byte with the high bit set.
 
 # RUN: yaml2obj --docnum=6 %s -o %t06
 # RUN: llvm-readelf --stack-sizes %t06 2>&1 | FileCheck %s --check-prefix=BADSIZE -DFILE=%t06
 # RUN: llvm-readobj --stack-sizes %t06 2>&1 | FileCheck %s --check-prefix=BADSIZE -DFILE=%t06
 
-# BADSIZE: warning: '[[FILE]]': could not extract a valid stack size in SHT_PROGBITS section with index 2
-# BADSIZE: warning: '[[FILE]]': could not extract a valid stack size in SHT_PROGBITS section with index 3
+# BADSIZE: warning: '[[FILE]]': could not extract a valid stack size from SHT_PROGBITS section with index 2
+# BADSIZE: warning: '[[FILE]]': could not extract a valid stack size from SHT_PROGBITS section with index 3
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_EXEC
 Sections:
   - Name:    .text

 # 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: warning: '[[FILE]]': unable to get address of symbol '_Z3foof': invalid section index: 10
+# BADSECTION-OUT-GNU-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 2
 # 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-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 0x18a0: offset goes past the end of file
+# BADSECTION-OUT-GNU-NEXT:            22     ?
+# BADSECTION-OUT-GNU-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 3 in SHT_RELA section with index 3: unable to access section [index 4] data at 0x18a0: offset goes past the end of file
+# BADSECTION-OUT-GNU-NEXT:            36     ?
 
 # 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: warning: '[[FILE]]': unable to get address of symbol '_Z3foof': invalid section index: 10
+# BADSECTION-OUT-LLVM-NEXT: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 2
 # 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: 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 0x18a0: offset goes past the end of file
 # BADSECTION-OUT-LLVM-NEXT:   Entry {
 # BADSECTION-OUT-LLVM-NEXT:     Function: ?
 # BADSECTION-OUT-LLVM-NEXT:     Size: 0x16
 # BADSECTION-OUT-LLVM-NEXT:   }
+# BADSECTION-OUT-LLVM-NEXT: warning: '[[FILE]]': unable to get the target of relocation with index 3 in SHT_RELA section with index 3: unable to access section [index 4] data at 0x18a0: offset goes past the end of file
+# BADSECTION-OUT-LLVM-NEXT:   Entry {
+# BADSECTION-OUT-LLVM-NEXT:     Function: ?
+# BADSECTION-OUT-LLVM-NEXT:     Size: 0x24
+# BADSECTION-OUT-LLVM-NEXT:   }
 # BADSECTION-OUT-LLVM-NEXT: ]
 
 # 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:    16
   - Name:    .stack_sizes
     Type:    SHT_PROGBITS
     Link:    .text
     Entries:
       - Size: 0x8
       - Size: 0x16
+      - Size: 0x24
   - 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
+## One more invalid symbol index with the same symbol value (0xff).
+    - Offset: 0x12
+      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

 
 # NONFUNCTIONSYM-LLVM:      StackSizes [
 # NONFUNCTIONSYM-LLVM-NEXT:   Entry {
 # NONFUNCTIONSYM-LLVM-NEXT:     Function: ?
 # NONFUNCTIONSYM-LLVM-NEXT:     Size: 0x0
 # NONFUNCTIONSYM-LLVM-NEXT:   }
 # NONFUNCTIONSYM-LLVM-NEXT: ]
 
-# NONFUNCTIONSYM-ERR: warning: '[[FILE]]': could not identify function symbol for stack size entry
+# NONFUNCTIONSYM-ERR: warning: '[[FILE]]': could not identify function symbol for stack size entry in SHT_PROGBITS section with index 2
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:

llvm/tools/llvm-readobj/ELFDumper.cpp

auto OnLibEntry = [&](StringRef Lib, uint64_t Offset) {
     SecEntries.push_back(NameOffset{Lib, Offset});
   };
 
   this->printDependentLibsHelper(OnSectionStart, OnLibEntry);
   if (SectionStarted)
     PrintSection();
 }
 
-// Used for printing symbol names in places where possible errors can be
-// ignored.
-static std::string getSymbolName(const ELFSymbolRef &Sym) {
-  Expected<StringRef> NameOrErr = Sym.getName();
-  if (NameOrErr)
-    return maybeDemangle(*NameOrErr);
-  consumeError(NameOrErr.takeError());
-  return "<?>";
-}
-
 template <class ELFT>
 void DumpStyle<ELFT>::printFunctionStackSize(
     uint64_t SymValue, Optional<const Elf_Shdr *> FunctionSec,
     const Elf_Shdr &StackSizeSec, DataExtractor Data, uint64_t *Offset) {
-  // This function ignores potentially erroneous input, unless it is directly
-  // related to stack size reporting.
-  SymbolRef FuncSym;
-  for (const ELFSymbolRef &Symbol : ElfObj.symbols()) {
-    Expected<uint64_t> SymAddrOrErr = Symbol.getAddress();
-    if (!SymAddrOrErr) {
-      consumeError(SymAddrOrErr.takeError());
+  uint32_t FuncSymIndex = 0;
+  if (const Elf_Shdr *SymTab = this->dumper().getDotSymtabSec()) {
+    if (Expected<Elf_Sym_Range> SymsOrError = Obj.symbols(SymTab)) {
+      uint32_t Index = (uint32_t)-1;
+      for (const Elf_Sym &Sym : *SymsOrError) {
+        ++Index;
+
+        if (Sym.st_shndx == ELF::SHN_UNDEF || Sym.getType() != ELF::STT_FUNC)
+          continue;
+
+        if (Expected<uint64_t> SymAddrOrErr =
+                ElfObj.toSymbolRef(SymTab, Index).getAddress()) {
+          if (SymValue != *SymAddrOrErr)
             continue;
+        } else {
+          std::string Name = this->dumper().getStaticSymbolName(Index);
+          reportUniqueWarning("unable to get address of symbol '" + Name +
+                              "': " + toString(SymAddrOrErr.takeError()));
+          break;
         }
-    if (Expected<uint32_t> SymFlags = Symbol.getFlags()) {
-      if (*SymFlags & SymbolRef::SF_Undefined)
+
+        // Check if the symbol is in the right section. FunctionSec == None
+        // means "any section".
+        if (FunctionSec) {
+          if (Expected<const Elf_Shdr *> SecOrErr =
+                  Obj.getSection(Sym, SymTab, this->dumper().getShndxTable())) {
+            if (*FunctionSec != *SecOrErr)
               continue;
-    } else
-      consumeError(SymFlags.takeError());
-    if (Symbol.getELFType() == ELF::STT_FUNC && *SymAddrOrErr == SymValue) {
-      // Check if the symbol is in the right section. FunctionSec == None means
-      // "any section".
-      if (!FunctionSec ||
-          ElfObj.toSectionRef(*FunctionSec).containsSymbol(Symbol)) {
-        FuncSym = Symbol;
+          } else {
+            std::string Name = this->dumper().getStaticSymbolName(Index);
+            // Note: it is impossible to trigger this error currently, it is
+            // untested.
+            reportUniqueWarning("unable to get section of symbol '" + Name +
+                                "': " + toString(SecOrErr.takeError()));
             break;
           }
         }
+
+        FuncSymIndex = Index;
+        break;
+      }
+    } else {
+      reportUniqueWarning("unable to read the symbol table: " +
+                          toString(SymsOrError.takeError()));
+    }
   }
 
   std::string FuncName = "?";
-  // A valid SymbolRef has a non-null object file pointer.
-  if (FuncSym.BasicSymbolRef::getObject())
-    FuncName = getSymbolName(FuncSym);
+  if (!FuncSymIndex)
+    reportUniqueWarning(
+        "could not identify function symbol for stack size entry in " +
+        describe(Obj, StackSizeSec));
   else
-    reportWarning(
-        createError("could not identify function symbol for stack size entry"),
-        FileName);
+    FuncName = this->dumper().getStaticSymbolName(FuncSymIndex);
 
   // Extract the size. The expectation is that Offset is pointing to the right
   // place, i.e. past the function address.
   uint64_t PrevOffset = *Offset;
   uint64_t StackSize = Data.getULEB128(Offset);
   // getULEB128() does not advance Offset if it is not able to extract a valid
   // integer.
   if (*Offset == PrevOffset) {
-    reportWarning(createStringError(object_error::parse_failed,
-                                    "could not extract a valid stack size in " +
-                                        describe(Obj, StackSizeSec)),
-                  FileName);
+    reportUniqueWarning("could not extract a valid stack size from " +
+                        describe(Obj, StackSizeSec));
     return;
   }
-
   printStackSizeEntry(StackSize, FuncName);
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printStackSizeEntry(uint64_t Size, StringRef FuncName) {
   OS.PadToColumn(2);
   OS << format_decimal(Size, 11);
   OS.PadToColumn(18);