[ELF/AArch64] Print a diagnostic message if nonzero addend is used in a GOT relocation.

[ikudrin]

Summary

The addend in the GOT relocations for AArch64 has a special meaning,
it requires a separate GOT entry to be created for each S + A case.

It is not clear, if nonzero addends for GOT relocations are used in the wild,
so let's just print a diagnostic message for now.

[ruiu]

Do we really want to have this check? We don't have to check all possible ABI violations of input files. This is a kind of ABI violation that if you set an addend to a relocation, the output will have a value with that addend, which is a natural consequence.

[ikudrin]

In D14991#296701, @ruiu wrote:

Do we really want to have this check? We don't have to check all possible ABI violations of input files. This is a kind of ABI violation that if you set an addend to a relocation, the output will have a value with that addend, which is a natural consequence.

As far as I can understand, it's not a violation of ABI rules in input files. See section "4.6.3 Relocation types" in http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056b/IHI0056B_aaelf64.pdf:

• GDAT(S+A) represents a 64-bit entry in the GOT for address S+A. The entry will be relocated at run time with relocation R_AARCH64_GLOB_DAT(S+A).
• G(expr) is the address of the GOT entry for the expression expr.

And in section "4.6.6 Static AArch64 relocations", "Table 4-14, GOT-relative instruction relocations":

R_AARCH64_ADR_GOT_PAGE: Page(G(GDAT(S+A))) - Page(P)
R_AARCH64_LD64_GOT_LO12_NC: G(GDAT(S+A))

So, the input is valid, but I'm doubt if it is in use or not. If we don't detect the situation we will silently produce broken executable file with bugs which are very hard to be caught. But if we print a message, it'll clearly point to the problem case.

[ruiu]

It sounds like a bit overkill. There are bunch of other cases in which the
linker silently creates broken outputs. As long as creating broken
executables is a natural consequence of providing broken inputs, I wouldn't
try to catch that. My concern is achieving a right balance -- this patch
introduces the concept of "isNonzeroAddendSupported" just for checking some
specific type of error.

[davide]

I'm personally against the work of catching all ABI violations for reasons fairly similar to the ones already stated by Rui. It adds complexity for very little gain, at least at this phase of development.

[ikudrin]

It's very weird for me to not detect a known flaw in the product.

[ruiu]

I'd disagree -- I think it's not a flaw of a product to not detect all
possible corner cases like this.

[ruiu]

As another data point, we don't check for an object file in an archive file
if the object file really contains a symbol that we found in the archive
file symbol table. Or, for x86-64 TLS relocation optimization, we don't
check for TLS optimization to be used for right combination of
instructions. Because these checks would be too costly for computers or
humans who read code or both.

[ikudrin]

Your first example violates the expected format of archive, if I understand it right. We may rely on underlying library and its checks in that case.

I have no comment for the second example. May be it's hard to imagine other command or something.

But in this case, I just wanted to detect our own ABI violation. Not to implement that case, as it might be rare, just detect. And I wanted to be honest with our users and not generate broken result from an ABI-compliant input if it can be avoided.

[ruiu]

They are examples in which we create invalid outputs for broken inputs. My
point still stands. It is not always unacceptable to create invalid outputs
in such case.

[davide]

Can this be closed?

ELF/InputSection.cpp

void InputSectionBase<ELFT>::relocate(
   typedef Elf_Rel_Impl<ELFT, isRela> RelType;
   for (const RelType &RI : Rels) {
     uint32_t SymIndex = RI.getSymbol(Config->Mips64EL);
     uint32_t Type = RI.getType(Config->Mips64EL);
     uintX_t Offset = getOffset(RI.r_offset);
     if (Offset == (uintX_t)-1)
       continue;
 
+    uintX_t Addend = getAddend<ELFT>(RI);
+    if (Addend != 0 && !Target->isNonzeroAddendSupported(Type))
+      error("Nonzero addend is not supported for relocation " +
+            getELFRelocationTypeName(Config->EMachine, Type));
+
     uint8_t *BufLoc = Buf + Offset;
     uintX_t AddrLoc = OutSec->getVA() + Offset;
 
     if (Target->isTlsLocalDynamicReloc(Type)) {
       Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc,
                           Out<ELFT>::Got->getVA() +
-                              Out<ELFT>::LocalModuleTlsIndexOffset +
-                              getAddend<ELFT>(RI));
+                              Out<ELFT>::LocalModuleTlsIndexOffset + Addend);
       continue;
     }
 
     // Handle relocations for local symbols -- they never get
     // resolved so we don't allocate a SymbolBody.
     const Elf_Shdr *SymTab = File->getSymbolTable();
     if (SymIndex < SymTab->sh_info) {
       uintX_t SymVA = getLocalRelTarget(*File, RI);
       Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc, SymVA);
       continue;
     }
 
     SymbolBody &Body = *File->getSymbolBody(SymIndex)->repl();
 
     if (Target->isTlsGlobalDynamicReloc(Type)) {
       Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc,
-                          Out<ELFT>::Got->getEntryAddr(Body) +
-                              getAddend<ELFT>(RI));
+                          Out<ELFT>::Got->getEntryAddr(Body) + Addend);
       continue;
     }
 
     if (Target->isTlsOptimized(Type, Body)) {
       Target->relocateTlsOptimize(BufLoc, BufEnd, AddrLoc,
                                   getSymVA<ELFT>(Body));
       continue;
     }
 
     uintX_t SymVA = getSymVA<ELFT>(Body);
     if (Target->relocNeedsPlt(Type, Body)) {
       SymVA = Out<ELFT>::Plt->getEntryAddr(Body);
       Type = Target->getPltRefReloc(Type);
     } else if (Target->relocNeedsGot(Type, Body)) {
       SymVA = Out<ELFT>::Got->getEntryAddr(Body);
       Type = Body.isTLS() ? Target->getTlsGotReloc()
                           : Target->getGotRefReloc(Type);
     } else if (Target->relocPointsToGot(Type)) {
       SymVA = Out<ELFT>::Got->getVA();
       Type = Target->getPCRelReloc();
     } else if (!Target->relocNeedsCopy(Type, Body) &&
                isa<SharedSymbol<ELFT>>(Body)) {
       continue;
     }
-    Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc,
-                        SymVA + getAddend<ELFT>(RI));
+    Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc, SymVA + Addend);
   }
 }
 
 template <class ELFT> void InputSection<ELFT>::writeTo(uint8_t *Buf) {
   if (this->Header->sh_type == SHT_NOBITS)
     return;
   // Copy section contents from source object file to output file.
   ArrayRef<uint8_t> Data = this->getSectionData();

ELF/Target.h

public:
   virtual unsigned getPltRefReloc(unsigned Type) const;
   virtual void writeGotHeaderEntries(uint8_t *Buf) const;
   virtual void writeGotPltHeaderEntries(uint8_t *Buf) const;
   virtual void writeGotPltEntry(uint8_t *Buf, uint64_t Plt) const = 0;
   virtual void writePltZeroEntry(uint8_t *Buf, uint64_t GotEntryAddr,
                                  uint64_t PltEntryAddr) const = 0;
   virtual void writePltEntry(uint8_t *Buf, uint64_t GotEntryAddr,
                              uint64_t PltEntryAddr, int32_t Index) const = 0;
+  virtual bool isNonzeroAddendSupported(uint32_t Type) const;
   virtual bool isRelRelative(uint32_t Type) const;
   virtual bool relocNeedsCopy(uint32_t Type, const SymbolBody &S) const;
   virtual bool relocNeedsGot(uint32_t Type, const SymbolBody &S) const = 0;
   virtual bool relocPointsToGot(uint32_t Type) const;
   virtual bool relocNeedsPlt(uint32_t Type, const SymbolBody &S) const = 0;
   virtual void relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
                            uint64_t P, uint64_t SA) const = 0;
   virtual bool isTlsOptimized(unsigned Type, const SymbolBody &S) const;

ELF/Target.cpp

public:
   AArch64TargetInfo();
   unsigned getGotRefReloc(unsigned Type) const override;
   unsigned getPltRefReloc(unsigned Type) const override;
   void writeGotPltEntry(uint8_t *Buf, uint64_t Plt) const override;
   void writePltZeroEntry(uint8_t *Buf, uint64_t GotEntryAddr,
                          uint64_t PltEntryAddr) const override;
   void writePltEntry(uint8_t *Buf, uint64_t GotEntryAddr,
                      uint64_t PltEntryAddr, int32_t Index) const override;
+  bool isNonzeroAddendSupported(uint32_t Type) const override;
   bool relocNeedsGot(uint32_t Type, const SymbolBody &S) const override;
   bool relocNeedsPlt(uint32_t Type, const SymbolBody &S) const override;
   void relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type, uint64_t P,
                    uint64_t SA) const override;
 };
 
 template <class ELFT> class MipsTargetInfo final : public TargetInfo {
 public:

 unsigned TargetInfo::getGotRefReloc(unsigned Type) const { return GotRefReloc; }
 
 unsigned TargetInfo::getPltRefReloc(unsigned Type) const { return PCRelReloc; }
 
 bool TargetInfo::relocPointsToGot(uint32_t Type) const { return false; }
 
 bool TargetInfo::isRelRelative(uint32_t Type) const { return true; }
 
+bool TargetInfo::isNonzeroAddendSupported(uint32_t Type) const { return true; }
+
 void TargetInfo::relocateTlsOptimize(uint8_t *Loc, uint8_t *BufEnd, uint64_t P,
                                      uint64_t SA) const {}
 
 void TargetInfo::writeGotHeaderEntries(uint8_t *Buf) const {}
 
 void TargetInfo::writeGotPltHeaderEntries(uint8_t *Buf) const {}
 
 X86TargetInfo::X86TargetInfo() {
void AArch64TargetInfo::writePltEntry(uint8_t *Buf, uint64_t GotEntryAddr,
   relocateOne(Buf, Buf + 4, R_AARCH64_ADR_PREL_PG_HI21, PltEntryAddr,
               GotEntryAddr);
   relocateOne(Buf + 4, Buf + 8, R_AARCH64_LDST64_ABS_LO12_NC, PltEntryAddr + 4,
               GotEntryAddr);
   relocateOne(Buf + 8, Buf + 12, R_AARCH64_ADD_ABS_LO12_NC, PltEntryAddr + 8,
               GotEntryAddr);
 }
 
+bool AArch64TargetInfo::isNonzeroAddendSupported(uint32_t Type) const {
+  return Type != R_AARCH64_ADR_GOT_PAGE && Type != R_AARCH64_LD64_GOT_LO12_NC;
+}
+
 bool AArch64TargetInfo::relocNeedsGot(uint32_t Type,
                                       const SymbolBody &S) const {
   return Type == R_AARCH64_ADR_GOT_PAGE || Type == R_AARCH64_LD64_GOT_LO12_NC ||
          relocNeedsPlt(Type, S);
 }
 
 bool AArch64TargetInfo::relocNeedsPlt(uint32_t Type,
                                       const SymbolBody &S) const {

test/ELF/aarch64-got-nonzero-addend-error.s

+// REQUIRES: aarch64
+// Until we support a nonzero addend for GOT relocations,
+// we have to print the corresponding error message.
+
+// RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
+// RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
+
+.global dat
+    adrp x0, :got:dat+1
+dat:
+    .word 42
+
+// CHECK: Nonzero addend is not supported for relocation R_AARCH64_ADR_GOT_PAGE