[lld] 06f863a - [lld-macho] Include address offsets in error messages

Jez Ng via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 7 18:06:30 PST 2022


Author: Jez Ng
Date: 2022-02-07T21:06:18-05:00
New Revision: 06f863ac5eb5d1749d70c4870669abd9719b1696

URL: https://github.com/llvm/llvm-project/commit/06f863ac5eb5d1749d70c4870669abd9719b1696
DIFF: https://github.com/llvm/llvm-project/commit/06f863ac5eb5d1749d70c4870669abd9719b1696.diff

LOG: [lld-macho] Include address offsets in error messages

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

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

Reviewed By: #lld-macho, oontvoo

Differential Revision: https://reviews.llvm.org/D118798

Added: 
    

Modified: 
    lld/MachO/InputFiles.cpp
    lld/MachO/InputSection.cpp
    lld/MachO/InputSection.h
    lld/MachO/Relocations.cpp
    lld/MachO/UnwindInfoSection.cpp
    lld/test/MachO/invalid/bad-got-to-dylib-tlv-reference.s
    lld/test/MachO/invalid/bad-got-to-tlv-reference.s
    lld/test/MachO/invalid/bad-tlv-relocation.s
    lld/test/MachO/invalid/compact-unwind-bad-reloc.s
    lld/test/MachO/invalid/cstring-dedup.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 13317ca956e78..aa3567b778b79 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -996,8 +996,8 @@ void ObjFile::registerCompactUnwind() {
             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.

diff  --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index a2213f8325c0c..439b9fdd5f012 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -55,6 +55,28 @@ static uint64_t resolveSymbolVA(const Symbol *sym, uint8_t type) {
   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 {
@@ -195,7 +217,7 @@ void CStringInputSection::splitIntoPieces() {
   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);

diff  --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 52f851de0e5aa..4d716e60caa49 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -49,6 +49,8 @@ class InputSection {
   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;

diff  --git a/lld/MachO/Relocations.cpp b/lld/MachO/Relocations.cpp
index 2f316154a1cab..a711157679c14 100644
--- a/lld/MachO/Relocations.cpp
+++ b/lld/MachO/Relocations.cpp
@@ -24,15 +24,15 @@ 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;

diff  --git a/lld/MachO/UnwindInfoSection.cpp b/lld/MachO/UnwindInfoSection.cpp
index 8b1e357499aad..b9dc7c13a0288 100644
--- a/lld/MachO/UnwindInfoSection.cpp
+++ b/lld/MachO/UnwindInfoSection.cpp
@@ -279,11 +279,12 @@ void UnwindInfoSectionImpl<Ptr>::prepareRelocations(ConcatInputSection *isec) {
 // 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);
 }
 
@@ -311,7 +312,7 @@ void UnwindInfoSectionImpl<Ptr>::relocateCompactUnwind(
       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.
@@ -319,7 +320,7 @@ void UnwindInfoSectionImpl<Ptr>::relocateCompactUnwind(
         }
       } 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);

diff  --git a/lld/test/MachO/invalid/bad-got-to-dylib-tlv-reference.s b/lld/test/MachO/invalid/bad-got-to-dylib-tlv-reference.s
index 85a077e4a35e7..463a044465172 100644
--- a/lld/test/MachO/invalid/bad-got-to-dylib-tlv-reference.s
+++ b/lld/test/MachO/invalid/bad-got-to-dylib-tlv-reference.s
@@ -8,7 +8,7 @@
 # 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

diff  --git a/lld/test/MachO/invalid/bad-got-to-tlv-reference.s b/lld/test/MachO/invalid/bad-got-to-tlv-reference.s
index 815a3de0df5b7..8934bc892a474 100644
--- a/lld/test/MachO/invalid/bad-got-to-tlv-reference.s
+++ b/lld/test/MachO/invalid/bad-got-to-tlv-reference.s
@@ -2,7 +2,7 @@
 # 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

diff  --git a/lld/test/MachO/invalid/bad-tlv-relocation.s b/lld/test/MachO/invalid/bad-tlv-relocation.s
index 6c3489d0d0ce9..23890131a6828 100644
--- a/lld/test/MachO/invalid/bad-tlv-relocation.s
+++ b/lld/test/MachO/invalid/bad-tlv-relocation.s
@@ -2,7 +2,7 @@
 # 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

diff  --git a/lld/test/MachO/invalid/compact-unwind-bad-reloc.s b/lld/test/MachO/invalid/compact-unwind-bad-reloc.s
index b6b6c36ccfac2..dca668c9487e6 100644
--- a/lld/test/MachO/invalid/compact-unwind-bad-reloc.s
+++ b/lld/test/MachO/invalid/compact-unwind-bad-reloc.s
@@ -2,9 +2,9 @@
 # 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

diff  --git a/lld/test/MachO/invalid/cstring-dedup.s b/lld/test/MachO/invalid/cstring-dedup.s
index 287f7d2156a31..1b73be36eeaca 100644
--- a/lld/test/MachO/invalid/cstring-dedup.s
+++ b/lld/test/MachO/invalid/cstring-dedup.s
@@ -10,7 +10,7 @@
 # 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


        


More information about the llvm-commits mailing list