[lib/Object] - Generalize the RelocationResolver API.

[grimar]

Summary

This changes the RelocationResolver to not use RelocationRef anymore:

using RelocationResolver = uint64_t (*)(uint64_t Type, uint64_t Offset,
                                        uint64_t S, uint64_t LocData,
                                        int64_t Addend);

It allows to reuse the RelocationResolver from the code
that doesn't want to deal with RelocationRef related API.

I am going to use it in llvm-readobj. See the description
of D91530 for more details.

[MaskRay]

Seems reasonable. RelocationRef may be awkward to use just because it needs an OwningObject.

Generalize the RelocationResolver API

The description should capture the main point of the patch: change the following type to not use RelocationRef:

using RelocationResolver = uint64_t (*)(uint64_t Type, uint64_t Offset,
                                        uint64_t S, uint64_t LocData,
                                        int64_t Addend);

llvm/lib/Object/RelocationResolver.cpp

42

You have changed the addend parameter from uint64_t to int64_t. What's the motivation?

[grimar]

llvm/lib/Object/RelocationResolver.cpp

42

Actually I am not. See: previously the code called getELFAddend which returns int64_t.
The implementation is still the same:

template <class ELFT>
Expected<int64_t>
ELFObjectFile<ELFT>::getRelocationAddend(DataRefImpl Rel) const {
  if (getRelSection(Rel)->sh_type != ELF::SHT_RELA)
    return createError("Section is not SHT_RELA");
  return (int64_t)getRela(Rel)->r_addend;
}

The argument was of type uint64_t and was called A. But in fact it was not r_addend,
it is the value of data at relocation location (which as we know might contain the addend, e.g on x86,
see how this the API is used by ELFDumper.cpp).
So, I've just renamed uint64_t A -> uint64_t LocData and added int64_t Addend. I haven't changed the type for it.

It just seems that the code here in this file not super clean/consistent. E.g. see:

static uint64_t resolveSparc32(RelocationRef R, uint64_t S, uint64_t A) {
  uint32_t Rel = R.getType();
  if (Rel == ELF::R_SPARC_32 || Rel == ELF::R_SPARC_UA32)
    return S + getELFAddend(R);
  return A;
}

or:

static uint64_t resolveX86_64(RelocationRef R, uint64_t S, uint64_t A) {
 case ELF::R_X86_64_NONE:
    return A;
  case ELF::R_X86_64_64:
  case ELF::R_X86_64_DTPOFF32:
  case ELF::R_X86_64_DTPOFF64:
    return S + getELFAddend(R);

I.e. It might use both getELFAddend(R) and A and seems might require some cleanup. I did not want to
do any functional changes in this patch though. I believe my change fully preserves the original functionality
and just allows to reuse the API.

Also, as far I understand, we need to have both LocData and Addend. E.g. the current implementation for RISCV
uses both of them:

static uint64_t resolveRISCV(uint64_t Type, uint64_t Offset, uint64_t S,
                             uint64_t LocData, int64_t Addend) {
  int64_t RA = Addend;
  int64_t A = LocData;
.....
    return (A & 0xC0) | ((S + RA) & 0x3F);
  case ELF::R_RISCV_SUB6:
    return (A & 0xC0) | (((A & 0x3F) - (S + RA)) & 0x3F);
  case ELF::R_RISCV_ADD8:
    return (A + (S + RA)) & 0xFF;
  case ELF::R_RISCV_SUB8:
    return (A - (S + RA)) & 0xFF;

[MaskRay]

Yes that some RISC-V relocations need both the addend and the implicit addend (LocData; I wonder whether we can improve the name but I don't have a good suggestion)

It just seems that the code here in this file not super clean/consistent. E.g. see:

Yes. In addition, the file is under-tested. It is used for DWARF/llvm-xray/.eh_frame but many relocation types don't have good way for testing.

[jhenderson]

All looks good to me, but probably want @MaskRay's input too.

llvm/lib/Object/RelocationResolver.cpp

270

In all these functions, it might be clearer if you commented out/deleted the name of the unused parameters.

[grimar]

It turns out that LLD also uses this API when works with DWARF. It doesn't compile now and I am investigating how to fix it.

[grimar]

• Updated implementation to make LLD compilable.
• Addressed review comments.

Note: The way how LLD works with relocation resolver API
is a bit tricky. Main logic is located in LLDDwarfObj<ELFT>::findAux
and LLDRelocationResolver::resolve functions:

https://github.com/llvm/llvm-project/blob/master/lld/ELF/DWARF.cpp#L78
https://github.com/llvm/llvm-project/blob/master/lld/ELF/DWARF.cpp#L126

the idea is that it creates the RelocationRef with no owning object
and sets the DataRefImpl.d field either to 0 (for Rel) or to r_addend (for Rela).
Then the DWARFDataExtractor::getRelocatedValue also passes the piece of object data
which contains the addend for Rel objects.

After that, LLD provides 2 resolve functions that do either s + a or s + ref.getRawDataRefImpl().p.
They don't use relocation types and offsets because it is assumed that the computation is
always (S + A). No owning object is requred in this scheme, so it worked before.

The tricky part is that the RelocationRef with no owning object is a specific corner case.
Without an owning object it is not possible to call RelocationRef::getType(),
RelocationRef::getOffset() etc methods.

I had to update the resolveRelocation method to support the LLD specific way of using it.

[grimar]

• Rebased correctly.

[MaskRay]

lld/ELF/DWARF.cpp

89 ↗

(On Diff #306057)

"stored in the field to be relocated" => "extracted from the relocated location"

http://www.sco.com/developers/gabi/latest/ch4.reloc.html has a similar wording.

[MaskRay]

Optional suggestion: I wonder whether RelocatedValue may be better than LocData

[jhenderson]

llvm/lib/Object/RelocationResolver.cpp

754

757

Is it simply not possible to provide Type and Offset information here? Otherwise, this code seems a little fragile to LLD deciding to try to use those fields for something.

[grimar]

llvm/lib/Object/RelocationResolver.cpp

757

Yes, it is not possible without an owning object :(

inline uint64_t RelocationRef::getOffset() const {
  return OwningObject->getRelocationOffset(RelocationPimpl);
}

inline uint64_t RelocationRef::getType() const {
  return OwningObject->getRelocationType(RelocationPimpl);
}

[jhenderson]

llvm/lib/Object/RelocationResolver.cpp

757

I think we need some sort of note added to the LLD resolver then, indicating that the values will always be 0 then (possibly could be an assertion?), because the information isn't available. That would at least hint to future developers to not try to use them.

[grimar]

llvm/lib/Object/RelocationResolver.cpp

757

The type and offset fields are commented out currently in lld/ELF/DWARF.cpp:

static uint64_t resolve(uint64_t /*type*/, uint64_t /*offset*/, uint64_t s,
                        uint64_t /*locData*/, int64_t addend) {
  return s + addend;
}

Do you want me to add an explicit comment about them or uncomment and add an assertion?

[jhenderson]

llvm/lib/Object/RelocationResolver.cpp

757

Either an explicit comment or assertion would be fine by me. Probably comment, because the assertion will need explaining anyway.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM!

llvm/include/llvm/Object/ELFObjectFile.h

protected:
   void getRelocationTypeName(DataRefImpl Rel,
                              SmallVectorImpl<char> &Result) const override;
 
   uint32_t getSectionType(DataRefImpl Sec) const override;
   uint64_t getSectionFlags(DataRefImpl Sec) const override;
   uint64_t getSectionOffset(DataRefImpl Sec) const override;
   StringRef getRelocationTypeName(uint32_t Type) const;
 
-  /// Get the relocation section that contains \a Rel.
-  const Elf_Shdr *getRelSection(DataRefImpl Rel) const {
-    auto RelSecOrErr = EF.getSection(Rel.d.a);
-    if (!RelSecOrErr)
-      report_fatal_error(errorToErrorCode(RelSecOrErr.takeError()).message());
-    return *RelSecOrErr;
-  }
-
   DataRefImpl toDRI(const Elf_Shdr *SymTable, unsigned SymbolNum) const {
     DataRefImpl DRI;
     if (!SymTable) {
       DRI.d.a = 0;
       DRI.d.b = 0;
       return DRI;
     }
     assert(SymTable->sh_type == ELF::SHT_SYMTAB ||
public:
 
   const Elf_Sym *getSymbol(DataRefImpl Sym) const {
     auto Ret = EF.template getEntry<Elf_Sym>(Sym.d.a, Sym.d.b);
     if (!Ret)
       report_fatal_error(errorToErrorCode(Ret.takeError()).message());
     return *Ret;
   }
 
+  /// Get the relocation section that contains \a Rel.
+  const Elf_Shdr *getRelSection(DataRefImpl Rel) const {
+    auto RelSecOrErr = EF.getSection(Rel.d.a);
+    if (!RelSecOrErr)
+      report_fatal_error(errorToErrorCode(RelSecOrErr.takeError()).message());
+    return *RelSecOrErr;
+  }
+
   const Elf_Shdr *getSection(DataRefImpl Sec) const {
     return reinterpret_cast<const Elf_Shdr *>(Sec.p);
   }
 
   basic_symbol_iterator symbol_begin() const override;
   basic_symbol_iterator symbol_end() const override;
 
   elf_symbol_iterator dynamic_symbol_begin() const;

llvm/include/llvm/Object/RelocationResolver.h

 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstdint>
 #include <system_error>
 
 namespace llvm {
 namespace object {
 
-using RelocationResolver = uint64_t (*)(RelocationRef R, uint64_t S, uint64_t A);
+using SupportsRelocation = bool (*)(uint64_t);
+using RelocationResolver = uint64_t (*)(uint64_t Type, uint64_t Offset,
+                                        uint64_t S, uint64_t LocData,
+                                        int64_t Addend);
 
-std::pair<bool (*)(uint64_t), RelocationResolver>
+std::pair<SupportsRelocation, RelocationResolver>
 getRelocationResolver(const ObjectFile &Obj);
 
+uint64_t resolveRelocation(RelocationResolver Resolver,
+                           const object::RelocationRef &R, uint64_t S,
+                           uint64_t LocData);
+
 } // end namespace object
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_RELOCVISITOR_H

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

for (const SectionRef &Section : Obj.sections()) {
           continue;
       }
 
       if (Section.relocation_begin() == Section.relocation_end())
         continue;
 
       // Symbol to [address, section index] cache mapping.
       std::map<SymbolRef, SymInfo> AddrCache;
-      bool (*Supports)(uint64_t);
+      SupportsRelocation Supports;
       RelocationResolver Resolver;
       std::tie(Supports, Resolver) = getRelocationResolver(Obj);
       for (const RelocationRef &Reloc : Section.relocations()) {
         // FIXME: it's not clear how to correctly handle scattered
         // relocations.
         if (isRelocScattered(Obj, Reloc))
           continue;
 

llvm/lib/DebugInfo/DWARF/DWARFDataExtractor.cpp

uint64_t DWARFDataExtractor::getRelocatedValue(uint32_t Size, uint64_t *Off,
 
   ErrorAsOutParameter ErrAsOut(Err);
   Optional<RelocAddrEntry> E = Obj->find(*Section, *Off);
   uint64_t A = getUnsigned(Off, Size, Err);
   if (!E || (Err && *Err))
     return A;
   if (SecNdx)
     *SecNdx = E->SectionIndex;
-  uint64_t R = E->Resolver(E->Reloc, E->SymbolValue, A);
+
+  uint64_t R =
+      object::resolveRelocation(E->Resolver, E->Reloc, E->SymbolValue, A);
   if (E->Reloc2)
-    R = E->Resolver(*E->Reloc2, E->SymbolValue2, R);
+    R = object::resolveRelocation(E->Resolver, *E->Reloc2, E->SymbolValue2, R);
   return R;
 }
 
 Optional<uint64_t>
 DWARFDataExtractor::getEncodedPointer(uint64_t *Offset, uint8_t Encoding,
                                       uint64_t PCRelOffset) const {
   if (Encoding == dwarf::DW_EH_PE_omit)
     return None;

llvm/lib/Object/RelocationResolver.cpp

static bool supportsX86_64(uint64_t Type) {
   case ELF::R_X86_64_32:
   case ELF::R_X86_64_32S:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveX86_64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveX86_64(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_X86_64_NONE:
-    return A;
+    return LocData;
   case ELF::R_X86_64_64:
   case ELF::R_X86_64_DTPOFF32:
   case ELF::R_X86_64_DTPOFF64:
-    return S + getELFAddend(R);
+    return S + Addend;
   case ELF::R_X86_64_PC32:
   case ELF::R_X86_64_PC64:
-    return S + getELFAddend(R) - R.getOffset();
+    return S + Addend - Offset;
   case ELF::R_X86_64_32:
   case ELF::R_X86_64_32S:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsAArch64(uint64_t Type) {
   switch (Type) {
   case ELF::R_AARCH64_ABS32:
   case ELF::R_AARCH64_ABS64:
   case ELF::R_AARCH64_PREL32:
   case ELF::R_AARCH64_PREL64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveAArch64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveAArch64(uint64_t Type, uint64_t Offset, uint64_t S,
+                               uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_AARCH64_ABS32:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   case ELF::R_AARCH64_ABS64:
-    return S + getELFAddend(R);
+    return S + Addend;
   case ELF::R_AARCH64_PREL32:
-    return (S + getELFAddend(R) - R.getOffset()) & 0xFFFFFFFF;
+    return (S + Addend - Offset) & 0xFFFFFFFF;
   case ELF::R_AARCH64_PREL64:
-    return S + getELFAddend(R) - R.getOffset();
+    return S + Addend - Offset;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsBPF(uint64_t Type) {
   switch (Type) {
   case ELF::R_BPF_64_32:
   case ELF::R_BPF_64_64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveBPF(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveBPF(uint64_t Type, uint64_t Offset, uint64_t S,
+                           uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_BPF_64_32:
-    return (S + A) & 0xFFFFFFFF;
+    return (S + LocData) & 0xFFFFFFFF;
   case ELF::R_BPF_64_64:
-    return S + A;
+    return S + LocData;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsMips64(uint64_t Type) {
   switch (Type) {
   case ELF::R_MIPS_32:
   case ELF::R_MIPS_64:
   case ELF::R_MIPS_TLS_DTPREL64:
   case ELF::R_MIPS_PC32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveMips64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveMips64(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_MIPS_32:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   case ELF::R_MIPS_64:
-    return S + getELFAddend(R);
+    return S + Addend;
   case ELF::R_MIPS_TLS_DTPREL64:
-    return S + getELFAddend(R) - 0x8000;
+    return S + Addend - 0x8000;
   case ELF::R_MIPS_PC32:
-    return S + getELFAddend(R) - R.getOffset();
+    return S + Addend - Offset;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsMSP430(uint64_t Type) {
   switch (Type) {
   case ELF::R_MSP430_32:
   case ELF::R_MSP430_16_BYTE:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveMSP430(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveMSP430(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_MSP430_32:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   case ELF::R_MSP430_16_BYTE:
-    return (S + getELFAddend(R)) & 0xFFFF;
+    return (S + Addend) & 0xFFFF;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsPPC64(uint64_t Type) {
   switch (Type) {
   case ELF::R_PPC64_ADDR32:
   case ELF::R_PPC64_ADDR64:
   case ELF::R_PPC64_REL32:
   case ELF::R_PPC64_REL64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolvePPC64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolvePPC64(uint64_t Type, uint64_t Offset, uint64_t S,
+                             uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_PPC64_ADDR32:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   case ELF::R_PPC64_ADDR64:
-    return S + getELFAddend(R);
+    return S + Addend;
   case ELF::R_PPC64_REL32:
-    return (S + getELFAddend(R) - R.getOffset()) & 0xFFFFFFFF;
+    return (S + Addend - Offset) & 0xFFFFFFFF;
   case ELF::R_PPC64_REL64:
-    return S + getELFAddend(R) - R.getOffset();
+    return S + Addend - Offset;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsSystemZ(uint64_t Type) {
   switch (Type) {
   case ELF::R_390_32:
   case ELF::R_390_64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveSystemZ(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveSystemZ(uint64_t Type, uint64_t Offset, uint64_t S,
+                               uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_390_32:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   case ELF::R_390_64:
-    return S + getELFAddend(R);
+    return S + Addend;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsSparc64(uint64_t Type) {
   switch (Type) {
   case ELF::R_SPARC_32:
   case ELF::R_SPARC_64:
   case ELF::R_SPARC_UA32:
   case ELF::R_SPARC_UA64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveSparc64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveSparc64(uint64_t Type, uint64_t Offset, uint64_t S,
+                               uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_SPARC_32:
   case ELF::R_SPARC_64:
   case ELF::R_SPARC_UA32:
   case ELF::R_SPARC_UA64:
-    return S + getELFAddend(R);
+    return S + Addend;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsAmdgpu(uint64_t Type) {
   switch (Type) {
   case ELF::R_AMDGPU_ABS32:
   case ELF::R_AMDGPU_ABS64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveAmdgpu(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveAmdgpu(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_AMDGPU_ABS32:
   case ELF::R_AMDGPU_ABS64:
-    return S + getELFAddend(R);
+    return S + Addend;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsX86(uint64_t Type) {
   switch (Type) {
   case ELF::R_386_NONE:
   case ELF::R_386_32:
   case ELF::R_386_PC32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveX86(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveX86(uint64_t Type, uint64_t Offset, uint64_t S,
+                           uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_386_NONE:
-    return A;
+    return LocData;
   case ELF::R_386_32:
-    return S + A;
+    return S + LocData;
   case ELF::R_386_PC32:
-    return S - R.getOffset() + A;
+    return S - Offset + LocData;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsPPC32(uint64_t Type) {
   switch (Type) {
   case ELF::R_PPC_ADDR32:
   case ELF::R_PPC_REL32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolvePPC32(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolvePPC32(uint64_t Type, uint64_t Offset, uint64_t S,
+                             uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_PPC_ADDR32:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   case ELF::R_PPC_REL32:
-    return (S + getELFAddend(R) - R.getOffset()) & 0xFFFFFFFF;
+    return (S + Addend - Offset) & 0xFFFFFFFF;
   }
   llvm_unreachable("Invalid relocation type");
 }
 
 static bool supportsARM(uint64_t Type) {
   switch (Type) {
   case ELF::R_ARM_ABS32:
   case ELF::R_ARM_REL32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveARM(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveARM(uint64_t Type, uint64_t Offset, uint64_t S,
+                           uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_ARM_ABS32:
-    return (S + A) & 0xFFFFFFFF;
+    return (S + LocData) & 0xFFFFFFFF;
   case ELF::R_ARM_REL32:
-    return (S + A - R.getOffset()) & 0xFFFFFFFF;
+    return (S + LocData - Offset) & 0xFFFFFFFF;
   }
   llvm_unreachable("Invalid relocation type");
 }
 
 static bool supportsAVR(uint64_t Type) {
   switch (Type) {
   case ELF::R_AVR_16:
   case ELF::R_AVR_32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveAVR(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveAVR(uint64_t Type, uint64_t Offset, uint64_t S,
+                           uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case ELF::R_AVR_16:
-    return (S + getELFAddend(R)) & 0xFFFF;
+    return (S + Addend) & 0xFFFF;
   case ELF::R_AVR_32:
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+    return (S + Addend) & 0xFFFFFFFF;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsLanai(uint64_t Type) {
   return Type == ELF::R_LANAI_32;
 }
 
-static uint64_t resolveLanai(RelocationRef R, uint64_t S, uint64_t A) {
-  if (R.getType() == ELF::R_LANAI_32)
-    return (S + getELFAddend(R)) & 0xFFFFFFFF;
+static uint64_t resolveLanai(uint64_t Type, uint64_t Offset, uint64_t S,
+                             uint64_t LocData, int64_t Addend) {
+  if (Type == ELF::R_LANAI_32)
+    return (S + Addend) & 0xFFFFFFFF;
   llvm_unreachable("Invalid relocation type");
 }
 
 static bool supportsMips32(uint64_t Type) {
   switch (Type) {
   case ELF::R_MIPS_32:
   case ELF::R_MIPS_TLS_DTPREL32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveMips32(RelocationRef R, uint64_t S, uint64_t A) {
+static uint64_t resolveMips32(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
   // FIXME: Take in account implicit addends to get correct results.
-  uint32_t Rel = R.getType();
-  if (Rel == ELF::R_MIPS_32)
-    return (S + A) & 0xFFFFFFFF;
-  if (Rel == ELF::R_MIPS_TLS_DTPREL32)
-    return (S + A) & 0xFFFFFFFF;
+  if (Type == ELF::R_MIPS_32)
+    return (S + LocData) & 0xFFFFFFFF;
+  if (Type == ELF::R_MIPS_TLS_DTPREL32)
+    return (S + LocData) & 0xFFFFFFFF;
   llvm_unreachable("Invalid relocation type");
 }
 
 static bool supportsSparc32(uint64_t Type) {
   switch (Type) {
   case ELF::R_SPARC_32:
   case ELF::R_SPARC_UA32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveSparc32(RelocationRef R, uint64_t S, uint64_t A) {
-  uint32_t Rel = R.getType();
-  if (Rel == ELF::R_SPARC_32 || Rel == ELF::R_SPARC_UA32)
-    return S + getELFAddend(R);
-  return A;
+static uint64_t resolveSparc32(uint64_t Type, uint64_t Offset, uint64_t S,
+                               uint64_t LocData, int64_t Addend) {
+  if (Type == ELF::R_SPARC_32 || Type == ELF::R_SPARC_UA32)
+    return S + Addend;
+  return LocData;
 }
 
 static bool supportsHexagon(uint64_t Type) {
   return Type == ELF::R_HEX_32;
 }
 
-static uint64_t resolveHexagon(RelocationRef R, uint64_t S, uint64_t A) {
-  if (R.getType() == ELF::R_HEX_32)
-    return S + getELFAddend(R);
+static uint64_t resolveHexagon(uint64_t Type, uint64_t Offset, uint64_t S,
+                               uint64_t LocData, int64_t Addend) {
+  if (Type == ELF::R_HEX_32)
+    return S + Addend;
   llvm_unreachable("Invalid relocation type");
 }
 
 static bool supportsRISCV(uint64_t Type) {
   switch (Type) {
   case ELF::R_RISCV_NONE:
   case ELF::R_RISCV_32:
   case ELF::R_RISCV_32_PCREL:
static bool supportsRISCV(uint64_t Type) {
   case ELF::R_RISCV_ADD64:
   case ELF::R_RISCV_SUB64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveRISCV(RelocationRef R, uint64_t S, uint64_t A) {
-  int64_t RA = getELFAddend(R);
-  switch (R.getType()) {
+static uint64_t resolveRISCV(uint64_t Type, uint64_t Offset, uint64_t S,
+                             uint64_t LocData, int64_t Addend) {
+  int64_t RA = Addend;
+  int64_t A = LocData;
+  switch (Type) {
   case ELF::R_RISCV_NONE:
-    return A;
+    return LocData;
   case ELF::R_RISCV_32:
     return (S + RA) & 0xFFFFFFFF;
   case ELF::R_RISCV_32_PCREL:
-    return (S + RA - R.getOffset()) & 0xFFFFFFFF;
+    return (S + RA - Offset) & 0xFFFFFFFF;
   case ELF::R_RISCV_64:
     return S + RA;
   case ELF::R_RISCV_SET6:
     return (A & 0xC0) | ((S + RA) & 0x3F);
   case ELF::R_RISCV_SUB6:
     return (A & 0xC0) | (((A & 0x3F) - (S + RA)) & 0x3F);
   case ELF::R_RISCV_ADD8:
     return (A + (S + RA)) & 0xFF;
static bool supportsCOFFX86(uint64_t Type) {
   case COFF::IMAGE_REL_I386_SECREL:
   case COFF::IMAGE_REL_I386_DIR32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveCOFFX86(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveCOFFX86(uint64_t Type, uint64_t Offset, uint64_t S,
+                               uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case COFF::IMAGE_REL_I386_SECREL:
   case COFF::IMAGE_REL_I386_DIR32:
-    return (S + A) & 0xFFFFFFFF;
+    return (S + LocData) & 0xFFFFFFFF;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsCOFFX86_64(uint64_t Type) {
   switch (Type) {
   case COFF::IMAGE_REL_AMD64_SECREL:
   case COFF::IMAGE_REL_AMD64_ADDR64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveCOFFX86_64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveCOFFX86_64(uint64_t Type, uint64_t Offset, uint64_t S,
+                                  uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case COFF::IMAGE_REL_AMD64_SECREL:
-    return (S + A) & 0xFFFFFFFF;
+    return (S + LocData) & 0xFFFFFFFF;
   case COFF::IMAGE_REL_AMD64_ADDR64:
-    return S + A;
+    return S + LocData;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsCOFFARM(uint64_t Type) {
   switch (Type) {
   case COFF::IMAGE_REL_ARM_SECREL:
   case COFF::IMAGE_REL_ARM_ADDR32:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveCOFFARM(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveCOFFARM(uint64_t Type, uint64_t Offset, uint64_t S,
+                               uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case COFF::IMAGE_REL_ARM_SECREL:
   case COFF::IMAGE_REL_ARM_ADDR32:
-    return (S + A) & 0xFFFFFFFF;
+    return (S + LocData) & 0xFFFFFFFF;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsCOFFARM64(uint64_t Type) {
   switch (Type) {
   case COFF::IMAGE_REL_ARM64_SECREL:
   case COFF::IMAGE_REL_ARM64_ADDR64:
     return true;
   default:
     return false;
   }
 }
 
-static uint64_t resolveCOFFARM64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveCOFFARM64(uint64_t Type, uint64_t Offset, uint64_t S,
+                                 uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case COFF::IMAGE_REL_ARM64_SECREL:
-    return (S + A) & 0xFFFFFFFF;
+    return (S + LocData) & 0xFFFFFFFF;
   case COFF::IMAGE_REL_ARM64_ADDR64:
-    return S + A;
+    return S + LocData;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
 static bool supportsMachOX86_64(uint64_t Type) {
   return Type == MachO::X86_64_RELOC_UNSIGNED;
 }
 
-static uint64_t resolveMachOX86_64(RelocationRef R, uint64_t S, uint64_t A) {
-  if (R.getType() == MachO::X86_64_RELOC_UNSIGNED)
+static uint64_t resolveMachOX86_64(uint64_t Type, uint64_t Offset, uint64_t S,
+                                   uint64_t LocData, int64_t Addend) {
+  if (Type == MachO::X86_64_RELOC_UNSIGNED)
     return S;
   llvm_unreachable("Invalid relocation type");
 }
 
 static bool supportsWasm32(uint64_t Type) {
   switch (Type) {
   case wasm::R_WASM_FUNCTION_INDEX_LEB:
   case wasm::R_WASM_TABLE_INDEX_SLEB:
static bool supportsWasm64(uint64_t Type) {
   case wasm::R_WASM_TABLE_INDEX_I64:
   case wasm::R_WASM_FUNCTION_OFFSET_I64:
     return true;
   default:
     return supportsWasm32(Type);
   }
 }
 
-static uint64_t resolveWasm32(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveWasm32(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case wasm::R_WASM_FUNCTION_INDEX_LEB:
   case wasm::R_WASM_TABLE_INDEX_SLEB:
   case wasm::R_WASM_TABLE_INDEX_I32:
   case wasm::R_WASM_MEMORY_ADDR_LEB:
   case wasm::R_WASM_MEMORY_ADDR_SLEB:
   case wasm::R_WASM_MEMORY_ADDR_I32:
   case wasm::R_WASM_TYPE_INDEX_LEB:
   case wasm::R_WASM_GLOBAL_INDEX_LEB:
   case wasm::R_WASM_FUNCTION_OFFSET_I32:
   case wasm::R_WASM_SECTION_OFFSET_I32:
   case wasm::R_WASM_EVENT_INDEX_LEB:
   case wasm::R_WASM_GLOBAL_INDEX_I32:
   case wasm::R_WASM_TABLE_NUMBER_LEB:
     // For wasm section, its offset at 0 -- ignoring Value
-    return A;
+    return LocData;
   default:
     llvm_unreachable("Invalid relocation type");
   }
 }
 
-static uint64_t resolveWasm64(RelocationRef R, uint64_t S, uint64_t A) {
-  switch (R.getType()) {
+static uint64_t resolveWasm64(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
   case wasm::R_WASM_MEMORY_ADDR_LEB64:
   case wasm::R_WASM_MEMORY_ADDR_SLEB64:
   case wasm::R_WASM_MEMORY_ADDR_I64:
   case wasm::R_WASM_TABLE_INDEX_SLEB64:
   case wasm::R_WASM_TABLE_INDEX_I64:
   case wasm::R_WASM_FUNCTION_OFFSET_I64:
     // For wasm section, its offset at 0 -- ignoring Value
-    return A;
+    return LocData;
   default:
-    return resolveWasm32(R, S, A);
+    return resolveWasm32(Type, Offset, S, LocData, Addend);
   }
 }
 
-std::pair<bool (*)(uint64_t), RelocationResolver>
+std::pair<SupportsRelocation, RelocationResolver>
 getRelocationResolver(const ObjectFile &Obj) {
   if (Obj.isCOFF()) {
     switch (Obj.getArch()) {
     case Triple::x86_64:
       return {supportsCOFFX86_64, resolveCOFFX86_64};
     case Triple::x86:
       return {supportsCOFFX86, resolveCOFFX86};
     case Triple::arm:
if (Obj.isCOFF()) {
     if (Obj.getArch() == Triple::wasm64)
       return {supportsWasm64, resolveWasm64};
     return {nullptr, nullptr};
   }
 
   llvm_unreachable("Invalid object file");
 }
 
+uint64_t resolveRelocation(RelocationResolver Resolver,
+                           const object::RelocationRef &R, uint64_t S,
+                           uint64_t LocData) {
+  int64_t Addend = 0;
+  const object::ObjectFile &Obj = *R.getObject();
+  if (R.getObject()->isELF()) {
+    auto GetRelSectionType = [&]() -> unsigned {
+      if (auto *Elf32LEObj = dyn_cast<ELF32LEObjectFile>(&Obj))
+        return Elf32LEObj->getRelSection(R.getRawDataRefImpl())->sh_type;
+      if (auto *Elf64LEObj = dyn_cast<ELF64LEObjectFile>(&Obj))
+        return Elf64LEObj->getRelSection(R.getRawDataRefImpl())->sh_type;
+      if (auto *Elf32BEObj = dyn_cast<ELF32BEObjectFile>(&Obj))
+        return Elf32BEObj->getRelSection(R.getRawDataRefImpl())->sh_type;
+      auto *Elf64BEObj = cast<ELF64BEObjectFile>(&Obj);
+      return Elf64BEObj->getRelSection(R.getRawDataRefImpl())->sh_type;
+    };
+
+    if (GetRelSectionType() == ELF::SHT_RELA)
+      Addend = getELFAddend(R);
+  }
+
+  return Resolver(R.getType(), R.getOffset(), S, LocData, Addend);
+}
+
 } // namespace object
 } // namespace llvm

llvm/lib/XRay/InstrumentationMap.cpp

uint32_t RelativeRelocation = [](object::ObjectFile *ObjFile) {
         return ELFObj->getELFFile()->getRelativeRelocationType();
       else if (const auto *ELFObj =
                    dyn_cast<object::ELF64BEObjectFile>(ObjFile))
         return ELFObj->getELFFile()->getRelativeRelocationType();
       else
         return static_cast<uint32_t>(0);
     }(ObjFile.getBinary());
 
-    bool (*SupportsRelocation)(uint64_t);
+    object::SupportsRelocation Supports;
     object::RelocationResolver Resolver;
-    std::tie(SupportsRelocation, Resolver) =
+    std::tie(Supports, Resolver) =
         object::getRelocationResolver(*ObjFile.getBinary());
 
     for (const object::SectionRef &Section : Sections) {
       for (const object::RelocationRef &Reloc : Section.relocations()) {
         if (ObjFile.getBinary()->getArch() == Triple::arm) {
-          if (SupportsRelocation && SupportsRelocation(Reloc.getType())) {
+          if (Supports && Supports(Reloc.getType())) {
             Expected<uint64_t> ValueOrErr = Reloc.getSymbol()->getValue();
             if (!ValueOrErr)
               return ValueOrErr.takeError();
-            Relocs.insert({Reloc.getOffset(), Resolver(Reloc, *ValueOrErr, 0)});
+            Relocs.insert(
+                {Reloc.getOffset(),
+                 object::resolveRelocation(Resolver, Reloc, *ValueOrErr, 0)});
           }
-        } else if (SupportsRelocation && SupportsRelocation(Reloc.getType())) {
+        } else if (Supports && Supports(Reloc.getType())) {
           auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend();
           auto A = AddendOrErr ? *AddendOrErr : 0;
           Expected<uint64_t> ValueOrErr = Reloc.getSymbol()->getValue();
           if (!ValueOrErr)
             // TODO: Test this error.
             return ValueOrErr.takeError();
-          Relocs.insert({Reloc.getOffset(), Resolver(Reloc, *ValueOrErr, A)});
+          Relocs.insert(
+              {Reloc.getOffset(),
+               object::resolveRelocation(Resolver, Reloc, *ValueOrErr, A)});
         } else if (Reloc.getType() == RelativeRelocation) {
           if (auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend())
             Relocs.insert({Reloc.getOffset(), *AddendOrErr});
         }
       }
     }
   }
 

llvm/test/Object/invalid.test

 ## Check that llvm-objdump reports an error when .shstrtab has an invalid
 ## size that goes past the end of the file.
 
 # RUN: not llvm-objdump -s %p/Inputs/invalid-strtab-size.elf 2>&1 \
 # RUN:   | FileCheck %s -DFILE=%p/Inputs/invalid-strtab-size.elf --check-prefix=INVALID-STRTAB-SIZE
 
 # INVALID-STRTAB-SIZE: error: '[[FILE]]': section [index 1] has a sh_offset (0x70) + sh_size (0xffffff) that is greater than the file size (0x218)
 
-## Check that llvm-dwarfdump reports an error during relocation resolution
-## when instead of expected SHT_RELA section it locates a section of a different type.
+## Document that llvm-dwarfdump doesn't reports errors/warnings during relocation resolution
+## when instead of SHT_RELA section we have a SHT_REL section in an object.
+## In this case it just doesn't try to read relocation addends and assumes they are 0.
 
+## Note: we don't check the output, because the intention of this test is just to check the
+## exit code and to document the fact that no warnings are reported.
 # RUN: yaml2obj %s --docnum=3 -o %t3
-# RUN: not --crash llvm-dwarfdump -debug-line %t3 2>&1 | FileCheck --check-prefix=RELA %s
-
-# RELA: LLVM ERROR: Section is not SHT_RELA
+# RUN: llvm-dwarfdump -debug-line %t3 2>&1 | FileCheck --implicit-check-not=warning: %s
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:

llvm/tools/llvm-readobj/ELFDumper.cpp

reportUniqueWarning(createStringError(
         object_error::parse_failed,
         "found invalid relocation offset (0x" + Twine::utohexstr(Offset) +
             ") into " + describe(Obj, StackSizeSec) +
             " while trying to extract a stack size entry"));
     return;
   }
 
   uint64_t Addend = Data.getAddress(&Offset);
-  uint64_t SymValue = Resolver(Reloc, RelocSymValue, Addend);
+  uint64_t SymValue = resolveRelocation(Resolver, Reloc, RelocSymValue, Addend);
   this->printFunctionStackSize(SymValue, FunctionSec, StackSizeSec, Data,
                                &Offset);
 }
 
 template <class ELFT>
 void DumpStyle<ELFT>::printNonRelocatableStackSizes(
     std::function<void()> PrintHeader) {
   // This function ignores potentially erroneous input, unless it is directly
if (!RelocSec) {
       continue;
     }
 
     // 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 *FunctionSec = unwrapOrError(
         this->FileName, Obj.getSection(StackSizesELFSec->sh_link));
-    bool (*IsSupportedFn)(uint64_t);
+
+    object::SupportsRelocation IsSupportedFn;
     RelocationResolver Resolver;
     std::tie(IsSupportedFn, Resolver) = getRelocationResolver(ElfObj);
     ArrayRef<uint8_t> Contents =
         unwrapOrError(this->FileName, Obj.getSectionContents(*StackSizesELFSec));
     DataExtractor Data(Contents, Obj.isLE(), sizeof(Elf_Addr));
 
     size_t I = 0;
     for (const RelocationRef &Reloc :