diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index e1d2b2b3ab00..9ab7220379b9 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -980,11 +980,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)); + CGF.getTypes().copyShouldPreserveTagsForPointee( + RecordTy, /*EffectiveTypeKnown=*/true, + CGF.Builder.getInt64(MemcpySize.getQuantity()))); reset(); } diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 0ad8502384fc..4232f3808ce3 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -1265,14 +1265,16 @@ 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 *I = Builder.CreateMemCpy( Loc, createUnnamedGlobalForMemcpyFrom(CGM, D, Builder, constant, Loc.getAlignment()), SizeVal, IsAutoInit ? llvm::PreserveCheriTags::Unnecessary - : CGM.getTypes().copyShouldPreserveTagsForPointee(D.getType()), + : CGM.getTypes().copyShouldPreserveTagsForPointee( + D.getType(), /*EffectiveTypeKnown=*/true, SizeVal), isVolatile); if (IsAutoInit) I->addAnnotationMetadata("auto-init"); diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 0ef328b0b947..fb0bed0c547c 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -749,7 +749,7 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) { CGF.getContext().getTypeSizeInChars(E->getType()).getQuantity()); Builder.CreateMemCpy( DestAddress, SourceAddress, SizeVal, - CGF.getTypes().copyShouldPreserveTagsForPointee(E->getType())); + CGF.getTypes().copyShouldPreserveTags(E, E->getSubExpr(), SizeVal)); break; } @@ -2170,10 +2170,14 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty, } } } - - auto Inst = Builder.CreateMemCpy( - DestPtr, SrcPtr, SizeVal, getTypes().copyShouldPreserveTagsForPointee(Ty), - 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 diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp index 887c21a4d933..14e266012bf7 100644 --- a/clang/lib/CodeGen/CodeGenTypes.cpp +++ b/clang/lib/CodeGen/CodeGenTypes.cpp @@ -952,29 +952,31 @@ bool CodeGenTypes::isZeroInitializable(QualType T) { return true; } -llvm::PreserveCheriTags -CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src, - const llvm::Value *Size) { +static bool isLessThanCapSize(const ASTContext &Context, + const llvm::Value *Size) { if (auto ConstSize = dyn_cast_or_null(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 getPointee(). - QualType Ty = E->IgnoreParenImpCasts()->getType(); - if (Ty->isAnyPointerType()) - return Ty->getPointeeType(); - return Ty; - }; + return false; +} - auto DstPreserve = copyShouldPreserveTagsForPointee(GetPointee(Dest)); +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. @@ -982,7 +984,7 @@ CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src, } assert(DstPreserve == llvm::PreserveCheriTags::Required || DstPreserve == llvm::PreserveCheriTags::Unknown); - auto SrcPreserve = copyShouldPreserveTagsForPointee(GetPointee(Src)); + 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. @@ -1001,14 +1003,42 @@ CodeGenTypes::copyShouldPreserveTags(const Expr *Dest, const Expr *Src, return DstPreserve; } -llvm::PreserveCheriTags -CodeGenTypes::copyShouldPreserveTags(QualType DestType) { - assert(DestType->isAnyPointerType() && "Copy dest must be a pointer type"); - return copyShouldPreserveTagsForPointee(DestType->getPointeeType()); +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) { +CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee, + bool EffectiveTypeKnown) { + assert(Context.getTargetInfo().SupportsCapabilities() && + "Should only be called for CHERI targets"); assert(!Pointee.isNull() && "Should only be called for valid types"); if (!getTarget().SupportsCapabilities()) { // If we are not compiling for CHERI, return Unknown to avoid adding the @@ -1020,6 +1050,25 @@ CodeGenTypes::copyShouldPreserveTagsForPointee(QualType 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 (!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 // conservative and assume that they might. @@ -1027,7 +1076,10 @@ CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee) { } 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 @@ -1046,15 +1098,16 @@ CodeGenTypes::copyShouldPreserveTagsForPointee(QualType Pointee) { // having to call the library function. return llvm::PreserveCheriTags::Unnecessary; } else if (Pointee->isArrayType()) { - return copyShouldPreserveTagsForPointee( - QualType(Pointee->getArrayElementTypeNoTypeQual(), 0)); + return copyShouldPreserveTagsForPointee(Context.getBaseElementType(Pointee), + EffectiveTypeKnown); } 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; } diff --git a/clang/lib/CodeGen/CodeGenTypes.h b/clang/lib/CodeGen/CodeGenTypes.h index a81dc335be43..026e31fdb9b3 100644 --- a/clang/lib/CodeGen/CodeGenTypes.h +++ b/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); - llvm::PreserveCheriTags copyShouldPreserveTags(const Expr *Dest, - const Expr *Src, + /// 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 DestType to be the + /// 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); + llvm::PreserveCheriTags + copyShouldPreserveTagsForPointee(QualType CopyTy, bool EffectiveTypeKnown, + const llvm::Value *SizeVal); bool isRecordLayoutComplete(const Type *Ty) const; bool noRecordsBeingLaidOut() const { @@ -324,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 diff --git a/clang/test/CodeGen/cheri/no-tag-copy-attribute-nocap-struct.c b/clang/test/CodeGen/cheri/no-tag-copy-attribute-nocap-struct.c index d7ba1c9c82dc..63613f9928ab 100644 --- a/clang/test/CodeGen/cheri/no-tag-copy-attribute-nocap-struct.c +++ b/clang/test/CodeGen/cheri/no-tag-copy-attribute-nocap-struct.c @@ -3,9 +3,8 @@ /// not to contain capabilities. Previously we assumed that all copies >= sizeof(capability) /// can contain capabilities and we therefore fell back to calling memcpy if the /// 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 +// RUN: %riscv64_cheri_purecap_cc1 %s -relaxed-aliasing -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK #if __has_feature(capabilities) #define CAP_SIZE sizeof(void *__capability) @@ -28,6 +27,9 @@ struct vdso_timehands { void test_binuptime_assign(struct bintime *bt, struct vdso_timehands *th) { *bt = th->th_offset; + // We should always be able add the no_preserve_tags attribute for this assignment + // since this falls under C2x 6.5 "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." // CHECK-LABEL: 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,7 +55,9 @@ 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: 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.]+}}, @@ -61,7 +65,7 @@ void test_bigger_union_copy(union large_union_align_half_cap_size *a, union larg } 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: 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.]+}}, @@ -70,7 +74,7 @@ void test_align_copy_voidptr(void *a, void *b) { void test_align_copy_charptr(char *a, char *b) { // char* could contain caps since it's (unfortunately) basically the same as void*, - // but again we don't add the attribute and rely on the backend to decide + // so again we don't add the attribute and rely on the backend to decide memcpy(a, b, CAP_SIZE); // CHECK-LABEL: void @test_align_copy_charptr( // 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) + // We don't know the effective type of the underlying objects, so we can't add + // no_preserve_cheri_tags despite both pointeee types being non-tag-carrying. memcpy(a, b, CAP_SIZE); // CHECK-LABEL: 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-SAME: i64 16, i1 false){{$}} } #if __has_feature(capabilities) @@ -115,10 +120,11 @@ 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, and we also can't assume this + // for the destination since we don't know the effective type of the underlying object. // CHECK-LABEL: 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-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 diff --git a/clang/test/CodeGen/cheri/no-tag-copy-attribute-with-caps.c b/clang/test/CodeGen/cheri/no-tag-copy-attribute-with-caps.c index 29a4a24c6bd2..721a5a42ab91 100644 --- a/clang/test/CodeGen/cheri/no-tag-copy-attribute-with-caps.c +++ b/clang/test/CodeGen/cheri/no-tag-copy-attribute-with-caps.c @@ -1,5 +1,6 @@ // REQUIRES: riscv-registered-target -// RUN: %riscv64_cheri_purecap_cc1 %s -o - -emit-llvm | FileCheck %s +// RUN: %riscv64_cheri_purecap_cc1 %s -o - -emit-llvm | FileCheck %s --check-prefixes=CHECK +// RUN: %riscv64_cheri_purecap_cc1 %s -o - -emit-llvm -relaxed-aliasing | FileCheck %s --check-prefixes=CHECK // Diagnostics are only emitted when generating assembly (with optimizations) // RUN: %riscv64_cheri_purecap_cc1 -debug-info-kind=standalone %s -o /dev/null -O1 -S -verify @@ -18,13 +19,17 @@ void test_addrof_char(struct OneCap *cap, char c, __uint128_t u) { // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 1 {{%[a-z0-9]+}}.addr, i8 addrspace(200)* align 16 {{%[a-z0-9]+}} // CHECK-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR]]{{$}} - // uint128_t cannot not hold tags -> no need to preserve them. + // uint128_t cannot not hold tags -> no need to preserve them since we can see the underlying allocation. __builtin_memmove(cap, &u, sizeof(u)); // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // FIXME: We can see the underlying decl, this should not need to preserve tags + // FIXME-CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR:#[0-9]+]]{{$}} __builtin_memmove(&u, cap, sizeof(u)); // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // FIXME: We can see the underlying decl, this should not need to preserve tags + // FIXME-CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR:#[0-9]+]]{{$}} } void test_small_copy(struct OneCap *cap1, struct OneCap *cap2) { @@ -49,14 +54,17 @@ void test_addrof_char_buf(struct OneCap *cap, struct strbuf s) { // Since this is an address-of expression we should be able to detect that // the source does not contain tags // FIXME: the struct contains a char[] so maybe we should assume that it might - // be used to hold tags? But then the user should have added an _Alignas() - // attribute so this should be safe + // be used to hold tags? But then the user should have added an _Alignas()... __builtin_memmove(cap, &s, sizeof(s)); // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 1 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]] + // FIXME: We can see the underlying decl, this should not need to preserve tags + // FIXME-CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR]]{{$}} __builtin_memmove(&s, cap, sizeof(s)); // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 1 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]] + // FIXME: We can see the underlying decl, this should not need to preserve tags + // FIXME-CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR]]{{$}} } void test_array_decay(struct OneCap *cap) { @@ -65,13 +73,17 @@ void test_array_decay(struct OneCap *cap) { int buf[16]; __builtin_memmove(cap, buf, sizeof(*cap)); // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 4 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]] + // FIXME: We can see the underlying decl, this should not need to preserve tags + // FIXME-CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR]]{{$}} + __builtin_memmove(buf, cap, sizeof(*cap)); // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 4 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]] + // FIXME: We can see the underlying decl, this should not need to preserve tags + // FIXME-CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR]]{{$}} // char array aligned to one byte -> for now be conservative about char* and don't add the attibute - // TODO: this does not contain tags char buf2[16]; __builtin_memmove(cap, buf2, sizeof(*cap)); // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 1 {{%[a-z0-9]+}} @@ -160,13 +172,20 @@ void test_u8_buffer(struct OneCap *cap, __UINT8_TYPE__ *buf) { void test_int_buffer(struct OneCap *cap, int *buf) { // CHECK-LABEL: void @test_int_buffer( - // However, for int* we can assume it does not contain tags. + // Note: we cannot assume the int buffer is free of tags since C's rules + // depend on the type stored to that memory location last and not the type of + // the pointer. + // FIXME: shouldn't print __builtin_memmove(cap, buf, sizeof(*cap)); + // expected-warning@-1{{memmove operation with capability argument and underaligned destination (aligned to 4 bytes) may be inefficient or result in CHERI tags bits being stripped}} + // expected-note@-2{{use __builtin_assume_aligned()}} // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 4 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR]]{{$}} __builtin_memmove(buf, cap, sizeof(*cap)); + // expected-warning@-1{{memmove operation with capability argument 'struct OneCap' and underaligned destination (aligned to 4 bytes) may be inefficient or result in CHERI tags bits being stripped}} + // expected-note@-2{{use __builtin_assume_aligned()}} // CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 4 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}} - // CHECK-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}} + // CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR]]{{$}} } // CHECK: attributes #0 = { diff --git a/clang/test/CodeGen/cheri/no-tag-copy-strict-aliasing.c b/clang/test/CodeGen/cheri/no-tag-copy-strict-aliasing.c new file mode 100644 index 000000000000..8364bc10e1e5 --- /dev/null +++ b/clang/test/CodeGen/cheri/no-tag-copy-strict-aliasing.c @@ -0,0 +1,65 @@ +// RUN: %riscv64_cheri_purecap_cc1 %s -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK +// RUN: %riscv64_cheri_purecap_cc1 %s -relaxed-aliasing -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK +/// Check that we don't add the no_preserve_cheri_tags attribute based on the +/// pointee type of the memcpy since that could break certain valid (albeit +/// dubious) code that relies on tag-preservation for types such as long* +/// See C2x 6.5p6 ("effective type") for more detail. + +void *malloc(__SIZE_TYPE__); +void *memcpy(void *, const void *, __SIZE_TYPE__); +void foo(long **p, long **q); + +void must_retain(long **p, long **q) { + *p = malloc(32); + *q = malloc(32); + (*p)[0] = 1; + (*p)[1] = 2; + // Note: Despite the pointer being a long*, the C standard states that the + // first store to a malloc'd memory location defines the type of the memory + // for strict-aliasing purposes. + // Therefore, this cast and store is fine and we need to retain tags + // in the memcpy below (i.e. we can't add no_preserve_cheri_tags). + *(void (**)(long **, long **))(*p + 2) = &foo; + memcpy(*q, *p, 32); + // CHECK: @must_retain(i64 addrspace(200)* addrspace(200)* [[P:%.*]], i64 addrspace(200)* addrspace(200)* [[Q:%.*]]) addrspace(200) + // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 8 {{%[0-9]+}}, i8 addrspace(200)* align 8 {{%[0-9]+}} + // CHECK-SAME: , i64 32, i1 false){{$}} + // CHECK-NEXT: ret void +} + +void no_retain(long **q) { + long p[4]; + *q = malloc(32); + p[0] = 1; + p[1] = 2; + *(void (**)(long **, long **))(p + 2) = &foo; + memcpy(*q, p, 32); + // Since we can see that p is a long[4] stack-allocated variable, the object + // type is long[4] and therefore the memory should not be used to store tags. + // We can therefore omit the copy of tags (although in practise it will + // probably be aligned so malloc will retain tags at run time). + // CHECK: @no_retain(i64 addrspace(200)* addrspace(200)* [[Q:%.*]]) addrspace(200) + // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 8 {{%[0-9]+}}, i8 addrspace(200)* align 8 {{%[0-9]+}} + // CHECK-SAME: , i64 32, i1 false){{$}} + // TODO-CHECK-SAME: , i64 32, i1 false) [[NO_TAGS_ATTR:#.*]]{{$}} + // CHECK-NEXT: ret void +} + +void retain_char_array(long **q) { + // The C standard treats character arrays specially and therefore we must + // assume that those can hold tags (even if they aren't sufficiently aligned + // for holding capabilities). + _Alignas(8) char p[32]; + *q = malloc(32); + ((long *)p)[0] = 1; + ((long *)p)[1] = 2; + *(void (**)(long **, long **))(p + 16) = &foo; + memcpy(*q, p, 32); + // This array + // CHECK: @retain_char_array(i64 addrspace(200)* addrspace(200)* [[Q:%.*]]) addrspace(200) + // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)* align 8 {{%[a-z0-9]+}}, i8 addrspace(200)* align 8 {{%[a-z0-9]+}} + // CHECK-SAME: , i64 32, i1 false){{$}} + // CHECK-NEXT: ret void +} + +// TODO-CHECK: [[NO_TAGS_ATTR]] = { no_preserve_cheri_tags } diff --git a/clang/test/CodeGenCXX/cheri/no-tag-copy-copy-ctor.cpp b/clang/test/CodeGenCXX/cheri/no-tag-copy-copy-ctor.cpp new file mode 100644 index 000000000000..17f6575da0b9 --- /dev/null +++ b/clang/test/CodeGenCXX/cheri/no-tag-copy-copy-ctor.cpp @@ -0,0 +1,68 @@ +// RUN: %riscv64_cheri_purecap_cc1 %s -emit-llvm -o - | FileCheck %s +// RUN: %riscv64_cheri_purecap_cc1 %s -relaxed-aliasing -emit-llvm -o -| FileCheck %s +/// Check that we add the no_preserve_tags/must_preserve_tags attribute to +/// C++ copy constructors. +/// TODO: we may need to special-case char[] fields + +struct TestLongArray { + long array[5]; + TestLongArray(const TestLongArray &) = default; +}; + +struct TestLongArray test_copy_ctor_long_array(const TestLongArray &t) { + // Since this type only contains a long[] we don't need to preserve tags when copying + return t; + // CHECK-LABEL: @_Z25test_copy_ctor_long_arrayRK13TestLongArray( + // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64( + // CHECK-SAME: i8 addrspace(200)* align 8 {{%[0-9]+}}, i8 addrspace(200)* align 8 {{%[0-9]+}} + // CHECK-SAME: , i64 40, i1 false) [[NO_TAGS_ATTR:#[0-9]+]] + // CHECK-NEXT: ret void +} + +struct TestCharPtr { + char *cap; + long array[5]; + TestCharPtr(const TestCharPtr &) = default; +}; + +struct TestCharPtr test_copy_ctor_with_ptr(const TestCharPtr &t) { + // Since this type only contains a char* we must preserve tags when copying + return t; + // CHECK-LABEL: @_Z23test_copy_ctor_with_ptrRK11TestCharPtr( + // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64( + // CHECK-SAME: i8 addrspace(200)* align 16 {{%[0-9]+}}, i8 addrspace(200)* align 16 {{%[0-9]+}} + // CHECK-SAME: , i64 64, i1 false) [[MUST_PRESERVE_TAGS_ATTR:#[0-9]+]] + // CHECK-NEXT: ret void +} + +// TODO: we may need to conservatively assume that char[] could be used to hold tags? +struct TestCharArray { + char array[32]; // XXX: or only with _Alignas()? + TestCharArray(const TestCharArray &) = default; +}; + +struct TestCharArray test_copy_ctor_char_array(const TestCharArray &t) { + return t; + // CHECK-LABEL: @_Z25test_copy_ctor_char_arrayRK13TestCharArray( + // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64( + // CHECK-SAME: i8 addrspace(200)* align 1 {{%[0-9]+}}, i8 addrspace(200)* align 1 {{%[0-9]+}} + // CHECK-SAME: , i64 32, i1 false) [[NO_TAGS_ATTR]] + // CHECK-NEXT: ret void +} + +struct TestOveralignedCharArray { + alignas(32) char array[32]; // XXX: should we assume this can be used to store tags? + TestOveralignedCharArray(const TestOveralignedCharArray &) = default; +}; + +struct TestOveralignedCharArray test_copy_ctor_overaligned_char_array(const TestOveralignedCharArray &t) { + return t; + // CHECK-LABEL: @_Z37test_copy_ctor_overaligned_char_arrayRK24TestOveralignedCharArray( + // CHECK: call void @llvm.memcpy.p200i8.p200i8.i64( + // CHECK-SAME: i8 addrspace(200)* align 32 {{%[0-9]+}}, i8 addrspace(200)* align 32 {{%[0-9]+}} + // CHECK-SAME: , i64 32, i1 false) [[NO_TAGS_ATTR]] + // CHECK-NEXT: ret void +} + +// CHECK: [[NO_TAGS_ATTR]] = { no_preserve_cheri_tags } +// CHECK: [[MUST_PRESERVE_TAGS_ATTR]] = { must_preserve_cheri_tags }