[lld] ELF: Support detection of relocation errors during processing

[wnewton]

Summary

At the moment errors in relocation processing such as out of range
values are not detected or at best trapped by asserts which will not
be present in release builds. This patch adds support for checking
error return values from applyRelocation() calls and printing an
appropriate error message. It also adds support for printing multiple
errors rather than just the first one.

[atanasyan]

lib/ReaderWriter/ELF/SectionChunks.h

426

Now parallel_for_each does all tasks in the single thread. But in the future the body of this loop can be parallelized. Does the printError ready to be called from multiple threads?

[compnerd]

Thanks for working on this, it is certainly something that we should handle better. Would it be possible for you to split this up into two patches? One introducing the error and a second actually making use of it in the AArch64 backend to perform range checks?

[shankarke]

lib/Core/Error.cpp

122 ↗

(On Diff #17761)

I think we should move this error category inside ELF.

[compnerd]

lib/Core/Error.cpp

122 ↗

(On Diff #17761)

Is there a strong reason to do so? I can see this error category being useful for the PE/COFF backend as well. There are a few relocations in the ARM target that are limited, and it would be nice to be able to return an error instead of asserting that the target is in range.

[shankarke]

lib/Core/Error.cpp

122 ↗

(On Diff #17761)

Sounds good then.

[wnewton]

1. Split out AArch64 ABS32 overflow checking code to a separate patch
2. Add a mutex to make sure error output it not interleaved when multi-threaded

[ruiu]

lib/Core/Error.cpp

123 ↗

(On Diff #17859)

Instead of adding a new error category for your purpose, I think it would be better to just use make_dynamic_error_code(). It's simpler to use.

lib/ReaderWriter/ELF/SectionChunks.h

285

This doesn't seems a correct way to use Twine. This should be

(Twine(errorStr) + "in file " ...).str()

instead of

Twine(errorStr + "in file " ...).str()

[wnewton]

Switched to using dynamic error rather than adding a new error category.
Fixed buggy usage of Twine.

[ruiu]

LGTM with these fixes.

lib/ReaderWriter/ELF/SectionChunks.h

424

s/EC/ec/

431

if (!success)

[reames]

Your concern is a legitimate one, but it's also not specific to this
patch. The JIT generally makes very little effort to report gracefully
when a compile fails. The current philosophy is more or less that the
frontend is expected to generate IR that is valid to compile; if it
doesn't, that's a fatal bug in the frontend. There is a desire to
change that, but no one has really volunteered to take that on.

Philip

lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.h

 namespace elf {
 typedef llvm::object::ELFType<llvm::support::little, 2, true> AArch64ELFType;
 
 template <class ELFT> class AArch64TargetLayout;
 
 class AArch64TargetRelocationHandler final
     : public TargetRelocationHandler<AArch64ELFType> {
 public:
-  AArch64TargetRelocationHandler(ELFLinkingContext &targetInfo)
-      : TargetRelocationHandler<AArch64ELFType>(targetInfo) {}
-
   std::error_code applyRelocation(ELFWriter &, llvm::FileOutputBuffer &,
                                   const lld::AtomLayout &,
                                   const Reference &) const override;
 
   static const Registry::KindStrings kindStrings[];
 };
 
 } // end namespace elf
 } // end namespace lld
 
 #endif // AArch64_RELOCATION_HANDLER_H

lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp

case R_AARCH64_TLSLE_ADD_TPREL_HI12:
     relocR_AARCH64_TLSLE_ADD_TPREL_HI12(location, relocVAddress, targetVAddress,
                                         ref.addend());
     break;
   case R_AARCH64_TLSLE_ADD_TPREL_LO12_NC:
     relocR_AARCH64_TLSLE_ADD_TPREL_LO12_NC(location, relocVAddress,
                                            targetVAddress, ref.addend());
     break;
   default:
-    unhandledReferenceType(*atom._atom, ref);
+    return make_unhandled_reloc_error();
   }
 
   return std::error_code();
 }

lib/ReaderWriter/ELF/AArch64/AArch64TargetHandler.cpp

 #include "AArch64TargetHandler.h"
 
 using namespace lld;
 using namespace elf;
 
 AArch64TargetHandler::AArch64TargetHandler(AArch64LinkingContext &context)
     : DefaultTargetHandler(context), _context(context),
       _AArch64TargetLayout(new AArch64TargetLayout<AArch64ELFType>(context)),
-      _AArch64RelocationHandler(
-          new AArch64TargetRelocationHandler(context)) {}
+      _AArch64RelocationHandler(new AArch64TargetRelocationHandler()) {}
 
 void AArch64TargetHandler::registerRelocationNames(Registry &registry) {
   registry.addKindTable(Reference::KindNamespace::ELF,
                         Reference::KindArch::AArch64, kindStrings);
 }
 
 std::unique_ptr<Writer> AArch64TargetHandler::getWriter() {
   switch (this->_context.getOutputELFType()) {

lib/ReaderWriter/ELF/Hexagon/HexagonRelocationHandler.h

 namespace lld {
 namespace elf {
 
 class HexagonTargetHandler;
 
 class HexagonTargetRelocationHandler final :
     public TargetRelocationHandler<HexagonELFType> {
 public:
-  HexagonTargetRelocationHandler(HexagonTargetLayout<HexagonELFType> &layout,
-                                 ELFLinkingContext &targetInfo)
-      : TargetRelocationHandler<HexagonELFType>(targetInfo),
-        _hexagonTargetLayout(layout) {}
+  HexagonTargetRelocationHandler(HexagonTargetLayout<HexagonELFType> &layout)
+      : _hexagonTargetLayout(layout) {}
 
   std::error_code applyRelocation(ELFWriter &, llvm::FileOutputBuffer &,
                                   const lld::AtomLayout &,
                                   const Reference &) const override;
 
 private:
   HexagonTargetLayout<HexagonELFType> &_hexagonTargetLayout;
 };
 } // elf
 } // lld
 #endif

lib/ReaderWriter/ELF/Hexagon/HexagonRelocationHandler.cpp

case R_HEX_GOTREL_32_6_X:
     break;
   case R_HEX_GOTREL_16_X:
   case R_HEX_GOTREL_11_X:
     relocHexGOTRELUnsigned(location, relocVAddress, targetVAddress,
                            ref.addend(), _hexagonTargetLayout.getGOTSymAddr());
     break;
 
   default:
-    unhandledReferenceType(*atom._atom, ref);
+    return make_unhandled_reloc_error();
   }
 
   return std::error_code();
 }
 
 

lib/ReaderWriter/ELF/Hexagon/HexagonTargetHandler.cpp

 
 using llvm::makeArrayRef;
 
 HexagonTargetHandler::HexagonTargetHandler(HexagonLinkingContext &context)
     : DefaultTargetHandler(context), _hexagonLinkingContext(context),
       _hexagonRuntimeFile(new HexagonRuntimeFile<HexagonELFType>(context)),
       _hexagonTargetLayout(new HexagonTargetLayout<HexagonELFType>(context)),
       _hexagonRelocationHandler(new HexagonTargetRelocationHandler(
-          *_hexagonTargetLayout.get(), context)) {}
+          *_hexagonTargetLayout.get())) {}
 
 std::unique_ptr<Writer> HexagonTargetHandler::getWriter() {
   switch (_hexagonLinkingContext.getOutputELFType()) {
   case llvm::ELF::ET_EXEC:
     return std::unique_ptr<Writer>(
         new elf::HexagonExecutableWriter<HexagonELFType>(
             _hexagonLinkingContext, *_hexagonTargetLayout.get()));
   case llvm::ELF::ET_DYN:

lib/ReaderWriter/ELF/Mips/MipsRelocationHandler.h

 namespace lld {
 namespace elf {
 
 class MipsTargetHandler;
 
 class MipsTargetRelocationHandler final
     : public TargetRelocationHandler<Mips32ElELFType> {
 public:
-  MipsTargetRelocationHandler(MipsTargetLayout<Mips32ElELFType> &layout,
-                              ELFLinkingContext &targetInfo)
-      : TargetRelocationHandler<Mips32ElELFType>(targetInfo),
-        _mipsTargetLayout(layout) {}
+  MipsTargetRelocationHandler(MipsTargetLayout<Mips32ElELFType> &layout)
+      : _mipsTargetLayout(layout) {}
 
   std::error_code applyRelocation(ELFWriter &, llvm::FileOutputBuffer &,
                                   const lld::AtomLayout &,
                                   const Reference &) const override;
 
 private:
   MipsTargetLayout<Mips32ElELFType> &_mipsTargetLayout;
 };
 
 } // elf
 } // lld
 
 #endif

lib/ReaderWriter/ELF/Mips/MipsRelocationHandler.cpp

case LLD_R_MIPS_HI16:
     break;
   case LLD_R_MIPS_LO16:
     relocLo16(ins, 0, targetVAddress, 0, false, false);
     break;
   case LLD_R_MIPS_STO_PLT:
     // Do nothing.
     break;
   default:
-    unhandledReferenceType(*atom._atom, ref);
+    return make_unhandled_reloc_error();
   }
 
   if (shuffle)
     ins = microShuffle(ins);
 
   endian::write<uint32_t, little, 2>(location, ins);
   return std::error_code();
 }

lib/ReaderWriter/ELF/Mips/MipsTargetHandler.cpp

 using namespace elf;
 
 typedef llvm::object::ELFType<llvm::support::little, 2, false> Mips32ElELFType;
 
 MipsTargetHandler::MipsTargetHandler(MipsLinkingContext &ctx)
     : DefaultTargetHandler(ctx), _ctx(ctx),
       _runtimeFile(new MipsRuntimeFile<Mips32ElELFType>(ctx)),
       _targetLayout(new MipsTargetLayout<Mips32ElELFType>(ctx)),
-      _relocationHandler(new MipsTargetRelocationHandler(*_targetLayout, ctx)) {}
+      _relocationHandler(new MipsTargetRelocationHandler(*_targetLayout)) {}
 
 std::unique_ptr<Writer> MipsTargetHandler::getWriter() {
   switch (_ctx.getOutputELFType()) {
   case llvm::ELF::ET_EXEC:
     return std::unique_ptr<Writer>(new MipsExecutableWriter<Mips32ElELFType>(
         _ctx, *_targetLayout, _elfFlagsMerger));
   case llvm::ELF::ET_DYN:
     return std::unique_ptr<Writer>(

lib/ReaderWriter/ELF/PPC/PPCTargetHandler.h

 template <class ELFT> class PPCTargetLayout : public TargetLayout<ELFT> {
 public:
   PPCTargetLayout(PPCLinkingContext &context) : TargetLayout<ELFT>(context) {}
 };
 
 class PPCTargetRelocationHandler final
     : public TargetRelocationHandler<PPCELFType> {
 public:
-  PPCTargetRelocationHandler(ELFLinkingContext &context,
-                             PPCTargetLayout<PPCELFType> &layout)
-      : TargetRelocationHandler<PPCELFType>(context),
-        _ppcTargetLayout(layout) {}
+  PPCTargetRelocationHandler(PPCTargetLayout<PPCELFType> &layout)
+      : _ppcTargetLayout(layout) {}
 
   virtual std::error_code applyRelocation(ELFWriter &, llvm::FileOutputBuffer &,
                                           const lld::AtomLayout &,
                                           const Reference &) const override;
 
 protected:
   PPCTargetLayout<PPCELFType> &_ppcTargetLayout;
 };

lib/ReaderWriter/ELF/PPC/PPCTargetHandler.cpp

if (ref.kindNamespace() != Reference::KindNamespace::ELF)
     return std::error_code();
   assert(ref.kindArch() == Reference::KindArch::PowerPC);
   switch (ref.kindValue()) {
   case R_PPC_REL24:
     relocB24PCREL(location, relocVAddress, targetVAddress, ref.addend());
     break;
 
   default:
-    unhandledReferenceType(*atom._atom, ref);
+    return make_unhandled_reloc_error();
   }
 
   return std::error_code();
 }
 
 PPCTargetHandler::PPCTargetHandler(PPCLinkingContext &context)
     : DefaultTargetHandler(context), _ppcLinkingContext(context),
       _ppcTargetLayout(new PPCTargetLayout<PPCELFType>(context)),
       _ppcRelocationHandler(
-          new PPCTargetRelocationHandler(context, *_ppcTargetLayout.get())) {}
+          new PPCTargetRelocationHandler(*_ppcTargetLayout.get())) {}
 
 void PPCTargetHandler::registerRelocationNames(Registry &registry) {
   registry.addKindTable(Reference::KindNamespace::ELF,
                         Reference::KindArch::PowerPC, kindStrings);
 }
 
 std::unique_ptr<Writer> PPCTargetHandler::getWriter() {
   switch (_ppcLinkingContext.getOutputELFType()) {

lib/ReaderWriter/ELF/SectionChunks.h

 #include "llvm/Object/ELF.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Dwarf.h"
 #include "llvm/Support/ELF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include <memory>
+#include <mutex>
 
 namespace lld {
 namespace elf {
 template <class> class OutputSection;
 using namespace llvm::ELF;
 template <class ELFT> class Segment;
 
 /// \brief An ELF section.
public:
   }
 
 protected:
   llvm::BumpPtrAllocator _alloc;
   int32_t _contentType;
   int32_t _contentPermissions;
   bool _isLoadedInMemory;
   std::vector<lld::AtomLayout *> _atoms;
+  mutable std::mutex _outputMutex;
+
+  void printError(const std::string &errorStr, const AtomLayout &atom,
+                  const Reference &ref) const {
+    StringRef kindValStr;
+    if (!this->_context.registry().referenceKindToString(ref.kindNamespace(),
+                                                         ref.kindArch(),
+                                                         ref.kindValue(),
+                                                         kindValStr)) {
+      kindValStr = "unknown";
+    }
+
+    std::string errStr = (Twine(errorStr) + " in file " +
+                          atom._atom->file().path() +
+                          ": reference from " + atom._atom->name() +
+                          "+" + Twine(ref.offsetInAtom()) +
+                          " to " + ref.target()->name() +
+                          "+" + Twine(ref.addend()) +
+                          " of type " + Twine(ref.kindValue()) +
+                          " (" + kindValStr + ")\n").str();
+
+    // Take the lock to prevent output getting interleaved between threads
+    std::lock_guard<std::mutex> lock(_outputMutex);
+    llvm::errs() << errStr;
+  }
 };
 
 /// Align the offset to the required modulus defined by the atom alignment
 template <class ELFT>
 uint64_t AtomSection<ELFT>::alignOffset(uint64_t offset,
                                         DefinedAtom::Alignment &atomAlign) {
   uint64_t requiredModulus = atomAlign.modulus;
   uint64_t alignment = 1u << atomAlign.powerOf2;
template <class ELFT> StringRef Section<ELFT>::segmentKindToStr() const {
   }
 }
 
 /// \brief Write the section and the atom contents to the buffer
 template <class ELFT>
 void AtomSection<ELFT>::write(ELFWriter *writer, TargetLayout<ELFT> &layout,
                               llvm::FileOutputBuffer &buffer) {
   uint8_t *chunkBuffer = buffer.getBufferStart();
+  bool success = true;
   parallel_for_each(_atoms.begin(), _atoms.end(), [&](lld::AtomLayout * ai) {
     DEBUG_WITH_TYPE("Section",
                     llvm::dbgs() << "Writing atom: " << ai->_atom->name()
                                  << " | " << ai->_fileOffset << "\n");
     const DefinedAtom *definedAtom = cast<DefinedAtom>(ai->_atom);
     if (!definedAtom->occupiesDiskSpace())
       return;
     // Copy raw content of atom to file buffer.
     ArrayRef<uint8_t> content = definedAtom->rawContent();
     uint64_t contentSize = content.size();
     if (contentSize == 0)
       return;
     uint8_t *atomContent = chunkBuffer + ai->_fileOffset;
     std::memcpy(atomContent, content.data(), contentSize);
     const TargetRelocationHandler<ELFT> &relHandler =
         this->_context.template getTargetHandler<ELFT>().getRelocationHandler();
-    for (const auto ref : *definedAtom)
-      relHandler.applyRelocation(*writer, buffer, *ai, *ref);
+    for (const auto ref : *definedAtom) {
+      if (std::error_code EC = relHandler.applyRelocation(*writer, buffer,
+                                                          *ai, *ref)) {
+        printError(EC.message(), *ai, *ref);
+        success = false;
+      }
+    }
   });
+  if (success == false)
+    llvm::report_fatal_error("relocating output");
 }
 
 /// \brief A OutputSection represents a set of sections grouped by the same
 /// name. The output file that gets written by the linker has sections grouped
 /// by similar names
 template <class ELFT> class OutputSection {
 public:
   // Iterators

lib/ReaderWriter/ELF/TargetHandler.h

 template <class ELFT> class ELFReference;
 class ELFWriter;
 template <class ELFT> class ELFHeader;
 template <class ELFT> class Section;
 template <class ELFT> class TargetLayout;
 
 template <class ELFT> class TargetRelocationHandler {
 public:
-  /// Constructor
-  TargetRelocationHandler(ELFLinkingContext &targetInfo)
-      : _context(targetInfo) {}
-
   virtual std::error_code applyRelocation(ELFWriter &, llvm::FileOutputBuffer &,
                                           const lld::AtomLayout &,
                                           const Reference &) const = 0;
 
-  void unhandledReferenceType(const Atom &atom, const Reference &ref) const {
-    llvm::errs() << "Unhandled reference type in file " << atom.file().path()
-                 << ": reference from " << atom.name() << "+"
-                 << ref.offsetInAtom() << " to " << ref.target()->name()
-                 << "+" << ref.addend() << " of type ";
-
-    StringRef kindValStr;
-    if (!_context.registry().referenceKindToString(ref.kindNamespace(),
-                                                   ref.kindArch(),
-                                                   ref.kindValue(),
-                                                   kindValStr)) {
-      kindValStr = "unknown";
-    }
-
-    llvm::errs() << ref.kindValue() << " (" << kindValStr << ")\n";
-    llvm::report_fatal_error("unhandled reference type");
-  }
-
   virtual ~TargetRelocationHandler() {}
-private:
-  ELFLinkingContext &_context;
 };
 
 /// \brief TargetHandler contains all the information responsible to handle a
 /// a particular target on ELF. A target might wish to override implementation
 /// of creating atoms and how the atoms are written to the output file.
 template <class ELFT> class TargetHandler : public TargetHandlerBase {
 
 public:
public:
   virtual std::unique_ptr<Reader> getDSOReader(bool) = 0;
 
   /// How does the target deal with writing ELF output.
   virtual std::unique_ptr<Writer> getWriter() = 0;
 
 private:
   ELFLinkingContext &_context;
 };
+
+inline std::error_code make_unhandled_reloc_error() {
+  return make_dynamic_error_code(Twine("Unhandled reference type"));
+}
+
+inline std::error_code make_out_of_range_reloc_error() {
+  return make_dynamic_error_code(Twine("Relocation out of range"));
+}
+
 } // end namespace elf
 } // end namespace lld
 
 #endif

lib/ReaderWriter/ELF/X86/X86RelocationHandler.h

 namespace lld {
 namespace elf {
 template <class ELFT> class X86TargetLayout;
 typedef llvm::object::ELFType<llvm::support::little, 2, false> X86ELFType;
 
 class X86TargetRelocationHandler final
     : public TargetRelocationHandler<X86ELFType> {
 public:
-  X86TargetRelocationHandler(ELFLinkingContext &targetInfo)
-      : TargetRelocationHandler<X86ELFType>(targetInfo) {}
-
   std::error_code applyRelocation(ELFWriter &, llvm::FileOutputBuffer &,
                                   const lld::AtomLayout &,
                                   const Reference &) const override;
 };
 
 } // end namespace elf
 } // end namespace lld
 
 #endif // X86_X86_RELOCATION_HANDLER_H

lib/ReaderWriter/ELF/X86/X86RelocationHandler.cpp

std::error_code X86TargetRelocationHandler::applyRelocation(
   switch (ref.kindValue()) {
   case R_386_32:
     reloc32(location, relocVAddress, targetVAddress, ref.addend());
     break;
   case R_386_PC32:
     relocPC32(location, relocVAddress, targetVAddress, ref.addend());
     break;
   default:
-    unhandledReferenceType(*atom._atom, ref);
+    return make_unhandled_reloc_error();
   }
 
   return std::error_code();
 }

lib/ReaderWriter/ELF/X86/X86TargetHandler.cpp

 void X86TargetHandler::registerRelocationNames(Registry &registry) {
   registry.addKindTable(Reference::KindNamespace::ELF, Reference::KindArch::x86,
                         kindStrings);
 }
 
 X86TargetHandler::X86TargetHandler(X86LinkingContext &context)
     : DefaultTargetHandler(context), _x86LinkingContext(context),
       _x86TargetLayout(new X86TargetLayout<X86ELFType>(context)),
-      _x86RelocationHandler(
-          new X86TargetRelocationHandler(context)) {}
+      _x86RelocationHandler(new X86TargetRelocationHandler()) {}

lib/ReaderWriter/ELF/X86_64/X86_64RelocationHandler.h

 namespace elf {
 typedef llvm::object::ELFType<llvm::support::little, 2, true> X86_64ELFType;
 
 template <class ELFT> class X86_64TargetLayout;
 
 class X86_64TargetRelocationHandler final
     : public TargetRelocationHandler<X86_64ELFType> {
 public:
-  X86_64TargetRelocationHandler(X86_64TargetLayout<X86_64ELFType> &layout,
-                                ELFLinkingContext &targetInfo)
-      : TargetRelocationHandler<X86_64ELFType>(targetInfo),
-        _tlsSize(0), _x86_64Layout(layout) {}
+  X86_64TargetRelocationHandler(X86_64TargetLayout<X86_64ELFType> &layout)
+      : _tlsSize(0), _x86_64Layout(layout) {}
 
   std::error_code applyRelocation(ELFWriter &, llvm::FileOutputBuffer &,
                                   const lld::AtomLayout &,
                                   const Reference &) const override;
 
 private:
   // Cached size of the TLS segment.
   mutable uint64_t _tlsSize;
   X86_64TargetLayout<X86_64ELFType> &_x86_64Layout;
 };
 
 } // end namespace elf
 } // end namespace lld
 
 #endif // X86_64_RELOCATION_HANDLER_H

lib/ReaderWriter/ELF/X86_64/X86_64RelocationHandler.cpp

std::error_code X86_64TargetRelocationHandler::applyRelocation(
   case R_X86_64_RELATIVE:
   case R_X86_64_IRELATIVE:
   case R_X86_64_JUMP_SLOT:
   case R_X86_64_GLOB_DAT:
   case R_X86_64_DTPMOD64:
   case R_X86_64_DTPOFF64:
     break;
   default:
-    unhandledReferenceType(*atom._atom, ref);
+    return make_unhandled_reloc_error();
   }
 
   return std::error_code();
 }

lib/ReaderWriter/ELF/X86_64/X86_64TargetHandler.cpp

 
 using namespace lld;
 using namespace elf;
 
 X86_64TargetHandler::X86_64TargetHandler(X86_64LinkingContext &context)
     : DefaultTargetHandler(context), _context(context),
       _x86_64TargetLayout(new X86_64TargetLayout<X86_64ELFType>(context)),
       _x86_64RelocationHandler(
-          new X86_64TargetRelocationHandler(*_x86_64TargetLayout.get(),
-                                            context)) {}
+          new X86_64TargetRelocationHandler(*_x86_64TargetLayout.get())) {}
 
 void X86_64TargetHandler::registerRelocationNames(Registry &registry) {
   registry.addKindTable(Reference::KindNamespace::ELF,
                         Reference::KindArch::x86_64, kindStrings);
 }
 
 std::unique_ptr<Writer> X86_64TargetHandler::getWriter() {
   switch (this->_context.getOutputELFType()) {