Don't perform type-base tag-preservation analysis with -fno-strict-al…

…iasing

Marking a memcpy() to/from a long* as not tag-preserving could result in
tag stripping for code that assumes relaxed aliasing.

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);
}
```

Adding the no_preserve_tags attribute to memcpy() is safe under
-fstrict-aliasing (since the store above is invalid), but with
-fno-strict-aliasing this aligned memcpy must preserve tags.

clang/lib/CodeGen/CGBuiltin.cpp

@@ -2749,7 +2749,8 @@ 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),
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
+                                          CGM.getCodeGenOpts().RelaxedAliasing),
         false);
     checkCapabilityCopy(*this, "memcpy", E->getArg(1), CI);
     if (BuiltinID == Builtin::BImempcpy ||
@@ -2769,10 +2770,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);
-    Builder.CreateMemCpyInline(
-        Dest, Src, Size,
-        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1),
-                                          Builder.getInt64(Size)));
+    Builder.CreateMemCpyInline(Dest, Src, Size,
+                               getTypes().copyShouldPreserveTags(
+                                   E->getArg(0), E->getArg(1),
+                                   Builder.getInt64(Size),
+                                   CGM.getCodeGenOpts().RelaxedAliasing));
     return RValue::get(nullptr);
   }
 
@@ -2799,7 +2801,8 @@ 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),
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
+                                          CGM.getCodeGenOpts().RelaxedAliasing),
         false);
     checkCapabilityCopy(*this, "memcpy", E->getArg(1), CI);
     return RValue::get(Dest.getPointer(), Dest.getAlignment().getQuantity());
@@ -2834,7 +2837,8 @@ 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),
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
+                                          CGM.getCodeGenOpts().RelaxedAliasing),
         false);
     checkCapabilityCopy(*this, "memmove", E->getArg(1), CI);
     return RValue::get(Dest.getPointer(), Dest.getAlignment().getQuantity());
@@ -2851,7 +2855,8 @@ 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),
+        getTypes().copyShouldPreserveTags(E->getArg(0), E->getArg(1), SizeVal,
+                                          CGM.getCodeGenOpts().RelaxedAliasing),
         false);
     checkCapabilityCopy(*this, "memmove", E->getArg(1), CI);
     return RValue::get(Dest.getPointer(), Dest.getAlignment().getQuantity());

clang/lib/CodeGen/CGClass.cpp

@@ -987,7 +987,8 @@ namespace {
           Dest.isBitField() ? Dest.getBitFieldAddress() : Dest.getAddress(CGF),
           Src.isBitField() ? Src.getBitFieldAddress() : Src.getAddress(CGF),
           MemcpySize,
-          CGF.getTypes().copyShouldPreserveTagsForPointee(RecordTy));
+          CGF.getTypes().copyShouldPreserveTagsForPointee(
+              RecordTy, /*IgnoreTypes=*/false));
       reset();
     }
 

clang/lib/CodeGen/CGDecl.cpp

@@ -1239,12 +1239,13 @@ static void emitStoresForConstant(CodeGenModule &CGM, const VarDecl &D,
   }
 
   // Copy from a global.
-  Builder.CreateMemCpy(
-      Loc,
-      createUnnamedGlobalForMemcpyFrom(CGM, D, Builder, constant,
-                                       Loc.getAlignment()),
-      SizeVal, CGM.getTypes().copyShouldPreserveTagsForPointee(D.getType()),
-      isVolatile);
+  Builder.CreateMemCpy(Loc,
+                       createUnnamedGlobalForMemcpyFrom(
+                           CGM, D, Builder, constant, Loc.getAlignment()),
+                       SizeVal,
+                       CGM.getTypes().copyShouldPreserveTagsForPointee(
+                           D.getType(), /*IgnoreTypes=*/false),
+                       isVolatile);
 }
 
 static void emitStoresForZeroInit(CodeGenModule &CGM, const VarDecl &D,

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()));
+    Builder.CreateMemCpy(DestAddress, SourceAddress, SizeVal,
+                         CGF.getTypes().copyShouldPreserveTagsForPointee(
+                             E->getType(), /*IgnoreTypes=*/false));
     break;
   }
 
@@ -2042,7 +2042,8 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty,
   }
 
   auto Inst = Builder.CreateMemCpy(
-      DestPtr, SrcPtr, SizeVal, getTypes().copyShouldPreserveTagsForPointee(Ty),
+      DestPtr, SrcPtr, SizeVal,
+      getTypes().copyShouldPreserveTagsForPointee(Ty, /*IgnoreTypes=*/false),
       isVolatile);
 
   // Determine the metadata to describe the position of any padding in this

clang/lib/CodeGen/CGExprScalar.cpp

@@ -4216,6 +4216,7 @@ 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

@@ -970,7 +970,8 @@ bool CodeGenTypes::isZeroInitializable(QualType T) {
 
 llvm::PreserveCheriTags
 CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src,
-                                     const llvm::Value *Size) {
+                                     const llvm::Value *Size,
+                                     bool IgnoreTypes) {
   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.
@@ -983,22 +984,24 @@ CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src,
     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 getPointee().
+    // so we can't always call getPointeeType().
     QualType Ty = E->IgnoreParenImpCasts()->getType();
     if (Ty->isAnyPointerType())
       return Ty->getPointeeType();
     return Ty;
   };
 
-  auto DstPreserve = copyShouldPreserveTagsForPointee(GetPointee(Dest));
+  auto DstPreserve =
+      copyShouldPreserveTagsForPointee(GetPointee(Dest), IgnoreTypes);
   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));
+  auto SrcPreserve =
+      copyShouldPreserveTagsForPointee(GetPointee(Src), IgnoreTypes);
   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.
@@ -1017,27 +1020,37 @@ CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src,
   return DstPreserve;
 }
 
-llvm::PreserveCheriTags
-CodeGenTypes::copyShouldPreserveTags(QualType DestType) {
+llvm::PreserveCheriTags CodeGenTypes::copyShouldPreserveTags(QualType DestType,
+                                                             bool IgnoreTypes) {
   assert(DestType->isAnyPointerType() && "Copy dest must be a pointer type");
-  return copyShouldPreserveTagsForPointee(DestType->getPointeeType());
+  return copyShouldPreserveTagsForPointee(DestType->getPointeeType(),
+                                          IgnoreTypes);
 }
 
 llvm::PreserveCheriTags
-CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee) {
+CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee,
+                                               bool IgnoreTypes) {
   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.
+    return llvm::PreserveCheriTags::Unknown;
   } else if (Pointee->isIncompleteType()) {
     // We don't know if incomplete types contain capabilities, so be
     // conservative and assume that they might.
     return llvm::PreserveCheriTags::Unknown;
   } else if (auto *RD = Pointee->getAsRecordDecl()) {
     // TODO: should maybe add ASTContext::neverContainsCapabilities()
     if (RD->hasFlexibleArrayMember()) {
-      // This record may store capabilities in the trailing array.
+      // If this record has a trailing flexible array, we conservatively assume
+      // that the flexible array could contain capabilities.
+      // TODO: We could find the flexible array member and use that to
+      //  determine whether the type may contain tags.
       return llvm::PreserveCheriTags::Unknown;
     } else if (RD->field_empty()) {
       // This structure could be used as an opaque type -> assume it might
@@ -1057,14 +1070,15 @@ CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee) {
     return llvm::PreserveCheriTags::Unnecessary;
   } else if (Pointee->isArrayType()) {
     return copyShouldPreserveTagsForPointee(
-        QualType(Pointee->getArrayElementTypeNoTypeQual(), 0));
+        QualType(Pointee->getArrayElementTypeNoTypeQual(), 0), IgnoreTypes);
   }
 
   if (Pointee->isScalarType()) {
     // We can return PreserveCheriTags::Unneccessary for scalar types that are
     // not void *, char *, or std::byte * (since those could point to anything).
     assert(!Pointee->isCHERICapabilityType(Context));
-    if (!Pointee->isStdByteType() && !Pointee->isCharType())
+    if (!Pointee->isStdByteType() && !Pointee->isCharType() &&
+        !Pointee->isVoidType())
       return llvm::PreserveCheriTags::Unnecessary;
   }
 

clang/lib/CodeGen/CodeGenTypes.h

@@ -308,13 +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.
-  llvm::PreserveCheriTags copyShouldPreserveTags(QualType DestType);
+  /// 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);
+                                                 const llvm::Value *SizeVal,
+                                                 bool IgnoreTypes);
   /// Same as the copyShouldPreserveTags(), but expects DestType to be the
   /// pointee type rather than the type of the buffer pointer.
-  llvm::PreserveCheriTags copyShouldPreserveTagsForPointee(QualType DestType);
+  llvm::PreserveCheriTags copyShouldPreserveTagsForPointee(QualType DestType,
+                                                           bool IgnoreTypes);
 
   bool isRecordLayoutComplete(const Type *Ty) const;
   bool noRecordsBeingLaidOut() const {

clang/test/CodeGen/cheri/no-tag-copy-attribute-nocap-struct.c

@@ -5,7 +5,8 @@
 /// alignment was less than >= alignof(capability).
 /// TODO: include some end-to-end testing to ensure that the frontend and backend agree on
 ///  eliding the memcpy() call that we were previously force to make?
-// RUN: %riscv64_cheri_purecap_cc1 %s -emit-llvm -o - -O0 | FileCheck %s
+// RUN: %riscv64_cheri_purecap_cc1 %s -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-ALIASING
+// RUN: %riscv64_cheri_purecap_cc1 %s -relaxed-aliasing -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK,CHECK-RELAXED-ALIASING
 
 #if __has_feature(capabilities)
 #define CAP_SIZE sizeof(void *__capability)
@@ -28,6 +29,7 @@ struct vdso_timehands {
 
 void test_binuptime_assign(struct bintime *bt, struct vdso_timehands *th) {
   *bt = th->th_offset;
+  // We should add the no_preserve_tags attribute for this assignment even with -fno-strict-aliasing:
   // CHECK-LABEL: define void @test_binuptime_assign(
   // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 8 {{%[0-9a-zA-Z.]+}}, i8 addrspace(200)* align 8 {{%[0-9a-zA-Z.]+}},
   // CHECK-SAME: i64 16, i1 false) [[NO_PRESERVE_TAGS_ATTR:#[0-9]+]]{{$}}
@@ -53,15 +55,17 @@ union large_union_align_half_cap_size {
 };
 
 void test_bigger_union_copy(union large_union_align_half_cap_size *a, union large_union_align_half_cap_size *b) {
-  // No tags in this union -> can inline the memcpy()
+  // No tags in this union -> can inline the memcpy().
+  // Note: we can do this even with -fno-strict-aliasing since this is a direct assignment.
+  // XXX: Do we need an option to treats underaligned char[] members as potentially tag-bearing?
   *a = *b;
   // CHECK-LABEL: define void @test_bigger_union_copy(
   // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 8 {{%[0-9a-zA-Z.]+}}, i8 addrspace(200)* align 8 {{%[0-9a-zA-Z.]+}},
   // CHECK-SAME: i64 64, i1 false) [[NO_PRESERVE_TAGS_ATTR]]{{$}}
 }
 
 void test_align_copy_voidptr(void *a, void *b) {
-  // void* could contain caps but we don't add the attribute and rely on the backend to decide
+  // void* could contain caps so we don't add the attribute and rely on the backend to decide
   memcpy(a, b, CAP_SIZE);
   // CHECK-LABEL: define void @test_align_copy_voidptr(
   // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 1 {{%[0-9a-zA-Z.]+}}, i8 addrspace(200)* align 1 {{%[0-9a-zA-Z.]+}},
@@ -78,11 +82,12 @@ void test_align_copy_charptr(char *a, char *b) {
 }
 
 void test_align_copy_longptr(long *a, long *b) {
-  // Note: here we don't infer the no-preserve tags (yet)
+  // Note: We cannot infer the no-preserve tags attribute when building with -fno-strict-aliasing
   memcpy(a, b, CAP_SIZE);
   // CHECK-LABEL: define void @test_align_copy_longptr(
   // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 8 {{%[0-9a-zA-Z.]+}}, i8 addrspace(200)* align 8 {{%[0-9a-zA-Z.]+}},
-  // CHECK-SAME: i64 16, i1 false) [[NO_PRESERVE_TAGS_ATTR]]
+  // CHECK-STRICT-ALIASING-SAME: i64 16, i1 false) [[NO_PRESERVE_TAGS_ATTR]]{{$}}
+  // CHECK-RELAXED-ALIASING-SAME: i64 16, i1 false){{$}}
 }
 
 #if __has_feature(capabilities)
@@ -115,10 +120,12 @@ void test_align_copy_fwd_declared_2(void *a, struct fwddecl *b) {
 }
 
 void test_align_copy_fwd_declared_dst_notag(long *a, struct fwddecl *b) {
-  // We don't know if src contains capabilities, but the destination can't contain tags
+  // We don't know if src contains capabilities, but the destination can't contain tags.
+  // Note: we can't make this assumption for -fno-strict-aliasing
   // CHECK-LABEL: define void @test_align_copy_fwd_declared_dst_notag(
   // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 8 {{%[0-9a-zA-Z.]+}}, i8 addrspace(200)* align 1 {{%[0-9a-zA-Z.]+}},
-  // CHECK-SAME: i64 16, i1 false) [[NO_PRESERVE_TAGS_ATTR]]
+  // CHECK-STRICT-ALIASING-SAME: i64 16, i1 false) [[NO_PRESERVE_TAGS_ATTR]]{{$}}
+  // CHECK-RELAXED-ALIASING-SAME: i64 16, i1 false){{$}}
   memcpy(a, b, CAP_SIZE);
   // Note: if you look at the assembly output for  this call, it
   // still uses memcpy despite the attribute. b is only aligned to one byte and