[ELF] Define a reportRangeError() overload for thunks and tidy up recent PPC64 thunk range errors

[MaskRay]

Summary

Prefer errorOrWarn to fatal for recoverable errors and graceful degradation
when --noinhibit-exec is specified.

Mention the destination symbol, otherwise the diagnostic is not really actionable.
Two errors are not tested but the patch does not intend to add the coverage.

[MaskRay]

Remove an unneeded change

[grimar]

lld/ELF/Relocations.cpp

116

I wonder if it should be a local helper function in Thunks.cpp, as it does not used anywhere else?

lld/ELF/Thunks.cpp

900

Forgot to remove the commented line?

lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s

13

Perhaps add --noinhibit-exec case, since the logic was changed to support it?

[MaskRay]

Address comments

lld/ELF/Relocations.cpp

116

getDefinedLocation(sym) is a static function. Defining the helper in Thunks.cpp would replicate getDefinedLocation(sym)

I think it is fine to define the overloads in the same place to keep the style consistent.

[grimar]

LGTM

[stefanp]

lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s

16

Question:
What does this do? There is no FileCheck after it and you are sending the output to /dev/null so what output file is retained?

[MaskRay]

lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s

16

--noinhibit-exec makes errorOrWarn report a warning, so the output is not inhibited and lld can still return 0.

lld/ELF/Relocations.cpp

if (errPlace.isec && errPlace.isec->name.startswith(".debug"))
     hint += "; consider recompiling with -fdebug-types-section to reduce size "
             "of debug sections";
 
   errorOrWarn(errPlace.loc + "relocation " + lld::toString(rel.type) +
               " out of range: " + v.str() + " is not in [" + Twine(min).str() +
               ", " + Twine(max).str() + "]" + hint);
 }
 
-void elf::checkRangeError(uint8_t *loc, int64_t v, int n, const Symbol &sym,
+void elf::reportRangeError(uint8_t *loc, int64_t v, int n, const Symbol &sym,
                            const Twine &msg) {
   ErrorPlace errPlace = getErrorPlace(loc);
   std::string hint;
-  if (!sym.isLocal())
+  if (!sym.getName().empty())
     hint = "; references " + lld::toString(sym) + getDefinedLocation(sym);
-  errorOrWarn(errPlace.loc + msg + " out of range: " + Twine(v) +
+  errorOrWarn(errPlace.loc + msg + " is out of range: " + Twine(v) +
               " is not in [" + Twine(llvm::minIntN(n)) + ", " +
               Twine(llvm::maxIntN(n)) + "]" + hint);
 }
 
 namespace {
 // Build a bitmask with one bit set for each RelExpr.
 //
 // Constexpr function arguments can't be used in static asserts, so we

lld/ELF/Target.h

 
 extern const TargetInfo *target;
 TargetInfo *getTarget();
 
 template <class ELFT> bool isMipsPIC(const Defined *sym);
 
 void reportRangeError(uint8_t *loc, const Relocation &rel, const Twine &v,
                       int64_t min, uint64_t max);
+void reportRangeError(uint8_t *loc, int64_t v, int n, const Symbol &sym,
+                      const Twine &msg);
 
 // Make sure that V can be represented as an N bit signed integer.
 inline void checkInt(uint8_t *loc, int64_t v, int n, const Relocation &rel) {
   if (v != llvm::SignExtend64(v, n))
     reportRangeError(loc, rel, Twine(v), llvm::minIntN(n), llvm::maxIntN(n));
 }
 
 // Make sure that V can be represented as an N bit unsigned integer.

 inline void checkAlignment(uint8_t *loc, uint64_t v, int n,
                            const Relocation &rel) {
   if ((v & (n - 1)) != 0)
     error(getErrorLocation(loc) + "improper alignment for relocation " +
           lld::toString(rel.type) + ": 0x" + llvm::utohexstr(v) +
           " is not aligned to " + Twine(n) + " bytes");
 }
 
-void checkRangeError(uint8_t *loc, int64_t v, int n, const Symbol &sym, const Twine &msg);
-
 // Endianness-aware read/write.
 inline uint16_t read16(const void *p) {
   return llvm::support::endian::read16(p, config->endianness);
 }
 
 inline uint32_t read32(const void *p) {
   return llvm::support::endian::read32(p, config->endianness);
 }

lld/ELF/Thunks.cpp

bool PPC64PltCallStub::isCompatibleWith(const InputSection &isec,
                                         const Relocation &rel) const {
   return rel.type == R_PPC64_REL24 || rel.type == R_PPC64_REL14;
 }
 
 void PPC64R2SaveStub::writeTo(uint8_t *buf) {
   int64_t offset = destination.getVA() - (getThunkTargetSym()->getVA() + 4);
   // The branch offset needs to fit in 26 bits.
   if (!isInt<26>(offset))
-    fatal("R2 save stub branch offset is too large: " + Twine(offset));
+    reportRangeError(buf, offset, 26, destination, "R2 save stub offset");
+  // fatal("R2 save stub branch offset is too large: " + Twine(offset));
   write32(buf + 0, 0xf8410018);                         // std  r2,24(r1)
   write32(buf + 4, 0x48000000 | (offset & 0x03fffffc)); // b    <offset>
 }
 
 void PPC64R2SaveStub::addSymbols(ThunkSection &isec) {
   Defined *s = addSymbol(saver.save("__toc_save_" + destination.getName()),
                          STT_FUNC, 0, isec);
   s->needsTocRestore = true;
 }
 
 void PPC64R12SetupStub::writeTo(uint8_t *buf) {
   int64_t offset = destination.getVA() - getThunkTargetSym()->getVA();
   if (!isInt<34>(offset))
-    fatal("offset must fit in 34 bits to encode in the instruction");
+    reportRangeError(buf, offset, 34, destination, "R12 setup stub offset");
   uint64_t paddi = PADDI_R12_NO_DISP | (((offset >> 16) & 0x3ffff) << 32) |
                    (offset & 0xffff);
 
   writePrefixedInstruction(buf + 0, paddi); // paddi r12, 0, func@pcrel, 1
   write32(buf + 8, MTCTR_R12);              // mtctr r12
   write32(buf + 12, BCTR);                  // bctr
 }
 
 void PPC64R12SetupStub::addSymbols(ThunkSection &isec) {
   addSymbol(saver.save("__gep_setup_" + destination.getName()), STT_FUNC, 0,
             isec);
 }
 
 void PPC64PCRelPLTStub::writeTo(uint8_t *buf) {
   int64_t offset = destination.getGotPltVA() - getThunkTargetSym()->getVA();
   if (!isInt<34>(offset))
-    fatal("offset must fit in 34 bits to encode in the instruction");
+    reportRangeError(buf, offset, 34, destination,
+                     "PC-relative PLT stub offset");
   uint64_t pld =
       PLD_R12_NO_DISP | (((offset >> 16) & 0x3ffff) << 32) | (offset & 0xffff);
 
   writePrefixedInstruction(buf + 0, pld); // pld r12, func@plt@pcrel
   write32(buf + 8, MTCTR_R12);            // mtctr r12
   write32(buf + 12, BCTR);                // bctr
 }
 

lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s

 # REQUIRES: ppc
 # RUN: echo 'SECTIONS { \
 # RUN:       .text_callee 0x10010000 : { *(.text_callee) } \
 # RUN:       .text_caller 0x20020000 : { *(.text_caller) } \
 # RUN:       }' > %t.script
 
 # RUN: llvm-mc -filetype=obj -triple=powerpc64le %s -o %t.o
 # RUN: not ld.lld -T %t.script %t.o -o /dev/null 2>&1 | FileCheck %s
 
 # RUN: llvm-mc -filetype=obj -triple=powerpc64 %s -o %t.o
 # RUN: not ld.lld -T %t.script %t.o -o /dev/null 2>&1 | FileCheck %s
 
-# CHECK: error: R2 save stub branch offset is too large: -268501028
+# CHECK: error: R2 save stub offset is out of range: -268501028 is not in [-33554432, 33554431]; references callee
 
 .section .text_callee, "ax", %progbits
 callee:
   .localentry callee, 1
   blr
 
 .section .text_caller, "ax", %progbits
 caller:
 .Lfunc_gep1:
   addis 2, 12, .TOC.-.Lfunc_gep1@ha
   addi 2, 2, .TOC.-.Lfunc_gep1@l