Only perform type-base tag-preservation analysis if we know the real …

…type

Marking a memcpy() to/from a long* as not tag-preserving could result in
tag stripping for code that using type casts and is correct under strict
aliasing rules since the first store to a memory location determines the
type. Example from CTSRD-CHERI#506:
```
void *malloc(__SIZE_TYPE__);
void *memcpy(void *, const void *, __SIZE_TYPE__);

void foo(long **p, long **q) {
    *p = malloc(32);
    *q = malloc(32);
    (*p)[0] = 1;
    (*p)[1] = 2;
    *(void (**)(long **, long **))(*p + 2) = &foo;
    memcpy(*q, *p, 32);
}
```

Despite the memcpy being a long* (and therefore intuitevly not tag
preserving), we can't add the attribute since we don't actually know the
type of the underlying object (malloc creates an allocated with no declared
type). From C99:

The effective type of an object for an access to its stored value is the
declared type of the object, if any (footnote 75: Allocated objects have
no declared type).

If a value is stored into an object having no declared type through an
lvalue having a type that is not a character type, then the type of the
lvalue becomes the effective type of the object for that access and for
subsequent accesses that do not modify the stored value.

If a value is copied into an object having no declared type using memcpy
or memmove, or is copied as an array of character type, then the effective
type of the modified object for that access and for subsequent accesses
that do not modify the value is the effective type of the object from
which the value is copied, if it has one.

For all other accesses to an object having no declared type, the effective
type of the object is simply the type of the lvalue used for the access.

clang/lib/CodeGen/CGBuiltin.cpp

@@ -2749,8 +2749,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
                         E->getArg(1)->getExprLoc(), FD, 1);
     auto CI = Builder.CreateMemCpy(
         Dest, Src, SizeVal,
-        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
-                                          CGM.getCodeGenOpts().RelaxedAliasing),
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal),
         false);
     checkCapabilityCopy(*this, "memcpy", E->getArg(1), CI);
     if (BuiltinID == Builtin::BImempcpy ||
@@ -2770,11 +2769,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,
-                               getTypes().copyShouldPreserveTags(
-                                   E->getArg(0), E->getArg(1),
-                                   Builder.getInt64(Size),
-                                   CGM.getCodeGenOpts().RelaxedAliasing));
+    Builder.CreateMemCpyInline(
+        Dest, Src, Size,
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1),
+                                          Builder.getInt64(Size)));
     return RValue::get(nullptr);
   }
 
@@ -2801,8 +2799,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     Value *SizeVal = llvm::ConstantInt::get(Builder.getContext(), Size);
     auto CI = Builder.CreateMemCpy(
         Dest, Src, SizeVal,
-        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
-                                          CGM.getCodeGenOpts().RelaxedAliasing),
+        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());
@@ -2837,8 +2834,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     Value *SizeVal = llvm::ConstantInt::get(Builder.getContext(), Size);
     auto CI = Builder.CreateMemMove(
         Dest, Src, SizeVal,
-        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
-                                          CGM.getCodeGenOpts().RelaxedAliasing),
+        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());
@@ -2855,8 +2851,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
                         E->getArg(1)->getExprLoc(), FD, 1);
     auto CI = Builder.CreateMemMove(
         Dest, Src, SizeVal,
-        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
-                                          CGM.getCodeGenOpts().RelaxedAliasing),
+        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());

clang/lib/CodeGen/CGClass.cpp

@@ -983,12 +983,14 @@ namespace {
       LValue SrcLV = CGF.MakeNaturalAlignAddrLValue(SrcPtr, RecordTy);
       LValue Src = CGF.EmitLValueForFieldInitialization(SrcLV, FirstField);
 
+      // We can pass EffectiveTypeKnown=true since this a C++ field copy.
       emitMemcpyIR(
           Dest.isBitField() ? Dest.getBitFieldAddress() : Dest.getAddress(CGF),
           Src.isBitField() ? Src.getBitFieldAddress() : Src.getAddress(CGF),
           MemcpySize,
           CGF.getTypes().copyShouldPreserveTagsForPointee(
-              RecordTy, /*IgnoreTypes=*/false));
+              RecordTy, /*EffectiveTypeKnown=*/true,
+              CGF.Builder.getInt64(MemcpySize.getQuantity())));
       reset();
     }
 

clang/lib/CodeGen/CGDecl.cpp

@@ -1238,13 +1238,13 @@ static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
     }
   }
 
-  // Copy from a global.
+  // Copy from a global (so the effective type of the variable is known).
   Builder.CreateMemCpy(Loc,
                        createUnnamedGlobalForMemcpyFrom(
                            CGM, D, Builder, constant, Loc.getAlignment()),
                        SizeVal,
                        CGM.getTypes().copyShouldPreserveTagsForPointee(
-                           D.getType(), /*IgnoreTypes=*/false),
+                           D.getType(), /*EffectiveTypeKnown=*/true, SizeVal),
                        isVolatile);
 }
 

clang/lib/CodeGen/CGExprAgg.cpp

@@ -743,9 +743,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,
-                         CGF.getTypes().copyShouldPreserveTagsForPointee(
-                             E->getType(), /*IgnoreTypes=*/false));
+    Builder.CreateMemCpy(
+        DestAddress, SourceAddress, SizeVal,
+        CGF.getTypes().copyShouldPreserveTags(E, E->getSubExpr(), SizeVal));
     break;
   }
 
@@ -2040,11 +2040,14 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty,
       }
     }
   }
-
-  auto Inst = Builder.CreateMemCpy(
-      DestPtr, SrcPtr, SizeVal,
-      getTypes().copyShouldPreserveTagsForPointee(Ty, /*IgnoreTypes=*/false),
-      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 Inst =
+      Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal,
+                           getTypes().copyShouldPreserveTagsForPointee(
+                               Ty, /*EffectiveTypeKnown=*/true, SizeVal),
+                           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

clang/lib/CodeGen/CGExprScalar.cpp

@@ -4216,7 +4216,6 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
                      ? cast<BinaryOperator>(op.E)->getRHS()
                      : nullptr;
   if (RHSExpr && RHSExpr->getType()->isIntCapType()) {
-    // FIXME: avoid warning if result is casted to int
     const bool IsAddrMode = CGF.CGM.getLangOpts().cheriUIntCapUsesAddr();
     // Subtraction of __intcap_t is ambiguous: could be pointer
     // increment/decrement or pointer difference

clang/lib/CodeGen/CodeGenTypes.cpp

@@ -968,40 +968,39 @@ bool CodeGenTypes::isZeroInitializable(QualType T) {
   return true;
 }
 
-llvm::PreserveCheriTags
-CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src,
-                                     const llvm::Value *Size,
-                                     bool IgnoreTypes) {
+static bool isLessThanCapSize(const ASTContext &Context,
+                              const llvm::Value *Size) {
   if (auto ConstSize = dyn_cast_or_null<llvm::ConstantInt>(Size)) {
     // If the copy size is smaller than capability size we do not need to
     // preserve tag bits.
     auto CapSize = Context.toCharUnitsFromBits(
         Context.getTargetInfo().getCHERICapabilityWidth());
     if (ConstSize->getValue().slt(CapSize.getQuantity()))
-      return llvm::PreserveCheriTags::Unnecessary;
+      return true;
   }
-  auto GetPointee = [](const Expr *E) {
-    assert(E->getType()->isPointerType());
-    // Ignore the implicit cast to void* for the memcpy call.
-    // Note: IgnoreParenImpCasts() might strip function/array-to-pointer decay
-    // so we can't always call getPointeeType().
-    QualType Ty = E->IgnoreParenImpCasts()->getType();
-    if (Ty->isAnyPointerType())
-      return Ty->getPointeeType();
-    return Ty;
-  };
+  return false;
+}
 
-  auto DstPreserve =
-      copyShouldPreserveTagsForPointee(GetPointee(Dest), IgnoreTypes);
+llvm::PreserveCheriTags
+CodeGenTypes::copyShouldPreserveTags(const Expr *DestPtr, const Expr *SrcPtr,
+                                     const llvm::Value *Size) {
+  // Don't add the no_preserve_tags/must_preserve_tags attribute for non-CHERI
+  // targets to avoid changing tests and to avoid compile-time impact.
+  if (!Context.getTargetInfo().SupportsCapabilities())
+    return llvm::PreserveCheriTags::Unknown;
+  if (isLessThanCapSize(Context, Size)) {
+    // Copies smaller than capability size do not need to preserve tag bits.
+    return llvm::PreserveCheriTags::Unnecessary;
+  }
+  auto DstPreserve = copyShouldPreserveTags(DestPtr);
   if (DstPreserve == llvm::PreserveCheriTags::Unnecessary) {
     // If the destination does not need to preserve tags, we know that we don't
     // need to retain tags even if the source is a capability type.
     return llvm::PreserveCheriTags::Unnecessary;
   }
   assert(DstPreserve == llvm::PreserveCheriTags::Required ||
          DstPreserve == llvm::PreserveCheriTags::Unknown);
-  auto SrcPreserve =
-      copyShouldPreserveTagsForPointee(GetPointee(Src), IgnoreTypes);
+  auto SrcPreserve = copyShouldPreserveTags(SrcPtr);
   if (SrcPreserve == llvm::PreserveCheriTags::Unnecessary) {
     // If the copy source never contains capabilities, we don't need to retain
     // tags even if the destination is contains capabilities.
@@ -1020,25 +1019,65 @@ CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src,
   return DstPreserve;
 }
 
-llvm::PreserveCheriTags CodeGenTypes::copyShouldPreserveTags(QualType DestType,
-                                                             bool IgnoreTypes) {
-  assert(DestType->isAnyPointerType() && "Copy dest must be a pointer type");
-  return copyShouldPreserveTagsForPointee(DestType->getPointeeType(),
-                                          IgnoreTypes);
+llvm::PreserveCheriTags CodeGenTypes::copyShouldPreserveTags(const Expr *E) {
+  assert(E->getType()->isAnyPointerType());
+  // Ignore the implicit cast to void* for the memcpy call.
+  // Note: IgnoreParenImpCasts() might strip function/array-to-pointer decay
+  // so we can't always call getPointeeType().
+  QualType Ty = E->IgnoreParenImpCasts()->getType();
+  if (Ty->isAnyPointerType())
+    Ty = Ty->getPointeeType();
+  // TODO: Find the underlying VarDecl to improve diagnostics
+  const VarDecl *UnderlyingVar = nullptr;
+  // TODO: this assert may be overly aggressive
+  assert((!UnderlyingVar || UnderlyingVar->getType() == Ty) &&
+         "Passed wrong VarDecl?");
+  // If we have an underlying VarDecl, we can assume that the dynamic type of
+  // the object is known and can perform more detailed analysis.
+  return copyShouldPreserveTagsForPointee(Ty, UnderlyingVar != nullptr);
+}
+
+llvm::PreserveCheriTags CodeGenTypes::copyShouldPreserveTagsForPointee(
+    QualType CopyTy, bool EffectiveTypeKnown, const llvm::Value *SizeVal) {
+  // Don't add the no_preserve_tags/must_preserve_tags attribute for non-CHERI
+  // targets to avoid changing tests and to avoid compile-time impact.
+  if (!Context.getTargetInfo().SupportsCapabilities())
+    return llvm::PreserveCheriTags::Unknown;
+  if (isLessThanCapSize(Context, SizeVal)) {
+    // Copies smaller than capability size do not need to preserve tag bits.
+    return llvm::PreserveCheriTags::Unnecessary;
+  }
+  return copyShouldPreserveTagsForPointee(CopyTy, EffectiveTypeKnown);
 }
 
 llvm::PreserveCheriTags
 CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee,
-                                               bool IgnoreTypes) {
+                                               bool EffectiveTypeKnown) {
+  assert(Context.getTargetInfo().SupportsCapabilities() &&
+         "Should only be called for CHERI targets");
   assert(!Pointee.isNull() && "Should only be called for valid types");
   if (Context.containsCapabilities(Pointee)) {
     // If this is a capability type or a structure/union containing
     // capabilities, we obviously need to retain tag bits when copying.
     return llvm::PreserveCheriTags::Required;
-  } else if (IgnoreTypes) {
-    // If we are ignoring the type of the copy (e.g. -fno-strict-aliasing), we
-    // cannot assume anything else so return Unknown if containsCapabilities()
-    // was false.
+  } else if (!EffectiveTypeKnown) {
+    // If we don't know the underlying type of the copy (i.e. we just have a
+    // pointer without any additional context), we cannot assume that the actual
+    // object at that location matches the type of the pointer, so we have to
+    // conservatively return Unknown if containsCapabilities() was false.
+    // This is needed because C's strict aliasing rules are not based on the
+    // type of the pointer but rather based on the type of what was last stored.
+    // See C2x 6.5:
+    // "If a value is copied into an object having no declared type using memcpy
+    // or memmove, or is copied as an array of character type, then the
+    // effective type of the modified object for that access and for subsequent
+    // accesses that do not modify the value is the effective type of the object
+    // from which the value is copied, if it has one. For all other accesses to
+    // an object having no declared type, the effective type of the object is
+    // simply the type of the lvalue used for the access."
+    // And importantly: "Allocated objects have no declared type.", so unless
+    // we know what the underlying VarDecl is, we cannot use the type of the
+    // expression to determine whether it could hold tags.
     return llvm::PreserveCheriTags::Unknown;
   } else if (Pointee->isIncompleteType()) {
     // We don't know if incomplete types contain capabilities, so be
@@ -1069,8 +1108,8 @@ CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee,
     // having to call the library function.
     return llvm::PreserveCheriTags::Unnecessary;
   } else if (Pointee->isArrayType()) {
-    return copyShouldPreserveTagsForPointee(
-        QualType(Pointee->getArrayElementTypeNoTypeQual(), 0), IgnoreTypes);
+    return copyShouldPreserveTagsForPointee(Context.getBaseElementType(Pointee),
+                                            EffectiveTypeKnown);
   }
 
   if (Pointee->isScalarType()) {

clang/lib/CodeGen/CodeGenTypes.h

@@ -308,18 +308,18 @@ class CodeGenTypes {
   /// pointer to DestType may need to preserve CHERI tags (i.e. needs to call
   /// the copy function at run time if the alignment is not greater than the
   /// alignment of the destination buffer.
-  /// IgnoreTypes can be set to not treat copies of a e.g. non-capability type
-  /// as potentially carrying tags (e.g. memcpy() with -fno-strict-aliasing).
-  llvm::PreserveCheriTags copyShouldPreserveTags(QualType DestType,
-                                                 bool IgnoreTypes);
-  llvm::PreserveCheriTags copyShouldPreserveTags(const Expr *Dest,
-                                                 const Expr *Src,
-                                                 const llvm::Value *SizeVal,
-                                                 bool IgnoreTypes);
-  /// Same as the copyShouldPreserveTags(), but expects DestType to be the
+  /// This function attempts to determine the effective type of the source and
+  /// destination values (C2x 6.5p6) by checking for the underlying storage
+  /// (e.g. a referenced VarDecl) and performing a more conservative analysis
+  /// if this is not the case.
+  llvm::PreserveCheriTags copyShouldPreserveTags(const Expr *DestPtr,
+                                                 const Expr *SrcPtr,
+                                                 const llvm::Value *SizeVal);
+  /// Same as the copyShouldPreserveTags(), but expects CopyTy to be the
   /// pointee type rather than the type of the buffer pointer.
-  llvm::PreserveCheriTags copyShouldPreserveTagsForPointee(QualType DestType,
-                                                           bool IgnoreTypes);
+  llvm::PreserveCheriTags
+  copyShouldPreserveTagsForPointee(QualType CopyTy, bool EffectiveTypeKnown,
+                                   const llvm::Value *SizeVal);
 
   bool isRecordLayoutComplete(const Type *Ty) const;
   bool noRecordsBeingLaidOut() const {
@@ -329,6 +329,10 @@ class CodeGenTypes {
     return RecordsBeingLaidOut.count(Ty);
   }
 
+private:
+  llvm::PreserveCheriTags copyShouldPreserveTags(const Expr *E);
+  llvm::PreserveCheriTags
+  copyShouldPreserveTagsForPointee(QualType DestType, bool EffectiveTypeKnown);
 };
 
 }  // end namespace CodeGen