[PATCH] D98386: [lld-macho][nfc] Refactor subtractor reloc handling

[Jez Ng via Phabricator via llvm-commits]

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added a project: lld-macho.
int3 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98386

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


Index: lld/test/MachO/reloc-subtractor.s
===================================================================
--- lld/test/MachO/reloc-subtractor.s
+++ 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:
Index: lld/MachO/InputSection.cpp
===================================================================
--- lld/MachO/InputSection.cpp
+++ lld/MachO/InputSection.cpp
@@ -58,14 +58,12 @@
   memcpy(buf, data.data(), data.size());
 
   for (size_t i = 0; i < relocs.size(); i++) {
-    auto *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) {
-      auto *toSym = r.referent.get<Symbol *>();
+    if (target->hasAttr(r.type, RelocAttrBits::SUBTRAHEND)) {
+      auto *fromSym = r.referent.get<Symbol *>();
+      auto *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) &&
Index: lld/MachO/InputFiles.cpp
===================================================================
--- lld/MachO/InputFiles.cpp
+++ lld/MachO/InputFiles.cpp
@@ -276,16 +276,6 @@
     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 @@
     }
 
     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);
+    }
   }
 }
 
Index: lld/MachO/Arch/ARM64.cpp
===================================================================
--- lld/MachO/Arch/ARM64.cpp
+++ lld/MachO/Arch/ARM64.cpp
@@ -79,7 +79,8 @@
 
 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 @@
   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:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98386.329829.patch
Type: text/x-patch
Size: 3863 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210311/085768c3/attachment.bin>