[CHERIoT] Add warning for common array offset patterns that generate out-of-bounds intermediate capabilities.

These aren't an issue on big CHERI where representable offsets usually extend well beyond bounds, but on CHERIoT our representation limits are quite tight, so this is a common source of bugs.

Author: resistor

clang/include/clang/Basic/DiagnosticGroups.td

@@ -182,6 +182,7 @@ def CHERICapabilityToIntegerCast : DiagGroup<"capability-to-integer-cast">;
 def CheriPedantic : DiagGroup<"cheri-pedantic", [CHERICapabilityToIntegerCast, CHERIPrototypesStrict, CHERIProvenancePedantic]>;
 // Warnings/Errors for bugs in the MIPS/CHERI backend
 def MIPSCHERIBugs: DiagGroup<"mips-cheri-bugs">;
+def Cheriot : DiagGroup<"cheriot">;
 
 // Generally useful CHERI errors
 def CHERIMissingCompartment: DiagGroup<"cheri-missing-compartment">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -219,6 +219,10 @@ def err_cheriot_invalid_sealing_key_type_name
     : Error<"the sealing key type name '%0' is not a valid identifier">;
 def warn_cheriot_use_of_builtin_sealing_key_type_no_compartment
     : Warning<"%0 used, but no compartment name given">,InGroup<CHERIMissingCompartment>,DefaultError;
+def warn_cheriot_array_offset
+    : Warning<"offset pattern of %0 may create an invalid intermediate "
+              "capability; consider reassociating the offsets together">,
+      InGroup<Cheriot>;
 
 // C99 variable-length arrays
 def ext_vla : Extension<"variable length arrays are a C99 feature">,

clang/lib/Driver/ToolChains/Clang.cpp

@@ -2068,6 +2068,9 @@ void Clang::AddRISCVTargetArgs(const ArgList &Args,
   CmdArgs.push_back("-target-abi");
   CmdArgs.push_back(ABIName.data());
 
+  if (ABIName == "cheriot" || ABIName == "cheriot-baremetal")
+    CmdArgs.push_back("-Wcheriot");
+
   if (Arg *A = Args.getLastArg(options::OPT_G)) {
     CmdArgs.push_back("-msmall-data-limit");
     CmdArgs.push_back(A->getValue());

clang/lib/Sema/SemaExpr.cpp

@@ -10790,6 +10790,94 @@ QualType Sema::CheckSizelessVectorOperands(ExprResult &LHS, ExprResult &RHS,
   return QualType();
 }
 
+static void checkStaticOOBCapOffset(Sema &S, ExprResult &LHS, ExprResult &RHS,
+                                    BinaryOperator::Opcode OuterOpcode,
+                                    SourceLocation OuterLoc) {
+  auto getConstantAddend = [&](const Expr *E, llvm::APSInt &Value) {
+    Expr::EvalResult Result;
+    if (!E->EvaluateAsInt(Result, S.Context))
+      return false;
+    Value = Result.Val.getInt();
+    return true;
+  };
+
+  if (OuterOpcode != BO_Add && OuterOpcode != BO_Sub)
+    return;
+
+  // Simple arr + cst or arr - cst
+  if (const auto *Decay = dyn_cast<ImplicitCastExpr>(LHS.get())) {
+    if (Decay->getCastKind() == CK_ArrayToPointerDecay) {
+      const auto *CAT =
+          dyn_cast<ConstantArrayType>(Decay->getSubExpr()->getType());
+      if (CAT) {
+        const int64_t ArrayElts = CAT->getSExtSize();
+
+        llvm::APSInt AddendInt;
+        if (getConstantAddend(RHS.get(), AddendInt)) {
+          const int64_t Addend = AddendInt.getSExtValue();
+
+          // Warn on obviously out-of-bounds offsets.
+          if ((OuterOpcode == BO_Sub && Addend > 0) ||
+              (OuterOpcode == BO_Add && Addend < 0) ||
+              (OuterOpcode == BO_Add && Addend > ArrayElts) ||
+              (OuterOpcode == BO_Sub && Addend < -ArrayElts)) {
+            S.Diag(OuterLoc, diag::warn_cheriot_array_offset)
+                << Decay->getSubExpr();
+            return;
+          }
+        }
+      }
+    }
+  }
+
+  // Nested (array + X) + constant
+  const auto *InnerBO = dyn_cast<BinaryOperator>(LHS.get());
+  if (!InnerBO)
+    return;
+  if (InnerBO->getOpcode() != BO_Add && InnerBO->getOpcode() != BO_Sub)
+    return;
+
+  const auto *Decay = dyn_cast<ImplicitCastExpr>(InnerBO->getLHS());
+  if (!Decay || Decay->getCastKind() != CK_ArrayToPointerDecay)
+    return;
+  const auto *CAT = dyn_cast<ConstantArrayType>(Decay->getSubExpr()->getType());
+  if (!CAT)
+    return;
+  int64_t ArrayElts = CAT->getSExtSize();
+
+  llvm::APSInt OuterAddendInt;
+  llvm::APSInt InnerAddendInt;
+  bool OuterAddendConstant = getConstantAddend(RHS.get(), OuterAddendInt);
+  bool InnerAddendConstant =
+      getConstantAddend(InnerBO->getRHS(), InnerAddendInt);
+
+  if (OuterAddendConstant && InnerAddendConstant)
+    return;
+
+  // RHS of outer add is constant
+  if (OuterAddendConstant) {
+    const int64_t Addend = OuterAddendInt.getSExtValue();
+    if (Addend >= ArrayElts) {
+      S.Diag(InnerBO->getExprLoc(), diag::warn_cheriot_array_offset)
+          << Decay->getSubExpr();
+    }
+    return;
+  }
+
+  // RHS is not constant, but the inner RHS is constant
+  if (!InnerAddendConstant)
+    return;
+  int64_t Addend = InnerAddendInt.getSExtValue();
+
+  // Only warn when we have (arr + cst) + X and cst is equal to array size,
+  // because all other cases were already handled when checking the inner
+  // expression.
+  if (OuterOpcode == BO_Add && InnerBO->getOpcode() == BO_Add &&
+      Addend == ArrayElts)
+    S.Diag(InnerBO->getExprLoc(), diag::warn_cheriot_array_offset)
+        << Decay->getSubExpr();
+}
+
 // checkArithmeticNull - Detect when a NULL constant is used improperly in an
 // expression.  These are mainly cases where the null pointer is used as an
 // integer instead of a pointer.
@@ -11424,6 +11512,9 @@ QualType Sema::CheckAdditionOperands(ExprResult &LHS, ExprResult &RHS,
   // note that we bias towards the LHS being the pointer.
   Expr *PExp = LHS.get(), *IExp = RHS.get();
 
+  if (LHS.get()->getType()->isCHERICapabilityType(Context))
+    checkStaticOOBCapOffset(*this, LHS, RHS, BO_Add, Loc);
+
   // Addition is not allowed on sealed pointers.
   if (PExp->getType()->isCHERISealedCapabilityType(Context) &&
       !isUnevaluatedContext())
@@ -11555,6 +11646,8 @@ QualType Sema::CheckSubtractionOperands(ExprResult &LHS, ExprResult &RHS,
         return InvalidOperands(Loc, LHS, RHS);
       if (RHS.get()->getType()->isCHERISealedCapabilityType(Context))
         return InvalidOperands(Loc, LHS, RHS);
+      if (LHS.get()->getType()->isCHERICapabilityType(Context))
+        checkStaticOOBCapOffset(*this, LHS, RHS, BO_Sub, Loc);
     }
 
     // Diagnose bad cases where we step over interface counts.

clang/test/Sema/cheri/cheriot-array-offset.c

@@ -0,0 +1,45 @@
+// RUN: %riscv32_cheri_cc1 "-triple" "riscv32cheriot-unknown-unknown" "-target-abi" "cheriot" -verify %s 
+
+void bar(void*);
+
+void foo(int i) {
+    int buf[4];
+    bar(buf + i + 1);
+    bar(buf + i + 4); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf + i + 5); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf + i - 1);
+    bar(buf + i - 4); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf + i - 5); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - i + 1);
+    bar(buf - i + 4); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - i + 5); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - i - 1);
+    bar(buf - i - 4); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - i - 5); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+
+    bar(buf + 1 + i);
+    bar(buf + 4 + i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf + 5 + i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - 1 + i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - 4 + i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - 5 + i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf + 1 - i);
+    bar(buf + 4 - i);
+    bar(buf + 5 - i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - 1 - i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - 4 - i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+    bar(buf - 5 - i); // expected-warning{{offset pattern of 'buf' may create an invalid intermediate capability; consider reassociating the offsets together}}
+
+    bar(buf + 0 + 1);
+    bar(buf + 0 + 4);
+    bar(buf + 0 + 5);
+    bar(buf + 0 - 1);
+    bar(buf + 0 - 4);
+    bar(buf + 0 - 5);
+    bar(buf - 0 + 1);
+    bar(buf - 0 + 4);
+    bar(buf - 0 + 5);
+    bar(buf - 0 - 1);
+    bar(buf - 0 - 4);
+    bar(buf - 0 - 5);
+}