[NFCI][ELF][CHERI] Restructure addCapabilityRelocation for clarity

Author: jrtc27

lld/ELF/Arch/Cheri.cpp

@@ -916,12 +916,6 @@ void addCapabilityRelocation(
          << ">>> referenced by " << sec->getObjMsg(offset);
   }
 
-  // Emit either the legacy __cap_relocs section or a R_CHERI_CAPABILITY reloc
-  // For local symbols we can also emit the untagged capability bits and
-  // instruct csu/rtld to run CBuildCap
-  CapRelocsMode capRelocMode = sym && sym->isPreemptible
-                                   ? CapRelocsMode::ElfReloc
-                                   : ctx.arg.localCapRelocsMode;
   bool needTrampoline = false;
   // In the PLT ABI (and fndesc?) we have to use an elf relocation for function
   // pointers to ensure that the runtime linker adds the required trampolines
@@ -944,81 +938,78 @@ void addCapabilityRelocation(
         needTrampoline = true;
     }
 
-    if (needTrampoline) {
-      capRelocMode = CapRelocsMode::ElfReloc;
-      if (ctx.arg.verboseCapRelocs)
-        message("Using trampoline for function pointer against " +
-                verboseToString(ctx, sym));
-    }
+    if (needTrampoline && ctx.arg.verboseCapRelocs)
+      message("Using trampoline for function pointer against " +
+              verboseToString(ctx, sym));
   }
 
   // Non-preemptible undef weak symbols are link-time constants and should use
   // addNullDerivedCapability
   if (sym)
     assert(sym->isPreemptible || !sym->isUndefWeak());
 
-  // local cap relocs don't need a Elf relocation with a full symbol lookup:
-  if (capRelocMode == CapRelocsMode::ElfReloc) {
-    assert(sym && "ELF relocs should not be used against sections");
-    assert((sym->isPreemptible || needTrampoline) &&
-           "ELF relocs should not be used for non-preemptible symbols");
-    assert((!sym->isLocal() || needTrampoline) &&
-           "ELF relocs should not be used for local symbols");
-    if (ctx.arg.emachine == llvm::ELF::EM_MIPS && ctx.arg.buildingFreeBSDRtld) {
-      Err(ctx) << "relocation " << type << " against " << verboseToString(ctx, sym)
-               << " cannot be using when building FreeBSD RTLD"
-               << referencedBy();
-      return;
-    }
-    if (!lld::elf::hasDynamicLinker(ctx)) {
-      auto diag = Err(ctx);
-      diag << "attempting to emit a R_CAPABILITY relocation against ";
-      if (sym->getName().empty())
-        diag << "local symbol";
-      else
-        diag << "symbol " << toStr(ctx, *sym);
-      diag << " in binary without a dynamic linker; try removing -Wl,-"
-           << (sym->isPreemptible ? "preemptible" : "local") << "-caprelocs=elf"
-           << referencedBy();
-      return;
-    }
-    assert(ctx.hasDynsym && "Should have been checked in Driver.cpp");
-    // We don't use a R_MIPS_CHERI_CAPABILITY relocation for the input but
-    // instead need to use an absolute pointer size relocation to write
-    // the offset addend
-    if (!dynRelSec) dynRelSec = ctx.mainPart->relaDyn.get();
-    // in the case that -local-caprelocs=elf is passed we need to ensure that
-    // the target symbol is included in the dynamic symbol table
-    if (!ctx.mainPart->dynSymTab) {
-      error("R_CHERI_CAPABILITY relocations need a dynamic symbol table");
-      return;
-    }
-    if (!isSymIncludedInDynsym(ctx, *sym)) {
-      if (!needTrampoline) {
-        error(
-            "added a R_CHERI_CAPABILITY relocation but symbol not included "
+  // Emit either the legacy __cap_relocs section or a R_CHERI_CAPABILITY reloc
+  // For local symbols we can also emit the untagged capability bits and
+  // instruct csu/rtld to run CBuildCap
+  if ((!sym || !sym->isPreemptible) && !needTrampoline) {
+    assert(ctx.arg.localCapRelocsMode == CapRelocsMode::Legacy &&
+           "relative ELF capability relocations not currently implemented");
+    ctx.in.capRelocs->addCapReloc({sec, offset}, {symOrSec, 0u}, addend);
+    return;
+  }
+
+  assert(sym && "ELF relocs should not be used against sections");
+  assert((sym->isPreemptible || needTrampoline) &&
+         "ELF relocs should not be used for non-preemptible symbols");
+  assert((!sym->isLocal() || needTrampoline) &&
+         "ELF relocs should not be used for local symbols");
+  if (ctx.arg.emachine == llvm::ELF::EM_MIPS && ctx.arg.buildingFreeBSDRtld) {
+    error("relocation " + toStr(ctx, type) + " against " +
+          verboseToString(ctx, sym) +
+          " cannot be using when building FreeBSD RTLD" + referencedBy());
+    return;
+  }
+  if (!lld::elf::hasDynamicLinker(ctx)) {
+    error("attempting to emit a R_CAPABILITY relocation against " +
+          (sym->getName().empty() ? "local symbol"
+                                  : "symbol " + toStr(ctx, *sym)) +
+          " in binary without a dynamic linker; try removing -Wl,-" +
+          (sym->isPreemptible ? "preemptible" : "local") + "-caprelocs=elf" +
+          referencedBy());
+    return;
+  }
+  assert(ctx.hasDynsym && "Should have been checked in Driver.cpp");
+  // We don't use a R_MIPS_CHERI_CAPABILITY relocation for the input but
+  // instead need to use an absolute pointer size relocation to write
+  // the offset addend
+  if (!dynRelSec)
+    dynRelSec = ctx.mainPart->relaDyn.get();
+  // in the case that -local-caprelocs=elf is passed we need to ensure that
+  // the target symbol is included in the dynamic symbol table
+  if (!ctx.mainPart->dynSymTab) {
+    error("R_CHERI_CAPABILITY relocations need a dynamic symbol table");
+    return;
+  }
+  if (!isSymIncludedInDynsym(ctx, *sym)) {
+    if (!needTrampoline) {
+      error("added a R_CHERI_CAPABILITY relocation but symbol not included "
             "in dynamic symbol: " +
             verboseToString(ctx, sym));
-        return;
-      }
-      // Hack: Add a new global symbol with a unique name so that we can use
-      // a dynamic relocation against it.
-      // TODO: should it be possible to add STB_LOCAL symbols to .dynsymtab?
-      Defined *newSym = ctx.symtab->ensureSymbolWillBeInDynsym(sym);
-      assert(newSym->isFunc() && "This should only be used for functions");
-      assert(newSym->isExported);
-      assert(newSym->binding == llvm::ELF::STB_GLOBAL);
-      assert(newSym->visibility() == llvm::ELF::STV_HIDDEN);
-      sym = newSym;  // Make the relocation point to the newly added symbol
+      return;
     }
-    dynRelSec->addReloc(
-        DynamicReloc::AgainstSymbol, type, *sec, offset, *sym, addend, expr,
-        /* Relocation type for the addend = */ ctx.target->symbolicRel);
-
-  } else {
-    assert(ctx.arg.localCapRelocsMode == CapRelocsMode::Legacy);
-    ctx.in.capRelocs->addCapReloc({sec, offset}, {symOrSec, 0u}, addend);
+    // Hack: Add a new global symbol with a unique name so that we can use
+    // a dynamic relocation against it.
+    // TODO: should it be possible to add STB_LOCAL symbols to .dynsymtab?
+    Defined *newSym = ctx.symtab->ensureSymbolWillBeInDynsym(sym);
+    assert(newSym->isFunc() && "This should only be used for functions");
+    assert(isSymIncludedInDynsym(ctx, *newSym));
+    assert(newSym->binding == llvm::ELF::STB_GLOBAL);
+    assert(newSym->visibility() == llvm::ELF::STV_HIDDEN);
+    sym = newSym; // Make the relocation point to the newly added symbol
   }
+  dynRelSec->addReloc(
+      DynamicReloc::AgainstSymbol, type, *sec, offset, *sym, addend, expr,
+      /* Relocation type for the addend = */ ctx.target->symbolicRel);
 }
 
 void addNullDerivedCapability(Ctx &ctx, Symbol &sym, InputSectionBase &sec,