[llvm-readelf/obj] - Stop printing wrong addresses for arm32 unwind info for non-relocatable objects.

[grimar]

Summary

This is the first patch for https://bugs.llvm.org/show_bug.cgi?id=47581.

Currently -u does not compute function addresses correctly and
dumps broken addresses for non-relocatable objects.

ARM spec says:
"An index table entry consists of 2 words.
The first word contains a prel31 offset (see Relocations) to the start of a function, with bit 31 clear."
...
"The relocated 31 bits form a place-relative signed offset to the referenced entity.
For brevity, this document will refer to the results of these relocations as "prel31 offsets"."

(https://developer.arm.com/documentation/ihi0038/c/?lang=en#index-table-entries)
(https://developer.arm.com/documentation/ihi0038/c/?lang=en#relocations)

Currently we use an address of the SHT_ARM_EXIDX section instead of an address of an entry
in computations. As a result we compute an offset that is not really "place-relative",
but section relative, what is wrong.

The patch fixes this issue.

[grimar]

• Fix a mistype in comment.

[psmith]

I think there may still be problems in a non-relocatable ELF file when there is more than one executable output section as there is only one .ARM.exidx output section and only one sh_link.

llvm/test/tools/llvm-readobj/ELF/ARM/unwind-non-relocatable.test

70

entry offset should be (0) for the first entry

llvm/tools/llvm-readobj/ARMEHABIPrinter.h

543–567

For a non-relocatable ELF file the sh_link field is not reliable as there can be more than one OutputSection, but there will only be one .ARM.exidx OutputSection and the sh_link can only point to one OutputSection. I think that FunctionAtAddress will need changing so that it doesn't require a matching section when there is non-relocatable objects.

554

Offset is a bit confusing in this context. As I understand it, it is an offset for relocatable objects but an absolute address for non-relocatable objects.

556

You may want to replace the IT->sh_addr with 0 rather than using the 0 from the relocatable object sh_addr, not a strong opinion though.

[grimar]

Thanks, Peter! I've addressed all review comments.

llvm/test/tools/llvm-readobj/ELF/ARM/unwind-non-relocatable.test

70

Done. I've also updated comments to mention that 31 bit is used in offsets.

llvm/tools/llvm-readobj/ARMEHABIPrinter.h

543–567

Thanks! I've did this change and updated the test case to drop sh_link field of the SHT_ARM_EXIDX section.

554

I think we probably can use Address for both? We pass it to the FunctionAtAddress below.

556

Yes, I also found we probably should do it, but I haven't changed it in this patch because it is a possible behavior change for relocatable objects.

If I change this to 0, no our tests fail, but it probably means that the behavior is not well documented by tests. I think changing this for relocatable objects probably belongs to a different patch that should also add a correspondent test.

[psmith]

Thanks for the update. LGTM. Happy to see other parts done in follow up commits

llvm/test/tools/llvm-readobj/ELF/ARM/unwind-non-relocatable.test

+## Check that we dump the unwind information for a non-relocatable input properly.
+
+## Check that we correctly decode function addresses and that we are able to
+## locate corresponding STT_FUNC symbols and dump function names properly.
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj -u %t | FileCheck --check-prefix=UNWIND %s
+# RUN: llvm-readelf -u %t | FileCheck --check-prefix=UNWIND %s
+
+# UNWIND:      UnwindInformation {
+# UNWIND-NEXT:   UnwindIndexTable {
+# UNWIND-NEXT:     SectionIndex: 2
+# UNWIND-NEXT:     SectionName: .ARM.exidx
+# UNWIND-NEXT:     SectionOffset: 0x34
+# UNWIND-NEXT:     Entries [
+# UNWIND-NEXT:       Entry {
+# UNWIND-NEXT:         FunctionAddress: 0x230
+# UNWIND-NEXT:         FunctionName: func1
+# UNWIND-NEXT:         Model: Compact (Inline)
+# UNWIND-NEXT:         PersonalityIndex: 0
+# UNWIND-NEXT:         Opcodes [
+# UNWIND-NEXT:           0xB0      ; finish
+# UNWIND-NEXT:           0xB0      ; finish
+# UNWIND-NEXT:           0xB0      ; finish
+# UNWIND-NEXT:         ]
+# UNWIND-NEXT:       }
+# UNWIND-NEXT:       Entry {
+# UNWIND-NEXT:         FunctionAddress: 0x234
+# UNWIND-NEXT:         FunctionName: func2
+# UNWIND-NEXT:         Model: Compact (Inline)
+# UNWIND-NEXT:         PersonalityIndex: 0
+# UNWIND-NEXT:         Opcodes [
+# UNWIND-NEXT:           0x9B      ; vsp = r11
+# UNWIND-NEXT:           0x84 0x80 ; pop {fp, lr}
+# UNWIND-NEXT:         ]
+# UNWIND-NEXT:       }
+# UNWIND-NEXT:       Entry {
+# UNWIND-NEXT:         FunctionAddress: 0x248
+# UNWIND-NEXT:         FunctionName: func3
+# UNWIND-NEXT:         Model: Compact (Inline)
+# UNWIND-NEXT:         PersonalityIndex: 0
+# UNWIND-NEXT:         Opcodes [
+# UNWIND-NEXT:           0xB0      ; finish
+# UNWIND-NEXT:           0xB0      ; finish
+# UNWIND-NEXT:           0xB0      ; finish
+# UNWIND-NEXT:         ]
+# UNWIND-NEXT:       }
+# UNWIND-NEXT:       Entry {
+# UNWIND-NEXT:         FunctionAddress: 0x24C
+# UNWIND-NEXT:         Model: CantUnwind
+# UNWIND-NEXT:       }
+# UNWIND-NEXT:     ]
+# UNWIND-NEXT:   }
+# UNWIND-NEXT: }
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_ARM
+Sections:
+  - Name:    .text
+    Type:    SHT_PROGBITS
+    Address: 0x230
+  - Name:    .ARM.exidx
+    Type:    SHT_ARM_EXIDX
+    Address: 0x24C
+    ContentArray: [
+## Entry 1. Address of .ARM.exidx (0x24C) + entry offset (0) + 0x7fffffe4 == 0x230 (func1).
+                    0xE4, 0xFF, 0xFF, 0x7F, ## Word(0) = 0x7fffffe4 (31 bit).
+                    0xB0, 0xB0, 0xB0, 0x80, ## Word(1): arbitrary opcodes.
+## Entry 2. Address of .ARM.exidx (0x24C) + entry offset (8) + 0x7fffffe0 == 0x234 (func2).
+                    0xE0, 0xFF, 0xFF, 0x7F, ## Word(0) = 0x7fffffe0 (31 bit).
+                    0x80, 0x84, 0x9B, 0x80, ## Word(1): arbitrary opcodes.
+## Entry 3. Address of .ARM.exidx (0x24C) + entry offset (16) + 0x7fffffec == 0x248 (func2).
+                    0xEC, 0xFF, 0xFF, 0x7F, ## Word(0) = 0x7fffffec (31 bit).
+                    0xB0, 0xB0, 0xB0, 0x80, ## Word(1): arbitrary opcodes.
+## Entry 4. Address of .ARM.exidx (0x24C) + entry offset (24) + 0x7fffffe8 == 0x24C.
+                    0xE8, 0xFF, 0xFF, 0x7F, ## Word(0) = 0x7fffffe8 (31 bit).
+                    0x01, 0x00, 0x00, 0x00  ## Word(1) == EXIDX_CANTUNWIND
+                  ]
+Symbols:
+  - Name:    func1
+    Type:    STT_FUNC
+    Section: .text
+    Value:   0x230
+  - Name:    func2
+    Type:    STT_FUNC
+    Section: .text
+    Value:   0x234
+  - Name:    func3
+    Type:    STT_FUNC
+    Section: .text
+    Value:   0x248

llvm/tools/llvm-readobj/ARMEHABIPrinter.h

class PrinterContext {
 
   static uint64_t PREL31(uint32_t Address, uint32_t Place) {
     uint64_t Location = Address & 0x7fffffff;
     if (Location & 0x04000000)
       Location |= (uint64_t) ~0x7fffffff;
     return Location + Place;
   }
 
-  ErrorOr<StringRef> FunctionAtAddress(unsigned Section, uint64_t Address) const;
+  ErrorOr<StringRef> FunctionAtAddress(uint64_t Address,
+                                       Optional<unsigned> SectionIndex) const;
   const Elf_Shdr *FindExceptionTable(unsigned IndexTableIndex,
                                      off_t IndexTableOffset) const;
 
   void PrintIndexTable(unsigned SectionIndex, const Elf_Shdr *IT) const;
   void PrintExceptionTable(const Elf_Shdr *IT, const Elf_Shdr *EHT,
                            uint64_t TableEntryOffset) const;
   void PrintOpcodes(const uint8_t *Entry, size_t Length, off_t Offset) const;
 
 public:
   PrinterContext(ScopedPrinter &SW, const object::ELFFile<ET> &ELF,
                  StringRef FileName, const Elf_Shdr *Symtab)
       : SW(SW), ELF(ELF), FileName(FileName), Symtab(Symtab) {}
 
   void PrintUnwindInformation() const;
 };
 
 template <typename ET>
 const size_t PrinterContext<ET>::IndexTableEntrySize = 8;
 
 template <typename ET>
 ErrorOr<StringRef>
-PrinterContext<ET>::FunctionAtAddress(unsigned Section,
-                                      uint64_t Address) const {
+PrinterContext<ET>::FunctionAtAddress(uint64_t Address,
+                                      Optional<unsigned> SectionIndex) const {
   if (!Symtab)
     return inconvertibleErrorCode();
   auto StrTableOrErr = ELF.getStringTableForSymtab(*Symtab);
   if (!StrTableOrErr)
     reportError(StrTableOrErr.takeError(), FileName);
   StringRef StrTable = *StrTableOrErr;
 
-  for (const Elf_Sym &Sym : unwrapOrError(FileName, ELF.symbols(Symtab)))
-    if (Sym.st_shndx == Section && Sym.st_value == Address &&
-        Sym.getType() == ELF::STT_FUNC) {
+  for (const Elf_Sym &Sym : unwrapOrError(FileName, ELF.symbols(Symtab))) {
+    if (SectionIndex && *SectionIndex != Sym.st_shndx)
+      continue;
+
+    if (Sym.st_value == Address && Sym.getType() == ELF::STT_FUNC) {
       auto NameOrErr = Sym.getName(StrTable);
       if (!NameOrErr) {
         // TODO: Actually report errors helpfully.
         consumeError(NameOrErr.takeError());
         return inconvertibleErrorCode();
       }
       return *NameOrErr;
     }
+  }
+
   return inconvertibleErrorCode();
 }
 
 template <typename ET>
 const typename ET::Shdr *
 PrinterContext<ET>::FindExceptionTable(unsigned IndexSectionIndex,
                                        off_t IndexTableOffset) const {
   /// Iterate through the sections, searching for the relocation section
case AEABI_UNWIND_CPP_PR2:
                    2);
       break;
     }
   } else {
     SW.printString("Model", StringRef("Generic"));
 
     uint64_t Address = PREL31(Word, EHT->sh_addr);
     SW.printHex("PersonalityRoutineAddress", Address);
-    if (ErrorOr<StringRef> Name = FunctionAtAddress(EHT->sh_link, Address))
+    if (ErrorOr<StringRef> Name =
+            FunctionAtAddress(Address, (unsigned)EHT->sh_link))
       SW.printString("PersonalityRoutineName", *Name);
   }
 }
 
 template <typename ET>
 void PrinterContext<ET>::PrintOpcodes(const uint8_t *Entry,
                                       size_t Length, off_t Offset) const {
   ListScope OCC(SW, "Opcodes");
void PrinterContext<ET>::PrintIndexTable(unsigned SectionIndex,
   ///     with bit 31 clear
   ///   - The exception-handling table entry itself with bit 31 set
   ///   - The special bit pattern EXIDX_CANTUNWIND, indicating that associated
   ///     frames cannot be unwound
 
   const support::ulittle32_t *Data =
     reinterpret_cast<const support::ulittle32_t *>(Contents->data());
   const unsigned Entries = IT->sh_size / IndexTableEntrySize;
+  const bool IsRelocatable = ELF.getHeader().e_type == ELF::ET_REL;
 
   ListScope E(SW, "Entries");
   for (unsigned Entry = 0; Entry < Entries; ++Entry) {
     DictScope E(SW, "Entry");
 
     const support::ulittle32_t Word0 =
       Data[Entry * (IndexTableEntrySize / sizeof(*Data)) + 0];
     const support::ulittle32_t Word1 =
       Data[Entry * (IndexTableEntrySize / sizeof(*Data)) + 1];
 
     if (Word0 & 0x80000000) {
       errs() << "corrupt unwind data in section " << SectionIndex << "\n";
       continue;
     }
 
-    const uint64_t Offset = PREL31(Word0, IT->sh_addr);
-    SW.printHex("FunctionAddress", Offset);
-    if (ErrorOr<StringRef> Name = FunctionAtAddress(IT->sh_link, Offset))
+    // FIXME: For a relocatable object ideally we might want to:
+    // 1) Find a relocation for the offset of Word0.
+    // 2) Verify this relocation is of an expected type (R_ARM_PREL31) and
+    //    verify the symbol index.
+    // 3) Resolve the relocation using it's symbol value, addend etc.
+    // Currently the code assumes that Word0 contains an addend of a
+    // R_ARM_PREL31 REL relocation that references a section symbol. RELA
+    // relocations are not supported and it works because addresses of sections
+    // are nulls in relocatable objects.
+    //
+    // For a non-relocatable object, Word0 contains a place-relative signed
+    // offset to the referenced entity.
+    const uint64_t Address =
+        IsRelocatable
+            ? PREL31(Word0, IT->sh_addr)
+            : PREL31(Word0, IT->sh_addr + Entry * IndexTableEntrySize);
+    SW.printHex("FunctionAddress", Address);
+
+    // In a relocatable output we might have many .ARM.exidx sections linked to
+    // their code sections via the sh_link field. For a non-relocatable ELF file
+    // the sh_link field is not reliable, because we have one .ARM.exidx section
+    // normally, but might have many code sections.
+    Optional<unsigned> SecIndex =
+        IsRelocatable ? Optional<unsigned>(IT->sh_link) : None;
+    if (ErrorOr<StringRef> Name = FunctionAtAddress(Address, SecIndex))
       SW.printString("FunctionName", *Name);
 
     if (Word1 == EXIDX_CANTUNWIND) {
       SW.printString("Model", StringRef("CantUnwind"));
       continue;
     }
 
     if (Word1 & 0x80000000) {