[lld-macho] Include address offsets in error messages

[int3]

Summary

This makes it easier to pinpoint the source of the problem.

TODO: Have more relocation error messages make use of this
functionality.

[oontvoo]

Nice! LG

lld/MachO/InputFiles.cpp

for (auto it = isec->relocs.begin(); it != isec->relocs.end();) {
         }
         add += sym->value;
         referentIsec = cast<ConcatInputSection>(sym->isec);
       } else {
         referentIsec =
             cast<ConcatInputSection>(r.referent.dyn_cast<InputSection *>());
       }
       if (referentIsec->getSegName() != segment_names::text)
-        error("compact unwind references address in " + toString(referentIsec) +
-              " which is not in segment __TEXT");
+        error(isec->getLocation(r.offset) + " references section " +
+              referentIsec->getName() + " which is not in segment __TEXT");
       // The functionAddress relocations are typically section relocations.
       // However, unwind info operates on a per-symbol basis, so we search for
       // the function symbol here.
       auto symIt = llvm::lower_bound(
           referentIsec->symbols, add,
           [](Defined *d, uint64_t add) { return d->value < add; });
       // The relocation should point at the exact address of a symbol (with no
       // addend).

lld/MachO/InputSection.h

public:
   StringRef getSegName() const { return section.segname; }
   uint32_t getFlags() const { return section.flags; }
   uint64_t getFileSize() const;
   // Translates \p off -- an offset relative to this InputSection -- into an
   // offset from the beginning of its parent OutputSection.
   virtual uint64_t getOffset(uint64_t off) const = 0;
   // The offset from the beginning of the file.
   uint64_t getVA(uint64_t off) const;
+  // Return a user-friendly string for use in diagnostics.
+  std::string getLocation(uint64_t off) const;
   // Whether the data at \p off in this InputSection is live.
   virtual bool isLive(uint64_t off) const = 0;
   virtual void markLive(uint64_t off) = 0;
   virtual InputSection *canonical() { return this; }
   virtual const InputSection *canonical() const { return this; }
 
 protected:
   InputSection(Kind kind, const Section &section, ArrayRef<uint8_t> data,

lld/MachO/InputSection.cpp

if (relocAttrs.hasAttr(RelocAttrBits::BRANCH))
     return sym->resolveBranchVA();
   if (relocAttrs.hasAttr(RelocAttrBits::GOT))
     return sym->resolveGotVA();
   if (relocAttrs.hasAttr(RelocAttrBits::TLV))
     return sym->resolveTlvVA();
   return sym->getVA();
 }
 
+std::string InputSection::getLocation(uint64_t off) const {
+  // First, try to find a symbol that's near the offset. Use it as a reference
+  // point.
+  for (size_t i = 0; i < symbols.size(); ++i)
+    if (symbols[i]->value <= off &&
+        (i + 1 == symbols.size() || symbols[i + 1]->value > off))
+      return (toString(getFile()) + ":(symbol " + symbols.front()->getName() +
+              "+0x" + Twine::utohexstr(off - symbols[i]->value) + ")")
+          .str();
+
+  // If that fails, use the section itself as a reference point.
+  for (const Subsection &subsec : section.subsections) {
+    if (subsec.isec == this) {
+      off += subsec.offset;
+      break;
+    }
+  }
+  return (toString(getFile()) + ":(" + getName() + "+0x" +
+          Twine::utohexstr(off) + ")")
+      .str();
+}
+
 // ICF needs to hash any section that might potentially be duplicated so
 // that it can match on content rather than identity.
 bool ConcatInputSection::isHashableForICF() const {
   switch (sectionType(getFlags())) {
   case S_REGULAR:
     return true;
   case S_CSTRING_LITERALS:
   case S_4BYTE_LITERALS:

 }
 
 void CStringInputSection::splitIntoPieces() {
   size_t off = 0;
   StringRef s = toStringRef(data);
   while (!s.empty()) {
     size_t end = s.find(0);
     if (end == StringRef::npos)
-      fatal(toString(this) + ": string is not null terminated");
+      fatal(getLocation(off) + ": string is not null terminated");
     size_t size = end + 1;
     uint32_t hash = config->dedupLiterals ? xxHash64(s.substr(0, size)) : 0;
     pieces.emplace_back(off, hash);
     s = s.substr(size);
     off += size;
   }
 }
 

lld/MachO/Relocations.cpp

 
 static_assert(sizeof(void *) != 8 || sizeof(Reloc) == 24,
               "Try to minimize Reloc's size; we create many instances");
 
 bool macho::validateSymbolRelocation(const Symbol *sym,
                                      const InputSection *isec, const Reloc &r) {
   const RelocAttrs &relocAttrs = target->getRelocAttrs(r.type);
   bool valid = true;
-  auto message = [relocAttrs, sym, isec, &valid](const Twine &diagnostic) {
+  auto message = [&](const Twine &diagnostic) {
     valid = false;
-    return (relocAttrs.name + " relocation " + diagnostic + " for `" +
-            sym->getName() + "' in " + toString(isec))
+    return (isec->getLocation(r.offset) + ": " + relocAttrs.name +
+            " relocation " + diagnostic)
         .str();
   };
 
   if (relocAttrs.hasAttr(RelocAttrBits::TLV) != sym->isTlv())
-    error(message(Twine("requires that variable ") +
+    error(message(Twine("requires that symbol ") + sym->getName() + " " +
                   (sym->isTlv() ? "not " : "") + "be thread-local"));
 
   return valid;
 }
 
 void macho::reportRangeError(const Reloc &r, const Twine &v, uint8_t bits,
                              int64_t min, uint64_t max) {
   std::string hint;

lld/MachO/UnwindInfoSection.cpp

for (size_t i = 0; i < isec->relocs.size(); ++i) {
     }
   }
 }
 
 // Unwind info lives in __DATA, and finalization of __TEXT will occur before
 // finalization of __DATA. Moreover, the finalization of unwind info depends on
 // the exact addresses that it references. So it is safe for compact unwind to
 // reference addresses in __TEXT, but not addresses in any other segment.
-static ConcatInputSection *checkTextSegment(InputSection *isec) {
+static ConcatInputSection *
+checkTextSegment(InputSection *isec, InputSection *cuIsec, uint64_t off) {
   if (isec->getSegName() != segment_names::text)
-    error("compact unwind references address in " + toString(isec) +
+    error(cuIsec->getLocation(off) + " references section " + isec->getName() +
           " which is not in segment __TEXT");
-  // __text should always be a ConcatInputSection.
+  // __TEXT should always contain ConcatInputSections.
   return cast<ConcatInputSection>(isec);
 }
 
 // We need to apply the relocations to the pre-link compact unwind section
 // before converting it to post-link form. There should only be absolute
 // relocations here: since we are not emitting the pre-link CU section, there
 // is no source address to make a relative location meaningful.
 template <class Ptr>
parallelForEachN(0, symbolsVec.size(), [&](size_t i) {
     // Write the rest of the CUE.
     memcpy(buf + sizeof(Ptr), d->unwindEntry->data.data(),
            d->unwindEntry->data.size());
     for (const Reloc &r : d->unwindEntry->relocs) {
       uint64_t referentVA = 0;
       if (auto *referentSym = r.referent.dyn_cast<Symbol *>()) {
         if (!isa<Undefined>(referentSym)) {
           if (auto *defined = dyn_cast<Defined>(referentSym))
-            checkTextSegment(defined->isec);
+            checkTextSegment(defined->isec, d->unwindEntry, r.offset);
           // At this point in the link, we may not yet know the final address of
           // the GOT, so we just encode the index. We make it a 1-based index so
           // that we can distinguish the null pointer case.
           referentVA = referentSym->gotIndex + 1;
         }
       } else {
         auto *referentIsec = r.referent.get<InputSection *>();
-        checkTextSegment(referentIsec);
+        checkTextSegment(referentIsec, d->unwindEntry, r.offset);
         referentVA = referentIsec->getVA(r.addend);
       }
       writeAddress(buf + r.offset, referentVA, r.length);
     }
   });
 }
 
 // There should only be a handful of unique personality pointers, so we can

lld/test/MachO/invalid/bad-got-to-dylib-tlv-reference.s

 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libtlv.s -o %t/libtlv.o
 # RUN: %lld -dylib -install_name @executable_path/libtlv.dylib \
 # RUN:   -lSystem -o %t/libtlv.dylib %t/libtlv.o
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
 # RUN: not %lld -lSystem -L%t -ltlv -o /dev/null %t/test.o 2>&1 | FileCheck %s -DFILE=%t/test.o
 
-# CHECK: error: GOT_LOAD relocation requires that variable not be thread-local for `_foo' in [[FILE]]:(__text)
+# CHECK: error: [[FILE]]:(symbol _main+0x3): GOT_LOAD relocation requires that symbol _foo not be thread-local
 
 #--- libtlv.s
 .section __DATA,__thread_vars,thread_local_variables
 .globl _foo
 _foo:
 
 #--- test.s
 .text
 .globl _main
 _main:
   movq _foo@GOTPCREL(%rip), %rax
   ret

lld/test/MachO/invalid/bad-got-to-tlv-reference.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: not %lld -o /dev/null %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 
-# CHECK: error: GOT_LOAD relocation requires that variable not be thread-local for `_foo' in [[FILE]]:(__text)
+# CHECK: error: [[FILE]]:(symbol _main+0x3): GOT_LOAD relocation requires that symbol _foo not be thread-local
 
 .text
 .globl _main
 _main:
   movq _foo@GOTPCREL(%rip), %rax
   ret
 
 .section __DATA,__thread_vars,thread_local_variables
 _foo:

lld/test/MachO/invalid/bad-tlv-relocation.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: not %lld -o /dev/null %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 
-# CHECK: TLV relocation requires that variable be thread-local for `_foo' in [[FILE]]:(__text)
+# CHECK: [[FILE]]:(symbol _main+0x3): TLV relocation requires that symbol _foo be thread-local
 
 .text
 .globl _main
 _main:
   leaq _foo@TLVP(%rip), %rax
   ret
 
 .data
 _foo:

lld/test/MachO/invalid/compact-unwind-bad-reloc.s

 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/bad-function.s -o %t/bad-function.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/bad-personality.s -o %t/bad-personality.o
-# RUN: not %lld -lSystem -lc++ %t/bad-function.o -o %t 2>&1 | FileCheck %s -DFILE=%t/bad-function.o
-# RUN: not %lld -lSystem -lc++ %t/bad-personality.o -o %t 2>&1 | FileCheck %s -DFILE=%t/bad-personality.o
-# CHECK: error: compact unwind references address in [[FILE]]:(__data) which is not in segment __TEXT
+# RUN: not %lld -lSystem -lc++ %t/bad-function.o -o %t 2>&1 | FileCheck %s -DFILE=%t/bad-function.o -D#OFF=0x0
+# RUN: not %lld -lSystem -lc++ %t/bad-personality.o -o %t 2>&1 | FileCheck %s -DFILE=%t/bad-personality.o -D#OFF=0x10
+# CHECK: error: [[FILE]]:(__compact_unwind+0x[[#%x,OFF]]) references section __data which is not in segment __TEXT
 
 #--- bad-function.s
 .data
 _not_a_function:
   .cfi_startproc
   .cfi_personality 155, ___gxx_personality_v0
   .cfi_def_cfa_offset 16
   retq

lld/test/MachO/invalid/cstring-dedup.s

 ## We're intentionally testing fatal errors (for malformed input files), and
 ## fatal errors aren't supported for testing when main is run twice.
 # XFAIL: main-run-twice
 
 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/not-terminated.s -o %t/not-terminated.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/relocs.s -o %t/relocs.o
 
 # RUN: not %lld -dylib --deduplicate-literals %t/not-terminated.o 2>&1 | FileCheck %s --check-prefix=TERM
 # RUN: not %lld -dylib --deduplicate-literals %t/relocs.o 2>&1 | FileCheck %s --check-prefix=RELOCS
 
-# TERM:   not-terminated.o:(__cstring): string is not null terminated
+# TERM:   not-terminated.o:(__cstring+0x4): string is not null terminated
 # RELOCS: relocs.o contains relocations in __TEXT,__cstring, so LLD cannot deduplicate literals. Try re-running without --deduplicate-literals.
 
 #--- not-terminated.s
 .cstring
 .asciz "foo"
 .ascii "oh no"
 
 #--- relocs.s
 .cstring
 _str:
 .asciz "foo"
 .quad _str