[llvm-readobj] [COFF] Try to resolve symbols in unwind info on x86

[mstorsjo]

Summary

This is the same as we do on arm64 already for the MSVC style label
symbols, but also handle the way GCC produces it - with all relocations
pointing at the .text section symbol, with various offsets.

[mstorsjo]

llvm/test/tools/llvm-readobj/COFF/x86_64-unwind-preferred-symbol-gcc.yaml

9

Note that we might resolve the end address into the symbol of the next function that starts directly after, in some cases (if there's no padding inbetween), but that's hopefully tolerable.

[mstorsjo]

Shrunk the text section of the test cases a little bit.

[mstorsjo]

Improve the logic for picking the symbol for the end of the range, to avoid picking the start of the next function.

[efriedma]

llvm/tools/llvm-readobj/Win64EHDumper.cpp

138

Is repeatedly iterating over every symbol in the file going to cause performance issues?

[mstorsjo]

llvm/tools/llvm-readobj/Win64EHDumper.cpp

138

Yes if this would be performance sensitive, we probably should populate some different data structure for it - but I think it's not. The original code that looks up the relocation, by scanning linearly over all relocations in the object file until it hits one at the right section+offset, also does a similar order of magnitude of iterations.

[efriedma]

llvm/tools/llvm-readobj/Win64EHDumper.cpp

138

I suspect we wouldn't notice even if there were performance issues: probably very few people use llvm-readobj on files of non-trivial size. I guess if you're not making the existing issues substantially worse, it's fine.

[rnk]

lgtm

llvm/test/tools/llvm-readobj/COFF/x86_64-unwind-preferred-symbol-gcc.yaml

+## Check that we print the external symbols "func1" and "func2", even though
+## the pdata relocations point at the '.text' symbol.
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: llvm-readobj --unwind %t.obj | FileCheck %s
+
+# CHECK: StartAddress: func1
+# CHECK: EndAddress: func1 +0x14
+# CHECK: UnwindInfoAddress: .xdata
+
+# CHECK: StartAddress: func2
+# CHECK: EndAddress: func2 +0x16
+# CHECK: UnwindInfoAddress: .xdata +0x8
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_LINE_NUMS_STRIPPED ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     4883EC38488D54242CE800000000904883C438C366662E0F1F840000000000904883EC3831C9488D54242CE800000000904883C438C3
+    Relocations:
+      - VirtualAddress:  10
+        SymbolName:      other
+        Type:            IMAGE_REL_AMD64_REL32
+      - VirtualAddress:  44
+        SymbolName:      other
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            .xdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '01040100046200000104010004620000'
+  - Name:            .pdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '000000001400000000000000200000003600000008000000'
+    Relocations:
+      - VirtualAddress:  0
+        SymbolName:      .text
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  4
+        SymbolName:      .text
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  8
+        SymbolName:      .xdata
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  12
+        SymbolName:      .text
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  16
+        SymbolName:      .text
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  20
+        SymbolName:      .xdata
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+symbols:
+  - Name:            func1
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+    FunctionDefinition:
+      TagIndex:        0
+      TotalSize:       0
+      PointerToLinenumber: 0
+      PointerToNextFunction: 0
+  - Name:            func2
+    Value:           32
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          54
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            .xdata
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          16
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            .pdata
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          24
+      NumberOfRelocations: 6
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            other
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

llvm/test/tools/llvm-readobj/COFF/x86_64-unwind-preferred-symbol-msvc.yaml

+## Check that we print the external symbols "func1" and "func2", even though
+## the pdata relocations point at the '$LN4' symbols.
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: llvm-readobj --unwind %t.obj | FileCheck %s
+
+# CHECK: StartAddress: func1
+# CHECK: EndAddress: func1 +0x13
+# CHECK: UnwindInfoAddress: $unwind$func1
+
+# CHECK: StartAddress: func2
+# CHECK: EndAddress: func2 +0x15
+# CHECK: UnwindInfoAddress: $unwind$func2
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     4883EC28488D542438E8000000004883C428C34883EC28488D54243033C9E8000000004883C428C3
+    Relocations:
+      - VirtualAddress:  10
+        SymbolName:      other
+        Type:            IMAGE_REL_AMD64_REL32
+      - VirtualAddress:  44
+        SymbolName:      other
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            .xdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '01040100044200000104010004420000'
+  - Name:            .pdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     '000000001300000000000000000000001500000000000000'
+    Relocations:
+      - VirtualAddress:  0
+        SymbolTableIndex: 5
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  4
+        SymbolTableIndex: 5
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  8
+        SymbolName:      '$unwind$func1'
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  12
+        SymbolTableIndex: 6
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  16
+        SymbolTableIndex: 6
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+      - VirtualAddress:  20
+        SymbolName:      '$unwind$func2'
+        Type:            IMAGE_REL_AMD64_ADDR32NB
+symbols:
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          53
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        973622814
+      Number:          0
+  - Name:            other
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            func1
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            func2
+    Value:           19
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            '$LN4'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_LABEL
+  - Name:            '$LN4'
+    Value:           19
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_LABEL
+  - Name:            .xdata
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          16
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        1006417668
+      Number:          0
+  - Name:            '$unwind$func1'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            .pdata
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          24
+      NumberOfRelocations: 6
+      NumberOfLinenumbers: 0
+      CheckSum:        2067061152
+      Number:          0
+  - Name:            '$pdata$func1'
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            '$unwind$func2'
+    Value:           8
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            '$pdata$func2'
+    Value:           12
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+...

llvm/tools/llvm-readobj/Win64EHDumper.cpp

for (const auto &Symbol : COFF.symbols()) {
     if (*Address == VA) {
       Sym = Symbol;
       return std::error_code();
     }
   }
   return inconvertibleErrorCode();
 }
 
+static object::SymbolRef getPreferredSymbol(const COFFObjectFile &COFF,
+                                            object::SymbolRef Sym,
+                                            uint32_t &SymbolOffset,
+                                            bool IsRangeEnd) {
+  // The symbol resolved by ResolveSymbol can be any internal
+  // nondescriptive symbol; try to resolve a more descriptive one.
+  COFFSymbolRef CoffSym = COFF.getCOFFSymbol(Sym);
+  if (CoffSym.getStorageClass() != COFF::IMAGE_SYM_CLASS_LABEL &&
+      CoffSym.getSectionDefinition() == nullptr)
+    return Sym;
+  for (const auto &S : COFF.symbols()) {
+    COFFSymbolRef CS = COFF.getCOFFSymbol(S);
+    if (CS.getSectionNumber() == CoffSym.getSectionNumber() &&
+        CS.getValue() <= CoffSym.getValue() + SymbolOffset &&
+        CS.getStorageClass() != COFF::IMAGE_SYM_CLASS_LABEL &&
+        CS.getSectionDefinition() == nullptr) {
+      uint32_t Offset = CoffSym.getValue() + SymbolOffset - CS.getValue();
+      // For the end of a range, don't pick a symbol with a zero offset;
+      // prefer a symbol with a small positive offset.
+      if (Offset <= SymbolOffset && (!IsRangeEnd || Offset > 0)) {
+        SymbolOffset = Offset;
+        Sym = S;
+        CoffSym = CS;
+        if (CS.isExternal() && SymbolOffset == 0)
+          return Sym;
+      }
+    }
+  }
+  return Sym;
+}
+
 static std::string formatSymbol(const Dumper::Context &Ctx,
                                 const coff_section *Section, uint64_t Offset,
-                                uint32_t Displacement) {
+                                uint32_t Displacement,
+                                bool IsRangeEnd = false) {
   std::string Buffer;
   raw_string_ostream OS(Buffer);
 
   SymbolRef Symbol;
   if (!Ctx.ResolveSymbol(Section, Offset, Symbol, Ctx.UserData)) {
+    // We found a relocation at the given offset in the section, pointing
+    // at a symbol.
+
+    // Try to resolve label/section symbols into function names.
+    Symbol = getPreferredSymbol(Ctx.COFF, Symbol, Displacement, IsRangeEnd);
+
     Expected<StringRef> Name = Symbol.getName();
     if (Name) {
       OS << *Name;
       if (Displacement > 0)
         OS << format(" +0x%X (0x%" PRIX64 ")", Displacement, Offset);
       else
         OS << format(" (0x%" PRIX64 ")", Offset);
       return OS.str();

 namespace Win64EH {
 void Dumper::printRuntimeFunctionEntry(const Context &Ctx,
                                        const coff_section *Section,
                                        uint64_t Offset,
                                        const RuntimeFunction &RF) {
   SW.printString("StartAddress",
                  formatSymbol(Ctx, Section, Offset + 0, RF.StartAddress));
   SW.printString("EndAddress",
-                 formatSymbol(Ctx, Section, Offset + 4, RF.EndAddress));
+                 formatSymbol(Ctx, Section, Offset + 4, RF.EndAddress,
+                              /*IsRangeEnd=*/true));
   SW.printString("UnwindInfoAddress",
                  formatSymbol(Ctx, Section, Offset + 8, RF.UnwindInfoOffset));
 }
 
 // Prints one unwind code. Because an unwind code can occupy up to 3 slots in
 // the unwind codes array, this function requires that the correct number of
 // slots is provided.
 void Dumper::printUnwindCode(const UnwindInfo& UI, ArrayRef<UnwindCode> UC) {