[llvm-readobj] Allow syms from all sections to match stack size entries

[jhenderson]

Summary

Prior to this change, for non-relocatable objects llvm-readobj would assume that all symbols that corresponded to a stack size section's entries were in the section specified by the section's sh_link field. However, in the event that there are multiple sections, the sh_link field cannot be respected, as it only indicates one of those sections (LLD sets the value to the first such section but see D76410 for a possible solution).

This patch changes llvm-readobj to ignore the section of symbols in a non-relocatable object (except for undefined symbols which are ignored). One behaviour change is that function symbols that share a value, but in different sections (typically these would appear at the end of a section) will result in the first being picked, even if the second is in the section for the sh_link value. As such symbols will almost never exist (it would require a zero-byte function or a symbol with an incorrect type), I think this is okay.

Fixes https://bugs.llvm.org/show_bug.cgi?id=45228.

[grimar]

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

156

Perhaps say which sh_link?

sh_link of `.stack_sizes` is ...

executables

and DSOs?

163

BTW, does would it be better to print all of candidates?
e.g:

Size   Function
16     other
32     other_end/foo/bar

llvm/tools/llvm-readobj/ELFDumper.cpp

5324–5325

/*FunctionSec=*/None

[jhenderson]

Address review comments.

[MaskRay]

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

101

--implicit-check-not=warning: may be slightly better.

It is unlikely that the absolute path of %s contains warning:

[MaskRay]

However, in the event that there are multiple sections, the sh_link field cannot be respected, as it only indicates one of those sections (LLD sets the value to the first such section but see D76410 for a possible solution).

In the presence of an output section description combining SHF_LINK_ORDER sections linking different output sections, this cannot be respected. A linker script section pattern is "by name" by nature.

[grimar]

LGTM.

(Looking on the code I am a bit unsure why we preferred to consumeError
in functions that print stack sizes, but this consern is unrelated to this patch.)

[jhenderson]

Make --implicit-check-not patterns tighter.

[jhenderson]

(Woops, one comment didn't get added yesterday).

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

163

I don't think like that is a good idea, since there may be people parsing the output and the current version is quite easy to parse.

There's probably a case for marking them on separate lines though, sharing the same value, although that would require trawling the entire symbol table for every entry (O(n^2)) rather than up to the first symbol, which would therefore be somewhat slower.

That would be more general too, and ensure that ICF-ed functions and other aliases are handled nicely. Consequently, I think that's outside the scope of this change.

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

- Name:    referenced_by_symbol_foo
     Binding: STB_GLOBAL
   - Name:    referenced_via_section_bar
     Section: .text
     Value:   0x10
     Type:    STT_FUNC
     Binding: STB_GLOBAL
 
 ## Check that we correctly report the stack sizes in an executable (non-relocatable)
-## object file.
+## 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 \
-# RUN:   | FileCheck %s --check-prefix=EXEC-GNU --strict-whitespace --match-full-lines
-# RUN: llvm-readobj --stack-sizes %t02 | FileCheck %s --check-prefix=EXEC-LLVM
+# 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:
+# RUN: llvm-readobj --stack-sizes %t02 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=EXEC-LLVM --implicit-check-not=warning:
 
 #      EXEC-GNU:         Size     Function
-# EXEC-GNU-NEXT:           16     foo
-# EXEC-GNU-NEXT:           32     bar
+# EXEC-GNU-NEXT:           16     other
+# EXEC-GNU-NEXT:           32     other_end
+# EXEC-GNU-NEXT:           48     bar
 #  EXEC-GNU-NOT:{{.}}
 
 # EXEC-LLVM:      StackSizes [
 # EXEC-LLVM-NEXT:   Entry {
-# EXEC-LLVM-NEXT:     Function: foo
+# EXEC-LLVM-NEXT:     Function: other
 # EXEC-LLVM-NEXT:     Size: 0x10
 # EXEC-LLVM-NEXT:   }
 # EXEC-LLVM-NEXT:   Entry {
-# EXEC-LLVM-NEXT:     Function: bar
+# EXEC-LLVM-NEXT:     Function: other_end
 # EXEC-LLVM-NEXT:     Size: 0x20
 # EXEC-LLVM-NEXT:   }
+# EXEC-LLVM-NEXT:   Entry {
+# EXEC-LLVM-NEXT:     Function: bar
+# EXEC-LLVM-NEXT:     Size: 0x30
+# EXEC-LLVM-NEXT:   }
 # EXEC-LLVM-NEXT: ]
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:
   - Name:    .text
     Type:    SHT_PROGBITS
     Flags:   [SHF_ALLOC]
     Size:    16
+  - Name:    .text2
+    Type:    SHT_PROGBITS
+    Flags:   [SHF_ALLOC]
+    Size:    16
   - Name:    .stack_sizes
     Type:    SHT_PROGBITS
     Entries:
-      - Address: 0x10
+      - Address: 0x0
         Size:    0x10
-      - Address: 0x20
+      - Address: 0x10
         Size:    0x20
-    Link:    .text
+      - Address: 0x20
+        Size:    0x30
+    Link:    .text2
 Symbols:
-  - Name:    foo
+  ## Undefined symbols are ignored.
+  - Name:    undefined
+    Type:    STT_FUNC
+    Binding: STB_GLOBAL
+  ## sh_link of .stack_sizes is ignored for non-reloctable objects.
+  - Name:    other
+    Section: .text
+    Value:   0
+    Type:    STT_FUNC
+    Binding: STB_GLOBAL
+  ## If two symbols have the same value, the first is picked, regardless of
+  ## the sh_link value of the .stack_sizes section.
+  - Name:    other_end
     Section: .text
     Value:   0x10
     Type:    STT_FUNC
     Binding: STB_GLOBAL
+  - Name:    foo
+    Section: .text2
+    Value:   0x10
+    Type:    STT_FUNC
+    Binding: STB_GLOBAL
   - Name:    bar
-    Section: .text
+    Section: .text2
     Value:   0x20
     Type:    STT_FUNC
     Binding: STB_GLOBAL
 
 ## Check that we report an error when we find relocations whose offsets point outside
 ## of the .stack_sizes section.
 
 # RUN: yaml2obj --docnum=3 %s -o %t03
Relocations:
         Type:   R_X86_64_64
 Symbols:
   - Name:    foo
     Section: .text
     Type:    STT_FUNC
     Binding: STB_GLOBAL
 
 ## Check that we warn about a function symbol that is not in the section
-## that is referenced by the stack sizes section's sh_link.
+## that is referenced by the stack sizes section's sh_link, for relocatable
+## output.
 
 # RUN: yaml2obj --docnum=4 %s -o %t04
 # RUN: llvm-readelf --stack-sizes %t04 2> %t04-gnu.err | FileCheck %s --check-prefix=WRONGSECTION-GNU
 # RUN: FileCheck %s < %t04-gnu.err --check-prefix=WRONGSECTION-ERR -DFILE=%t04
 # RUN: llvm-readobj --stack-sizes %t04 2> %t04-llvm.err | FileCheck %s --check-prefix=WRONGSECTION-LLVM
 # RUN: FileCheck %s < %t04-llvm.err --check-prefix=WRONGSECTION-ERR -DFILE=%t04
 
 # RUN: llvm-readelf --stack-sizes --demangle %t04 2>&1 | FileCheck %s --check-prefix=WRONGSECTION-DEMANGLE-ERR -DFILE=%t04

 # MULTIPLE-LLVM-NEXT: ]
 
 #        OBJECT:File: [[FILE2]]
 #       ARCHIVE:File: [[FILE]]({{.*02}})
 
 # MULTIPLE-GNU-EMPTY:
 # MULTIPLE-GNU-NEXT:Stack Sizes:
 # MULTIPLE-GNU-NEXT:         Size     Function
-# MULTIPLE-GNU-NEXT:           16     foo
-# MULTIPLE-GNU-NEXT:           32     bar
+# MULTIPLE-GNU-NEXT:           16     other
+# MULTIPLE-GNU-NEXT:           32     other_end
+# MULTIPLE-GNU-NEXT:           48     bar
 
 # MULTIPLE-LLVM:      StackSizes [
 # MULTIPLE-LLVM-NEXT:   Entry {
-# MULTIPLE-LLVM-NEXT:     Function: foo
+# MULTIPLE-LLVM-NEXT:     Function: other
 # MULTIPLE-LLVM-NEXT:     Size: 0x10
 # MULTIPLE-LLVM-NEXT:   }
 # MULTIPLE-LLVM-NEXT:   Entry {
-# MULTIPLE-LLVM-NEXT:     Function: bar
+# MULTIPLE-LLVM-NEXT:     Function: other_end
 # MULTIPLE-LLVM-NEXT:     Size: 0x20
 # MULTIPLE-LLVM-NEXT:   }
+# MULTIPLE-LLVM-NEXT:   Entry {
+# MULTIPLE-LLVM-NEXT:     Function: bar
+# MULTIPLE-LLVM-NEXT:     Size: 0x30
+# MULTIPLE-LLVM-NEXT:   }
 # MULTIPLE-LLVM-NEXT: ]
 
 ## Check that we do not consider symbols that are not function symbols, even though
 ## a relocation references them. 
 
 # RUN: yaml2obj --docnum=9 %s -o %t14
 # RUN: llvm-readelf --stack-sizes %t14 2> %t14-gnu.err | FileCheck %s --check-prefix=NONFUNCTIONSYM-GNU
 # RUN: FileCheck %s < %t14-gnu.err --check-prefix=NONFUNCTIONSYM-ERR -DFILE=%t14

llvm/tools/llvm-readobj/ELFDumper.cpp

public:
   virtual void printNotes(const ELFFile<ELFT> *Obj) = 0;
   virtual void printELFLinkerOptions(const ELFFile<ELFT> *Obj) = 0;
   virtual void printStackSizes(const ELFObjectFile<ELFT> *Obj) = 0;
   void printNonRelocatableStackSizes(const ELFObjectFile<ELFT> *Obj,
                                      std::function<void()> PrintHeader);
   void printRelocatableStackSizes(const ELFObjectFile<ELFT> *Obj,
                                   std::function<void()> PrintHeader);
   void printFunctionStackSize(const ELFObjectFile<ELFT> *Obj, uint64_t SymValue,
-                              SectionRef FunctionSec,
+                              Optional<SectionRef> FunctionSec,
                               const StringRef SectionName, DataExtractor Data,
                               uint64_t *Offset);
   void printStackSize(const ELFObjectFile<ELFT> *Obj, RelocationRef Rel,
                       SectionRef FunctionSec,
                       const StringRef &StackSizeSectionName,
                       const RelocationResolver &Resolver, DataExtractor Data);
   virtual void printStackSizeEntry(uint64_t Size, StringRef FuncName) = 0;
   virtual void printMipsGOT(const MipsGOTParser<ELFT> &Parser) = 0;
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(
-    const ELFObjectFile<ELFT> *Obj, uint64_t SymValue, SectionRef FunctionSec,
-    const StringRef SectionName, DataExtractor Data, uint64_t *Offset) {
+void DumpStyle<ELFT>::printFunctionStackSize(const ELFObjectFile<ELFT> *Obj,
+                                             uint64_t SymValue,
+                                             Optional<SectionRef> FunctionSec,
+                                             const StringRef SectionName,
+                                             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 : Obj->symbols()) {
     Expected<uint64_t> SymAddrOrErr = Symbol.getAddress();
     if (!SymAddrOrErr) {
       consumeError(SymAddrOrErr.takeError());
       continue;
     }
+    if (Expected<uint32_t> SymFlags = Symbol.getFlags()) {
+      if (*SymFlags & SymbolRef::SF_Undefined)
+        continue;
+    } else
+      consumeError(SymFlags.takeError());
     if (Symbol.getELFType() == ELF::STT_FUNC && *SymAddrOrErr == SymValue) {
-      // Check if the symbol is in the right section.
-      if (FunctionSec.containsSymbol(Symbol)) {
+      // Check if the symbol is in the right section. FunctionSec == None means
+      // "any section".
+      if (!FunctionSec || FunctionSec->containsSymbol(Symbol)) {
         FuncSym = Symbol;
         break;
       }
     }
   }
 
   std::string FuncName = "?";
   // A valid SymbolRef has a non-null object file pointer.
for (const SectionRef &Sec : Obj->sections()) {
     StringRef SectionName = getSectionName(Sec);
     if (SectionName != ".stack_sizes")
       continue;
     PrintHeader();
     const Elf_Shdr *ElfSec = Obj->getSection(Sec.getRawDataRefImpl());
     ArrayRef<uint8_t> Contents =
         unwrapOrError(this->FileName, EF->getSectionContents(ElfSec));
     DataExtractor Data(Contents, Obj->isLittleEndian(), sizeof(Elf_Addr));
-    // A .stack_sizes section header's sh_link field is supposed to point
-    // to the section that contains the functions whose stack sizes are
-    // described in it.
-    const Elf_Shdr *FunctionELFSec =
-        unwrapOrError(this->FileName, EF->getSection(ElfSec->sh_link));
     uint64_t Offset = 0;
     while (Offset < Contents.size()) {
       // The function address is followed by a ULEB representing the stack
       // size. Check for an extra byte before we try to process the entry.
       if (!Data.isValidOffsetForDataOfSize(Offset, sizeof(Elf_Addr) + 1)) {
         reportError(
             createStringError(
                 object_error::parse_failed,
                 "section %s ended while trying to extract a stack size entry",
                 SectionName.data()),
             FileStr);
       }
       uint64_t SymValue = Data.getAddress(&Offset);
-      printFunctionStackSize(Obj, SymValue, Obj->toSectionRef(FunctionELFSec),
-                             SectionName, Data, &Offset);
+      printFunctionStackSize(Obj, SymValue, /*FunctionSec=*/None, SectionName,
+                             Data, &Offset);
     }
   }
 }
 
 template <class ELFT>
 void DumpStyle<ELFT>::printRelocatableStackSizes(
     const ELFObjectFile<ELFT> *Obj, std::function<void()> PrintHeader) {
   const ELFFile<ELFT> *EF = Obj->getELFFile();