[lld] a723db9 - [lld-macho][nfc] Refactor subtractor reloc handling

[Jez Ng via llvm-commits]

Author: Jez Ng
Date: 2021-03-11T13:28:13-05:00
New Revision: a723db92d87d098d564ac8ace8b1d24bb02a9fdb

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

LOG: [lld-macho][nfc] Refactor subtractor reloc handling

SUBTRACTOR relocations are always paired with UNSIGNED
relocations to indicate a pair of symbols whose address difference we
want. Functionally they are like a single relocation: only one pointer
gets written / relocated. Previously, we would handle these pairs by
skipping over the SUBTRACTOR relocation and writing the pointer when
handling the UNSIGNED reloc. This diff reverses things, so we write
while handling SUBTRACTORs and skip over the UNSIGNED relocs instead.

Being able to distinguish between SUBTRACTOR and UNSIGNED relocs in the
write phase (i.e. inside `relocateOne`) is useful for the upcoming range
check diff: we want to check that SUBTRACTOR relocs write signed values,
but UNSIGNED relocs (naturally) write unsigned values.

Reviewed By: #lld-macho, thakis

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

Added: 
    

Modified: 
    lld/MachO/Arch/ARM64.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/InputSection.cpp
    lld/test/MachO/reloc-subtractor.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Arch/ARM64.cpp b/lld/MachO/Arch/ARM64.cpp
index 7419046c8e0b..0d1e2f447adc 100644
--- a/lld/MachO/Arch/ARM64.cpp
+++ b/lld/MachO/Arch/ARM64.cpp
@@ -79,7 +79,8 @@ const RelocAttrs &ARM64::getRelocAttrs(uint8_t type) const {
 
 uint64_t ARM64::getEmbeddedAddend(MemoryBufferRef mb, const section_64 &sec,
                                   const relocation_info rel) const {
-  if (rel.r_type != ARM64_RELOC_UNSIGNED) {
+  if (rel.r_type != ARM64_RELOC_UNSIGNED &&
+      rel.r_type != ARM64_RELOC_SUBTRACTOR) {
     // All other reloc types should use the ADDEND relocation to store their
     // addends.
     // TODO(gkm): extract embedded addend just so we can assert that it is 0
@@ -159,6 +160,7 @@ void ARM64::relocateOne(uint8_t *loc, const Reloc &r, uint64_t value,
   case ARM64_RELOC_BRANCH26:
     value = encodeBranch26(base, value - pc);
     break;
+  case ARM64_RELOC_SUBTRACTOR:
   case ARM64_RELOC_UNSIGNED:
     break;
   case ARM64_RELOC_POINTER_TO_GOT:

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index f986818d14f7..32ef5b46730a 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -276,16 +276,6 @@ void ObjFile::parseRelocations(const section_64 &sec,
     if (relInfo.r_address & R_SCATTERED)
       fatal("TODO: Scattered relocations not supported");
 
-    Reloc p;
-    if (target->hasAttr(relInfo.r_type, RelocAttrBits::SUBTRAHEND)) {
-      p.type = relInfo.r_type;
-      p.referent = symbols[relInfo.r_symbolnum];
-      relInfo = relInfos[++i];
-      // SUBTRACTOR relocations should always be followed by an UNSIGNED one
-      // indicating the minuend symbol.
-      assert(target->hasAttr(relInfo.r_type, RelocAttrBits::UNSIGNED) &&
-             relInfo.r_extern);
-    }
     uint64_t embeddedAddend = target->getEmbeddedAddend(mb, sec, relInfo);
     assert(!(embeddedAddend && pairedAddend));
     uint64_t totalAddend = pairedAddend + embeddedAddend;
@@ -320,9 +310,19 @@ void ObjFile::parseRelocations(const section_64 &sec,
     }
 
     InputSection *subsec = findContainingSubsection(subsecMap, &r.offset);
-    if (p.type != GENERIC_RELOC_INVALID)
-      subsec->relocs.push_back(p);
     subsec->relocs.push_back(r);
+
+    if (target->hasAttr(r.type, RelocAttrBits::SUBTRAHEND)) {
+      relInfo = relInfos[++i];
+      // SUBTRACTOR relocations should always be followed by an UNSIGNED one
+      // indicating the minuend symbol.
+      assert(target->hasAttr(relInfo.r_type, RelocAttrBits::UNSIGNED) &&
+             relInfo.r_extern);
+      Reloc p;
+      p.type = relInfo.r_type;
+      p.referent = symbols[relInfo.r_symbolnum];
+      subsec->relocs.push_back(p);
+    }
   }
 }
 

diff  --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index a54d83618bb0..03d2fc54a596 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -58,15 +58,12 @@ void InputSection::writeTo(uint8_t *buf) {
   memcpy(buf, data.data(), data.size());
 
   for (size_t i = 0; i < relocs.size(); i++) {
-    const Symbol *fromSym =
-        target->hasAttr(relocs[i].type, RelocAttrBits::SUBTRAHEND)
-            ? relocs[i++].referent.get<Symbol *>()
-            : nullptr;
     const Reloc &r = relocs[i];
     uint8_t *loc = buf + r.offset;
     uint64_t referentVA = 0;
-    if (fromSym) {
-      const Symbol *toSym = r.referent.get<Symbol *>();
+    if (target->hasAttr(r.type, RelocAttrBits::SUBTRAHEND)) {
+      const Symbol *fromSym = r.referent.get<Symbol *>();
+      const Symbol *toSym = relocs[++i].referent.get<Symbol *>();
       referentVA = toSym->getVA() - fromSym->getVA();
     } else if (auto *referentSym = r.referent.dyn_cast<Symbol *>()) {
       if (target->hasAttr(r.type, RelocAttrBits::LOAD) &&

diff  --git a/lld/test/MachO/reloc-subtractor.s b/lld/test/MachO/reloc-subtractor.s
index dead12f06766..d426adc60bf2 100644
--- a/lld/test/MachO/reloc-subtractor.s
+++ b/lld/test/MachO/reloc-subtractor.s
@@ -21,7 +21,7 @@
 # CHECK-NEXT:  segment  section            address     type
 # CHECK-EMPTY:
 
-.globl _main, _subtrahend_1, _subtrahend_2, _minued1, _minued2
+.globl _main, _subtrahend_1, _subtrahend_2, _minuend1, _minuend2
 
 .data
 _subtrahend_1: