[CHERIoT] Simplify CHERIoT compartment relocation handling in LLD.

lld/ELF/Arch/Cheri.cpp

@@ -496,8 +496,8 @@ void MipsCheriCapTableSection::addEntry(Symbol &sym, RelExpr expr,
   idx.firstUse = SymbolAndOffset(isec, offset);
   assert(!idx.firstUse->symOrSec.isNull());
   switch (expr) {
-  case R_MIPS_CHERI_CAPTAB_INDEX_SMALL_IMMEDIATE:
-  case R_MIPS_CHERI_CAPTAB_INDEX_CALL_SMALL_IMMEDIATE:
+  case RE_MIPS_CHERI_CAPTAB_INDEX_SMALL_IMMEDIATE:
+  case RE_MIPS_CHERI_CAPTAB_INDEX_CALL_SMALL_IMMEDIATE:
     idx.needsSmallImm = true;
     break;
   default:
@@ -509,8 +509,8 @@ void MipsCheriCapTableSection::addEntry(Symbol &sym, RelExpr expr,
   // not used as a function pointer and therefore does not need a unique
   // address (plt stub) across all DSOs.
   switch (expr) {
-  case R_MIPS_CHERI_CAPTAB_INDEX_CALL:
-  case R_MIPS_CHERI_CAPTAB_INDEX_CALL_SMALL_IMMEDIATE:
+  case RE_MIPS_CHERI_CAPTAB_INDEX_CALL:
+  case RE_MIPS_CHERI_CAPTAB_INDEX_CALL_SMALL_IMMEDIATE:
     if (!sym.isFunc() && !sym.isUndefWeak()) {
       CheriCapRelocLocation loc{isec, offset};
       std::string msg = "call relocation against non-function symbol " +

lld/ELF/Arch/Mips.cpp

@@ -121,7 +121,7 @@ RelExpr MIPS<ELFT>::getRelExpr(RelType type, const Symbol &s,
   case R_MICROMIPS_GPREL7_S2:
     return RE_MIPS_GOTREL;
   case R_MIPS_CHERI_CAPTABLEREL16:
-    return R_MIPS_CHERI_CAPTAB_REL;
+    return RE_MIPS_CHERI_CAPTAB_REL;
   case R_MIPS_26:
   case R_MICROMIPS_26_S1:
     return R_PLT;
@@ -210,25 +210,25 @@ RelExpr MIPS<ELFT>::getRelExpr(RelType type, const Symbol &s,
     return R_ABS_CAP;
   case R_MIPS_CHERI_CAPTAB_LO16:
   case R_MIPS_CHERI_CAPTAB_HI16:
-    return R_MIPS_CHERI_CAPTAB_INDEX;
+    return RE_MIPS_CHERI_CAPTAB_INDEX;
   case R_MIPS_CHERI_CAPCALL_LO16:
   case R_MIPS_CHERI_CAPCALL_HI16:
-    return R_MIPS_CHERI_CAPTAB_INDEX_CALL;
+    return RE_MIPS_CHERI_CAPTAB_INDEX_CALL;
   case R_MIPS_CHERI_CAPTAB_CLC11:
   case R_MIPS_CHERI_CAPTAB20:
-    return R_MIPS_CHERI_CAPTAB_INDEX_SMALL_IMMEDIATE;
+    return RE_MIPS_CHERI_CAPTAB_INDEX_SMALL_IMMEDIATE;
   case R_MIPS_CHERI_CAPCALL_CLC11:
   case R_MIPS_CHERI_CAPCALL20:
-    return R_MIPS_CHERI_CAPTAB_INDEX_CALL_SMALL_IMMEDIATE;
+    return RE_MIPS_CHERI_CAPTAB_INDEX_CALL_SMALL_IMMEDIATE;
   case R_MIPS_CHERI_CAPTAB_TLS_GD_LO16:
   case R_MIPS_CHERI_CAPTAB_TLS_GD_HI16:
-    return R_MIPS_CHERI_CAPTAB_TLSGD;
+    return RE_MIPS_CHERI_CAPTAB_TLSGD;
   case R_MIPS_CHERI_CAPTAB_TLS_LDM_LO16:
   case R_MIPS_CHERI_CAPTAB_TLS_LDM_HI16:
-    return R_MIPS_CHERI_CAPTAB_TLSLD;
+    return RE_MIPS_CHERI_CAPTAB_TLSLD;
   case R_MIPS_CHERI_CAPTAB_TLS_TPREL_LO16:
   case R_MIPS_CHERI_CAPTAB_TLS_TPREL_HI16:
-    return R_MIPS_CHERI_CAPTAB_TPREL;
+    return RE_MIPS_CHERI_CAPTAB_TPREL;
   default:
     Err(ctx) << getErrorLoc(ctx, loc) << "unknown relocation (" << type.v
              << ") against symbol " << &s;

lld/ELF/Arch/RISCV.cpp

@@ -62,6 +62,9 @@ class RISCV final : public TargetInfo {
 #define INTERNAL_R_RISCV_X0REL_I 258
 #define INTERNAL_R_RISCV_X0REL_S 259
 
+#define INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_LO_I 270
+#define INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_HI 271
+
 const uint64_t dtpOffset = 0x800;
 
 namespace {
@@ -396,14 +399,17 @@ RelExpr RISCV::getRelExpr(const RelType type, const Symbol &s,
     return R_GOT_PC;
   case R_RISCV_CHERI_TLS_GD_CAPTAB_PCREL_HI20:
     return R_TLSGD_PC;
+  case INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_HI:
+    return R_PC;
   case R_RISCV_CHERIOT_COMPARTMENT_HI:
-    return isPCCRelative(ctx, loc, &s) ? R_PC : R_CHERIOT_COMPARTMENT_CGPREL_HI;
+    return RE_CHERIOT_COMPARTMENT_CGPREL_HI;
   case R_RISCV_CHERIOT_COMPARTMENT_LO_I:
-    return R_CHERIOT_COMPARTMENT_CGPREL_LO_I;
   case R_RISCV_CHERIOT_COMPARTMENT_LO_S:
-    return R_CHERIOT_COMPARTMENT_CGPREL_LO_S;
+    return RE_CHERIOT_COMPARTMENT_CGPREL_LO;
+  case INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_LO_I:
+    return RE_RISCV_PC_INDIRECT;
   case R_RISCV_CHERIOT_COMPARTMENT_SIZE:
-    return R_CHERIOT_COMPARTMENT_SIZE;
+    return RE_CHERIOT_COMPARTMENT_SIZE;
   default:
     Err(ctx) << getErrorLoc(ctx, loc) << "unknown relocation (" << type.v
              << ") against symbol " << &s;
@@ -495,18 +501,16 @@ void RISCV::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
     int64_t hi = SignExtend64(val + 0x800, bits) >> 12;
     checkInt(ctx, loc, hi, 20, rel);
     if (isInt<20>(hi)) {
-      relocate(loc,
-               Relocation{R_NONE,
-                          ctx.arg.isCheriot ? R_RISCV_CHERIOT_COMPARTMENT_HI
-                                            : R_RISCV_PCREL_HI20,
-                          0, 0, rel.sym},
-               val);
-      relocate(loc + 4,
-               Relocation{R_NONE,
-                          ctx.arg.isCheriot ? R_RISCV_CHERIOT_COMPARTMENT_LO_I
-                                            : R_RISCV_PCREL_LO12_I,
-                          0, 0, rel.sym},
-               val);
+      relocateNoSym(loc,
+                    ctx.arg.isCheriot
+                        ? INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_HI
+                        : R_RISCV_PCREL_HI20,
+                    val);
+      relocateNoSym(loc + 4,
+                    ctx.arg.isCheriot
+                        ? INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_LO_I
+                        : R_RISCV_PCREL_LO12_I,
+                    val);
     }
     return;
   }
@@ -640,15 +644,18 @@ void RISCV::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
       write32le(loc + 4, val);
     break;
   case R_RISCV_CHERIOT_COMPARTMENT_LO_I: {
-    if (isPCCRelative(ctx, loc, rel.sym)) {
-      // Attach a negative sign bit to LO12 if the offset is negative.
-      // However, if HI20 alone is enough to reach the target, then this should
-      // not be done and LO14 should just be 0 regardless.
-      if (int64_t(val) >= 0 || (val & 0x7ff) == 0)
-        val &= 0x7ff;
-      else
-        val = (uint64_t(-1) & ~0x7ff) | (val & 0x7ff);
-    }
+    checkInt(ctx, loc, val, 12, rel);
+    write32le(loc, (read32le(loc) & 0x000fffff) | (val << 20));
+    break;
+  }
+  case INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_LO_I: {
+    // Attach a negative sign bit to LO12 if the offset is negative.
+    // However, if HI20 alone is enough to reach the target, then this should
+    // not be done and LO14 should just be 0 regardless.
+    if (int64_t(val) >= 0 || (val & 0x7ff) == 0)
+      val &= 0x7ff;
+    else
+      val = (uint64_t(-1) & ~0x7ff) | (val & 0x7ff);
     checkInt(ctx, loc, val, 12, rel);
     write32le(loc, (read32le(loc) & 0x000fffff) | (val << 20));
     break;
@@ -669,12 +676,24 @@ void RISCV::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
   case R_RISCV_CHERIOT_COMPARTMENT_HI: {
     // AUICGP
     uint32_t opcode = AUICGP;
-    if (isPCCRelative(ctx, loc, rel.sym)) {
-      opcode = AUIPCC;
-      if (int64_t(val) < 0)
-        val = (val + 0x7ff) & ~0x7ff;
-      val = int64_t(val) >> 11;
-    }
+    uint32_t existingOpcode = read32le(loc) & 0x7f;
+    if ((existingOpcode != AUIPCC) && (existingOpcode != AUICGP))
+      warn("R_RISCV_CHERIOT_COMPARTMENT_HI relocation applied to instruction "
+           "with unexpected opcode " +
+           Twine(existingOpcode));
+    checkInt(ctx, loc, SignExtend64(val + 0x800, bits) >> 12, 20, rel);
+    // Preserve the target register.  We will rewrite the opcode (source
+    // register) to either AUICGP or AUIPCC and set the immediate field.
+    uint32_t insn = read32le(loc) & 0x00000f80;
+    write32le(loc, insn | (val << 12) | opcode);
+    break;
+  }
+  case INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_HI: {
+    // AUIPCC
+    uint32_t opcode = AUIPCC;
+    if (int64_t(val) < 0)
+      val = (val + 0x7ff) & ~0x7ff;
+    val = int64_t(val) >> 11;
     uint32_t existingOpcode = read32le(loc) & 0x7f;
     if ((existingOpcode != AUIPCC) && (existingOpcode != AUICGP))
       warn("R_RISCV_CHERIOT_COMPARTMENT_HI relocation applied to instruction "
@@ -1037,69 +1056,85 @@ static void relaxCGP(Ctx &ctx, const InputSection &sec, size_t i, uint64_t loc,
 }
 
 /**
- * Find all R_RISCV_CHERIOT_COMPARTMENT_LO_I relocations that are CGP-relative
- * and rewrite them to be relative to the target of the current relocation.
- * These relocations mirror the HI20/LO12 PC-relative relocations and are
- * written as pairs where the first has the real relocation target as its
- * symbol and the second has the location of the first as its target.  This is
- * necessary for PC-relative relocations because the final address depends on
- * the location of the first instruction.  For CHERIoT, both PCC and
- * CGP-relative relocations use the same relocation types and we don't know
- * whether it is relative to PCC or CGP until we know the target.  That would
- * be fine, except that relaxation can delete the AUICGP, which means that we
- * then can't find the target.  We void this by doing a pass to find these
- * relocation targets and attaching them to the
- * R_RISCV_CHERIOT_COMPARTMENT_LO_I relocations for the cases where the target
- * is CGP-relative.
+ * Perform a substantial pre-pass of Cheriot relocations ahead of relaxation
+ * and relocation. This pre-pass has two goals:
+ *  - Separate PCC-relative and CGP-relative relocations. This reduces
+ *    complexity in relaxation and relocation by making the decision once
+ * upfront.
+ *  - Resolve the target address for the second in a pair of relocations. This
+ *    is required because relaxation may eliminate the first in a pair of
+ *    relocations, which would leave the second one unable to be relocated.
  *
- * Note: If we ever get direct PC[C]-relative loads in RISC-V then other
- * relocations will want to reuse this path.
+ * The specific post-conditions are:
+ *  - All PCC-relative CHERIOT_COMPARTMENT_HI relocations are represented as
+ *    INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_HI.
+ *  - All CGP-relative CHERIOT_COMPARTMENT_HI relocations are represented as
+ *    R_RISCV_CHERIOT_COMPARTMENT_HI.
+ *  - All PCC-relative CHERIOT_COMPARTMENT_LO_I relocations are represented as
+ *    INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_LO_I.
+ *  - All CGP-relative CHERIOT_COMPARTMENT_LO_I relocations are represented as
+ *    R_RISCV_CHERIOT_COMPARTMENT_LO_I.
+ *  - The targets of all CHERIOT_COMPARTMENT_LO_I relocations are resolved to
+ *    the target of their paired CHERIOT_COMPARTMENT_HI relocation.
+ *  - PCC-relative CHERIOT_COMPARTMENT_LO_S relocations do not exist.
+ *  - All CGP-relative CHERIOT_COMPARTMENT_LO_S relocations are represented as
+ *    R_RISCV_CHERIOT_COMPARTMENT_LO_S.
  */
 static bool rewriteCheriotLowRelocs(Ctx &ctx, InputSection &sec) {
   bool modified = false;
-  for (auto it : llvm::enumerate(sec.relocations)) {
-    Relocation &r = it.value();
-    if (r.type == R_RISCV_CHERIOT_COMPARTMENT_LO_I) {
+  for (auto &r : sec.relocations) {
+    if (r.type == R_RISCV_CHERIOT_COMPARTMENT_HI &&
+        isPCCRelative(ctx, nullptr, r.sym)) {
+      modified = true;
+      r.type = INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_HI;
+      r.expr = R_PC;
+    } else if (r.type == R_RISCV_CHERIOT_COMPARTMENT_LO_I) {
       // If this is PCC-relative, then the relocation points to the auicgp /
       // auipcc instruction and we need to look there to find the real target.
-      if (isPCCRelative(ctx, nullptr, r.sym)) {
-        const Defined *d = cast<Defined>(r.sym);
-        if (!d->section)
-          error("R_RISCV_CHERIOT_COMPARTMENT_LO_I relocation points to an "
-                "absolute symbol: " +
-                r.sym->getName());
-        InputSection *isec = cast<InputSection>(d->section);
-
-        // Relocations are sorted by offset, so we can use std::equal_range to
-        // do binary search.
-        Relocation targetReloc;
-        targetReloc.offset = d->value;
-        auto range = std::equal_range(
-            isec->relocations.begin(), isec->relocations.end(), targetReloc,
-            [](const Relocation &lhs, const Relocation &rhs) {
-              return lhs.offset < rhs.offset;
-            });
-
-        const Relocation *target = nullptr;
-        for (auto it = range.first; it != range.second; ++it)
-          if (it->type == R_RISCV_CHERIOT_COMPARTMENT_HI) {
-            target = &*it;
-            break;
-          }
-        if (!target) {
-          error(
-              "Could not find R_RISCV_CHERIOT_COMPARTMENT_HI relocation for " +
-              toStr(ctx, *r.sym));
+      if (!isPCCRelative(ctx, nullptr, r.sym))
+        fatal("R_RISCV_CHERIOT_COMPARTMENT_LO_I must point to "
+              "R_RISCV_COMPARTMENT_HI");
+
+      const Defined *d = cast<Defined>(r.sym);
+      if (!d->section)
+        error("R_RISCV_CHERIOT_COMPARTMENT_LO_I relocation points to an "
+              "absolute symbol: " +
+              r.sym->getName());
+      InputSection *isec = cast<InputSection>(d->section);
+
+      // Relocations are sorted by offset, so we can use std::equal_range to
+      // do binary search.
+      Relocation targetReloc;
+      targetReloc.offset = d->value;
+      auto range = std::equal_range(
+          isec->relocations.begin(), isec->relocations.end(), targetReloc,
+          [](const Relocation &lhs, const Relocation &rhs) {
+            return lhs.offset < rhs.offset;
+          });
+
+      const Relocation *target = nullptr;
+      for (auto it = range.first; it != range.second; ++it)
+        if (it->type == R_RISCV_CHERIOT_COMPARTMENT_HI ||
+            it->type == INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_HI) {
+          target = &*it;
+          break;
         }
-        // If the target is PCC-relative then the auipcc can't be erased and so
-        // skip the rewriting.
-        if (isPCCRelative(ctx, nullptr, target->sym))
-          continue;
-        // Update our relocation to point to the target thing.
-        r.sym = target->sym;
-        r.addend = target->addend;
-        modified = true;
+      if (!target) {
+        error("Could not find R_RISCV_CHERIOT_COMPARTMENT_HI relocation for " +
+              toStr(ctx, *r.sym));
+      }
+      modified = true;
+      // If the target is PCC-relative then the auipcc can't be erased and so
+      // skip the rewriting.
+      if (isPCCRelative(ctx, nullptr, target->sym)) {
+        r.type = INTERNAL_R_RISCV_CHERIOT_COMPARTMENT_PCCREL_LO_I;
+        r.expr = RE_RISCV_PC_INDIRECT;
+        continue;
       }
+      // Update our relocation to point to the target thing.
+      r.sym = target->sym;
+      r.addend = target->addend;
+      modified = true;
     }
   }
   return modified;
@@ -1115,6 +1150,8 @@ static bool relax(Ctx &ctx, int pass, InputSection &sec) {
   bool tlsdescRelax = false, toLeShortForm = false;
 
   // On the first pass, do a scan of LO_I CHERIoT relocations
+  // FIXME: One the relocation scan loop is under target control, this should be
+  // applied outside of relaxation.
   if (pass == 0)
     changed |= rewriteCheriotLowRelocs(ctx, sec);