[llvm-readobj] [COFF] Resolve relocations pointing at section symbols for arm64 too

[mstorsjo]

Summary

This syncs parts from the x86 implementation to the ARMWinEH
implementation.

Currently, neither of the compilers targeting COFF/arm64 (MSVC, LLVM)
produce such relocations, but LLVM might after a later patch.

[jhenderson]

Couple of minor comments, since my Herald rule added me, but @rnk should do the real review.

llvm/test/tools/llvm-readobj/COFF/arm64-unwind-preferred-symbol2.yaml

19

I've not got much experience with yaml2obj for COFF, but is this reasonably minimal? It looks like a lot of data, for what looks like a fairly small piece of testing.

llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp

292

Nit: add missing full stop whilst you're moving this comment.

[mstorsjo]

llvm/test/tools/llvm-readobj/COFF/arm64-unwind-preferred-symbol2.yaml

19

It's two functions that are nontrivial enough to actually generate unwind info, and one of them even less trivial so that it can't be described with the packed unwind info format. (The packed unwind info format has one relocation, the full format has two relocations, thus wanting to include both forms in the test.)

Sure there's no strict need to have valid code here, but it doesn't cost anything with regards to actual number of lines in this test, and makes for a slightly more understandable example to have the code actually match the unwind info.

llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp

292

Sure, will do.

[rnk]

lgtm

llvm/test/tools/llvm-readobj/COFF/arm64-unwind-preferred-symbol2.yaml

+## Check that we print the external symbol "func1" and "func2", even though
+## the pdata relocations point at .text with an immediate offset.
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: llvm-readobj --unwind %t.obj | FileCheck %s
+
+# CHECK: Function: func1 (0x0)
+# CHECK: Function: func2 (0x14)
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_ARM64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     FE0F1FF8E133009100000094FE0741F8C0035FD6FF8300D1F60B00F9FE0F00F9E1330091E0031FAA00000094FE0F40F9F60B40F9FF830091C0035FD6
+    Relocations:
+      - VirtualAddress:  8
+        SymbolName:      other
+        Type:            IMAGE_REL_ARM64_BRANCH26
+      - VirtualAddress:  40
+        SymbolName:      other
+        Type:            IMAGE_REL_ARM64_BRANCH26
+  - Name:            .xdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     0A002010D2C3D0C202E4E3E3
+  - Name:            .pdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     000000001500A0001400000000000000
+    Relocations:
+      - VirtualAddress:  0
+        SymbolName:      .text
+        Type:            IMAGE_REL_ARM64_ADDR32NB
+      - VirtualAddress:  8
+        SymbolName:      .text
+        Type:            IMAGE_REL_ARM64_ADDR32NB
+      - VirtualAddress:  12
+        SymbolName:      .xdata
+        Type:            IMAGE_REL_ARM64_ADDR32NB
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          60
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        2313530660
+      Number:          1
+  - Name:            .xdata
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          12
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        2034129209
+      Number:          2
+  - Name:            .pdata
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          16
+      NumberOfRelocations: 3
+      NumberOfLinenumbers: 0
+      CheckSum:        3306336236
+      Number:          3
+  - Name:            func1
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            other
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            func2
+    Value:           20
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

llvm/tools/llvm-readobj/ARMWinEHPrinter.h

class Decoder {
   ErrorOr<object::SymbolRef>
   getSymbolForLocation(const object::COFFObjectFile &COFF,
                        const object::SectionRef &Section,
                        uint64_t OffsetInSection, uint64_t ImmediateOffset,
                        uint64_t &SymbolAddress, uint64_t &SymbolOffset,
                        bool FunctionOnly = false);
 
   object::SymbolRef getPreferredSymbol(const object::COFFObjectFile &COFF,
-                                       object::SymbolRef Sym);
+                                       object::SymbolRef Sym,
+                                       uint64_t &SymbolOffset);
 
   bool dumpXDataRecord(const object::COFFObjectFile &COFF,
                        const object::SectionRef &Section,
                        uint64_t FunctionAddress, uint64_t VA);
   bool dumpUnpackedEntry(const object::COFFObjectFile &COFF,
                          const object::SectionRef Section, uint64_t Offset,
                          unsigned Index, const RuntimeFunction &Entry);
   bool dumpPackedEntry(const object::COFFObjectFile &COFF,

llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp

ErrorOr<SymbolRef> Decoder::getRelocatedSymbol(const COFFObjectFile &,
   for (const auto &Relocation : Section.relocations()) {
     uint64_t RelocationOffset = Relocation.getOffset();
     if (RelocationOffset == Offset)
       return *Relocation.getSymbol();
   }
   return inconvertibleErrorCode();
 }
 
-SymbolRef Decoder::getPreferredSymbol(const COFFObjectFile &COFF,
-                                      SymbolRef Sym) {
+SymbolRef Decoder::getPreferredSymbol(const COFFObjectFile &COFF, SymbolRef Sym,
+                                      uint64_t &SymbolOffset) {
   // The symbol resolved by getRelocatedSymbol 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)
+  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()) {
-      if (CS.isExternal())
-        return S;
-      if (CS.getStorageClass() != COFF::IMAGE_SYM_CLASS_LABEL) {
+        CS.getValue() <= CoffSym.getValue() + SymbolOffset &&
+        CS.getStorageClass() != COFF::IMAGE_SYM_CLASS_LABEL &&
+        CS.getSectionDefinition() == nullptr) {
+      uint32_t Offset = CoffSym.getValue() + SymbolOffset - CS.getValue();
+      if (Offset <= SymbolOffset) {
+        SymbolOffset = Offset;
         Sym = S;
         CoffSym = CS;
+        if (CS.isExternal() && SymbolOffset == 0)
+          return Sym;
       }
     }
   }
   return Sym;
 }
 
 ErrorOr<SymbolRef> Decoder::getSymbolForLocation(
     const COFFObjectFile &COFF, const SectionRef &Section,
if (!AddressOrErr) {
       llvm::raw_string_ostream OS(Buf);
       logAllUnhandledErrors(AddressOrErr.takeError(), OS);
       OS.flush();
       report_fatal_error(Buf);
     }
     // We apply SymbolOffset here directly. We return it separately to allow
     // the caller to print it as an offset on the symbol name.
     SymbolAddress = *AddressOrErr + SymbolOffset;
+
+    if (FunctionOnly) // Resolve label/section symbols into function names.
+      SymOrErr = getPreferredSymbol(COFF, *SymOrErr, SymbolOffset);
   } else {
     // No matching relocation found; operating on a linked image. Try to
     // find a descriptive symbol if possible. The immediate offset contains
     // the image relative address, and we shouldn't add any offset to the
     // symbol.
     SymbolAddress = COFF.getImageBase() + ImmediateOffset;
     SymbolOffset = 0;
     SymOrErr = getSymbol(COFF, SymbolAddress, FunctionOnly);
   }
-  if (SymOrErr && FunctionOnly) // Resolve label symbols into function names
-    SymOrErr = getPreferredSymbol(COFF, *SymOrErr);
   return SymOrErr;
 }
 
 bool Decoder::opcode_0xxxxxxx(const uint8_t *OC, unsigned &Offset,
                               unsigned Length, bool Prologue) {
   uint8_t Imm = OC[Offset] & 0x7f;
   SW.startLine() << format("0x%02x                ; %s sp, #(%u * 4)\n",
                            OC[Offset],