Set no_preserve_tags for copies of structs without capabilities

clang/include/clang/AST/ASTContext.h

@@ -2210,6 +2210,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
 private:
   /// Map storing whether a type contains capabilities.
   mutable llvm::DenseMap<void*, bool> ContainsCapabilities;
+  mutable llvm::DenseMap<void *, bool> CannotContainCapabilities;
 
   CanQualType getFromTargetType(unsigned Type) const;
   TypeInfo getTypeInfoImpl(const Type *T) const;
@@ -2478,6 +2479,16 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// capability or an aggregate type that contains one or more capabilities.
   bool containsCapabilities(QualType Ty) const;
 
+  /// Returns true if the record type cannot contain capabilities.
+  /// NB: this is a conservative analysis that treats overaligned char arrays as
+  /// potentially containing capabilities.
+  bool cannotContainCapabilities(const RecordDecl *RD) const;
+  /// Returns true if the type is a scalar type that has a representationa
+  /// that cannot be used to (legally) store capabilities.
+  /// NB: this is a conservative analysis that treats overaligned char arrays as
+  /// potentially containing capabilities.
+  bool cannotContainCapabilities(QualType Ty) const;
+
   /// Return true if the specified type has unique object representations
   /// according to (C++17 [meta.unary.prop]p9)
   bool hasUniqueObjectRepresentations(QualType Ty) const;

clang/lib/AST/ASTContext.cpp

@@ -11730,6 +11730,62 @@ bool ASTContext::containsCapabilities(QualType Ty) const {
   return Ret;
 }
 
+bool ASTContext::cannotContainCapabilities(const RecordDecl *RD) const {
+  for (auto i = RD->field_begin(), e = RD->field_end(); i != e; ++i) {
+    const QualType Ty = i->getType();
+    if (Ty->isCHERICapabilityType(*this))
+      return false;
+    else if (const RecordType *RT = Ty->getAs<RecordType>()) {
+      if (!cannotContainCapabilities(RT->getDecl()))
+        return false;
+    } else if (!cannotContainCapabilities(Ty))
+      return false;
+  }
+  // In the case of C++ classes, also check base classes
+  if (const CXXRecordDecl *CRD = dyn_cast<CXXRecordDecl>(RD)) {
+    for (auto i = CRD->bases_begin(), e = CRD->bases_end(); i != e; ++i) {
+      const QualType Ty = i->getType();
+      if (const RecordType *RT = Ty->getAs<RecordType>())
+        if (!cannotContainCapabilities(RT->getDecl()))
+          return false;
+    }
+  } else if (RD->field_empty()) {
+    // A struct without fields could be used as an opaque type -> assume it
+    // might contain capabilities
+    return false;
+  }
+  return true; // Check all types that could contain capabilities
+}
+
+bool ASTContext::cannotContainCapabilities(QualType Ty) const {
+  // If we've already looked up this type, then return the cached value.
+  auto Cached = CannotContainCapabilities.find(Ty.getAsOpaquePtr());
+  if (Cached != CannotContainCapabilities.end())
+    return Cached->second;
+  // Don't bother caching the trivial cases.
+  if (containsCapabilities(Ty))
+    return false;
+  if (Ty->isArrayType()) {
+    QualType ElTy(Ty->getBaseElementTypeUnsafe(), 0);
+    // We have to be conservative here and assume that (unsigned) char[] as
+    // well as std::byte can be used for buffers that store capabilities.
+    // TODO: we could restrict this to buffers that are large enough and
+    // sufficiently aligned to store a capability.
+    if (ElTy->isCharType() || ElTy->isStdByteType())
+      return false;
+    return cannotContainCapabilities(ElTy);
+  }
+  const RecordType *RT = Ty->getAs<RecordType>();
+  if (!RT) {
+    // Not a record type, and the check above ensured this is not a capability
+    // type, so this type can't contain capabilities.
+    return true;
+  }
+  bool Ret = cannotContainCapabilities(RT->getDecl());
+  CannotContainCapabilities[Ty.getAsOpaquePtr()] = Ret;
+  return Ret;
+}
+
 QualType ASTContext::getCorrespondingSaturatedType(QualType Ty) const {
   assert(Ty->isFixedPointType());
 

clang/lib/CodeGen/CGBuiltin.cpp

@@ -2164,18 +2164,17 @@ RValue CodeGenFunction::emitRotate(const CallExpr *E, bool IsRotateRight) {
 }
 
 // Diagnose misaligned copies (memmove/memcpy) of source types that contain
-// capabilities to a dst buffer that is less than capability aligned.
-// This can result in tags being lost at runtime if the buffer is not actually
-// capability aligned. Furthermore, if the user adds a __builtin_assume_aligned()
-// or a cast to a capability we can assume it is capability aligned an use
-// csc/clc if the memcpy()/memmove() is expanded inline.
+// capabilities to a dst buffer that is less than capability aligned. This can
+// result in tags being lost at runtime if the buffer is not actually capability
+// aligned. Another benefit of this diagnostic is that it can cause the the user
+// to add __builtin_assume_aligned() or a cast to a capability. This allows us
+// to potentially expand the memcpy()/memmove() inline.
 // TODO: maybe there needs to be an attribute __memmove_like__ or similar to
 // indicate that a function behaves like memmove/memcpy and we can use that
 // to diagnose unaligned copies.
-static void
-diagnoseMisalignedCapabiliyCopyDest(CodeGenFunction &CGF, StringRef Function,
-                                    const Expr *Src, const CharUnits DstAlignCU,
-                                    AnyMemTransferInst *MemInst = nullptr) {
+static void checkCapabilityCopy(CodeGenFunction &CGF, StringRef Function,
+                                const Expr *Src, const CharUnits DstAlignCU,
+                                AnyMemTransferInst *MemInst = nullptr) {
   // we want the real type not the implicit conversion to void*
   // TODO: ignore the first explicit cast to void*?
   auto UnderlyingSrcTy = Src->IgnoreParenImpCasts()->getType();
@@ -2187,18 +2186,15 @@ diagnoseMisalignedCapabiliyCopyDest(CodeGenFunction &CGF, StringRef Function,
   if (!Ctx.containsCapabilities(UnderlyingSrcTy))
     return;
 
-  // Add a must_preserve_cheri_tags attribute to the memcpy/memmove
-  // intrinsic to ensure that the backend will not lower it to an inlined
-  // sequence of 1/2/4/8 byte loads and stores which would strip the tag bits.
-  // TODO: a clc/csc that works on unaligned data but traps for a csc
-  // with a tagged value and unaligned address could also prevent tags
-  // from being lost.
+  // If we have a memory intrinsic, we let the backend diagnose this issue
+  // since the clang frontend rarely has enough information to correctly infer
+  // the alignment.
   if (MemInst) {
-    // If we have a memory intrinsic let the backend diagnose this issue:
-    // First, tell the backend that this copy must preserve tags
-    MemInst->addAttribute(llvm::AttributeList::FunctionIndex,
-                          llvm::Attribute::MustPreserveCheriTags);
-    // And also tell it what the underlying type was for improved diagnostics.
+    // No need to diagnose anything if we aren't preserving tags.
+    if (MemInst->shouldPreserveCheriTags() == PreserveCheriTags::Unnecessary)
+      return;
+    // Add a "frontend-memtransfer-type" attribute to the intrinsic
+    // to ensure that the backend can diagnose misaligned capability copies.
     std::string TypeName = UnderlyingSrcTy.getAsString();
     std::string CanonicalStr = UnderlyingSrcTy.getCanonicalType().getAsString();
     if (CanonicalStr != TypeName)
@@ -2255,23 +2251,20 @@ diagnoseMisalignedCapabiliyCopyDest(CodeGenFunction &CGF, StringRef Function,
   }
 }
 
-static void diagnoseMisalignedCapabiliyCopyDest(CodeGenFunction &CGF,
-                                                StringRef Function,
-                                                const Expr *Src, CallInst *CI) {
+static void checkCapabilityCopy(CodeGenFunction &CGF, StringRef Function,
+                                const Expr *Src, CallInst *CI) {
   AnyMemTransferInst *MemInst = cast<AnyMemTransferInst>(CI);
-  diagnoseMisalignedCapabiliyCopyDest(
-      CGF, Function, Src, CharUnits::fromQuantity(MemInst->getDestAlignment()),
-      MemInst);
+  checkCapabilityCopy(CGF, Function, Src,
+                      CharUnits::fromQuantity(MemInst->getDestAlignment()),
+                      MemInst);
 }
 
-static void diagnoseMisalignedCapabiliyCopyDest(CodeGenFunction &CGF,
-                                                StringRef Function,
-                                                const Expr *Src,
-                                                const Expr *Dst) {
+static void checkCapabilityCopy(CodeGenFunction &CGF, StringRef Function,
+                                const Expr *Src, const Expr *Dst) {
   auto UnderlyingDstTy = QualType(
       Dst->IgnoreImpCasts()->getType()->getPointeeOrArrayElementType(), 0);
-  diagnoseMisalignedCapabiliyCopyDest(
-      CGF, Function, Src, CGF.CGM.getNaturalTypeAlignment(UnderlyingDstTy));
+  checkCapabilityCopy(CGF, Function, Src,
+                      CGF.CGM.getNaturalTypeAlignment(UnderlyingDstTy));
 }
 
 // Map math builtins for long-double to f128 version.
@@ -3490,9 +3483,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
                         E->getArg(0)->getExprLoc(), FD, 0);
     EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(),
                         E->getArg(1)->getExprLoc(), FD, 1);
-    auto CI = Builder.CreateMemCpy(Dest, Src, SizeVal,
-                                   llvm::PreserveCheriTags::TODO, false);
-    diagnoseMisalignedCapabiliyCopyDest(*this, "memcpy", E->getArg(1), CI);
+    auto CI = Builder.CreateMemCpy(
+        Dest, Src, SizeVal,
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal),
+        false);
+    checkCapabilityCopy(*this, "memcpy", E->getArg(1), CI);
     if (BuiltinID == Builtin::BImempcpy ||
         BuiltinID == Builtin::BI__builtin_mempcpy)
       return RValue::get(Builder.CreateInBoundsGEP(Dest.getElementType(),
@@ -3511,7 +3506,10 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
                         E->getArg(0)->getExprLoc(), FD, 0);
     EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(),
                         E->getArg(1)->getExprLoc(), FD, 1);
-    Builder.CreateMemCpyInline(Dest, Src, Size);
+    Builder.CreateMemCpyInline(
+        Dest, Src, Size,
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1),
+                                          CharUnits::fromQuantity(Size)));
     return RValue::get(nullptr);
   }
 
@@ -3524,23 +3522,23 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     Expr::EvalResult SizeResult, DstSizeResult;
     if (!E->getArg(2)->EvaluateAsInt(SizeResult, CGM.getContext()) ||
         !E->getArg(3)->EvaluateAsInt(DstSizeResult, CGM.getContext())) {
-      diagnoseMisalignedCapabiliyCopyDest(*this, "__memcpy_chk", E->getArg(1),
-                                          E->getArg(0));
+      checkCapabilityCopy(*this, "__memcpy_chk", E->getArg(1), E->getArg(0));
       break;
     }
     llvm::APSInt Size = SizeResult.Val.getInt();
     llvm::APSInt DstSize = DstSizeResult.Val.getInt();
     if (Size.ugt(DstSize)) {
-      diagnoseMisalignedCapabiliyCopyDest(*this, "__memcpy_chk", E->getArg(1),
-                                          E->getArg(0));
+      checkCapabilityCopy(*this, "__memcpy_chk", E->getArg(1), E->getArg(0));
       break;
     }
     Address Dest = EmitPointerWithAlignment(E->getArg(0));
     Address Src = EmitPointerWithAlignment(E->getArg(1));
     Value *SizeVal = llvm::ConstantInt::get(Builder.getContext(), Size);
-    auto CI = Builder.CreateMemCpy(Dest, Src, SizeVal,
-                                   llvm::PreserveCheriTags::TODO, false);
-    diagnoseMisalignedCapabiliyCopyDest(*this, "memcpy", E->getArg(1), CI);
+    auto CI = Builder.CreateMemCpy(
+        Dest, Src, SizeVal,
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal),
+        false);
+    checkCapabilityCopy(*this, "memcpy", E->getArg(1), CI);
     return RValue::get(Dest.getPointer(), Dest.getAlignment().getQuantity());
   }
 
@@ -3559,23 +3557,23 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     Expr::EvalResult SizeResult, DstSizeResult;
     if (!E->getArg(2)->EvaluateAsInt(SizeResult, CGM.getContext()) ||
         !E->getArg(3)->EvaluateAsInt(DstSizeResult, CGM.getContext())) {
-      diagnoseMisalignedCapabiliyCopyDest(*this, "__memmove_chk", E->getArg(1),
-                                          E->getArg(0));
+      checkCapabilityCopy(*this, "__memmove_chk", E->getArg(1), E->getArg(0));
       break;
     }
     llvm::APSInt Size = SizeResult.Val.getInt();
     llvm::APSInt DstSize = DstSizeResult.Val.getInt();
     if (Size.ugt(DstSize)) {
-      diagnoseMisalignedCapabiliyCopyDest(*this, "__memmove_chk", E->getArg(1),
-                                          E->getArg(0));
+      checkCapabilityCopy(*this, "__memmove_chk", E->getArg(1), E->getArg(0));
       break;
     }
     Address Dest = EmitPointerWithAlignment(E->getArg(0));
     Address Src = EmitPointerWithAlignment(E->getArg(1));
     Value *SizeVal = llvm::ConstantInt::get(Builder.getContext(), Size);
-    auto CI = Builder.CreateMemMove(Dest, Src, SizeVal,
-                                    llvm::PreserveCheriTags::TODO, false);
-    diagnoseMisalignedCapabiliyCopyDest(*this, "memmove", E->getArg(1), CI);
+    auto CI = Builder.CreateMemMove(
+        Dest, Src, SizeVal,
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal),
+        false);
+    checkCapabilityCopy(*this, "memmove", E->getArg(1), CI);
     return RValue::get(Dest.getPointer(), Dest.getAlignment().getQuantity());
   }
 
@@ -3588,9 +3586,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
                         E->getArg(0)->getExprLoc(), FD, 0);
     EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(),
                         E->getArg(1)->getExprLoc(), FD, 1);
-    auto CI = Builder.CreateMemMove(Dest, Src, SizeVal,
-                                    llvm::PreserveCheriTags::TODO, false);
-    diagnoseMisalignedCapabiliyCopyDest(*this, "memmove", E->getArg(1), CI);
+    auto CI = Builder.CreateMemMove(
+        Dest, Src, SizeVal,
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal),
+        false);
+    checkCapabilityCopy(*this, "memmove", E->getArg(1), CI);
     return RValue::get(Dest.getPointer(), Dest.getAlignment().getQuantity());
   }
   case Builtin::BImemset:

clang/lib/CodeGen/CGClass.cpp

@@ -980,10 +980,13 @@ namespace {
       LValue SrcLV = CGF.MakeNaturalAlignAddrLValue(SrcPtr, RecordTy);
       LValue Src = CGF.EmitLValueForFieldInitialization(SrcLV, FirstField);
 
+      // We can pass EffectiveTypeKnown=true since this a C++ field copy.
+      auto PreserveTags = CGF.getTypes().copyShouldPreserveTagsForPointee(
+          RecordTy, /*EffectiveTypeKnown=*/true, MemcpySize);
       emitMemcpyIR(
           Dest.isBitField() ? Dest.getBitFieldAddress() : Dest.getAddress(CGF),
           Src.isBitField() ? Src.getBitFieldAddress() : Src.getAddress(CGF),
-          MemcpySize, llvm::PreserveCheriTags::TODO);
+          MemcpySize, PreserveTags);
       reset();
     }
 

clang/lib/CodeGen/CGDecl.cpp

@@ -1265,12 +1265,18 @@ static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
     }
   }
 
-  // Copy from a global.
+  // Copy from a global (and therefore the effective type of the variable is
+  // known).
+  auto PreserveTags =
+      IsAutoInit || !ContainsCaps
+          ? llvm::PreserveCheriTags::Unnecessary
+          : CGM.getTypes().copyShouldPreserveTagsForPointee(
+                D.getType(), /*EffectiveTypeKnown=*/true, SizeVal);
   auto *I =
       Builder.CreateMemCpy(Loc,
                            createUnnamedGlobalForMemcpyFrom(
                                CGM, D, Builder, constant, Loc.getAlignment()),
-                           SizeVal, llvm::PreserveCheriTags::TODO, isVolatile);
+                           SizeVal, PreserveTags, isVolatile);
   if (IsAutoInit)
     I->addAnnotationMetadata("auto-init");
 }
@@ -1799,11 +1805,13 @@ void CodeGenFunction::emitZeroOrPatternForAutoVarInit(QualType type,
     llvm::PHINode *Cur = Builder.CreatePHI(Begin.getType(), 2, "vla.cur");
     Cur->addIncoming(Begin.getPointer(), OriginBB);
     CharUnits CurAlign = Loc.getAlignment().alignmentOfArrayElement(EltSize);
+    // Pattern init never writes valid tags, so we can pass
+    // PreserveCheriTags::Unnecessary to the CreateMemCpy() call
     auto *I = Builder.CreateMemCpy(
         Address(Cur, CurAlign),
         createUnnamedGlobalForMemcpyFrom(CGM, D, Builder, Constant,
                                          ConstantAlign),
-        BaseSizeInChars, llvm::PreserveCheriTags::TODO, isVolatile);
+        BaseSizeInChars, llvm::PreserveCheriTags::Unnecessary, isVolatile);
     I->addAnnotationMetadata("auto-init");
     llvm::Value *Next =
         Builder.CreateInBoundsGEP(Int8Ty, Cur, BaseSizeInChars, "vla.next");

clang/lib/CodeGen/CGExprAgg.cpp

@@ -747,7 +747,9 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) {
     llvm::Value *SizeVal = llvm::ConstantInt::get(
         CGF.SizeTy,
         CGF.getContext().getTypeSizeInChars(E->getType()).getQuantity());
-    Builder.CreateMemCpy(DestAddress, SourceAddress, SizeVal);
+    Builder.CreateMemCpy(
+        DestAddress, SourceAddress, SizeVal,
+        CGF.getTypes().copyShouldPreserveTags(E, E->getSubExpr(), SizeVal));
     break;
   }
 
@@ -2168,9 +2170,13 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty,
       }
     }
   }
-
-  auto Inst = Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal,
-                                   llvm::PreserveCheriTags::TODO, isVolatile);
+  // Note: this is used for expressions such as x = y, and not memcpy() calls,
+  // so according to C2x 6.5 "the effective type of the object is simply
+  // the type of the lvalue used for the access."
+  auto PreserveTags = getTypes().copyShouldPreserveTagsForPointee(
+      Ty, /*EffectiveTypeKnown=*/true, SizeVal);
+  auto Inst =
+      Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal, PreserveTags, isVolatile);
 
   // Determine the metadata to describe the position of any padding in this
   // memcpy, as well as the TBAA tags for the members of the struct, in case