Skip to content

Commit

Permalink
Memory copies of less than cap size do not need to preserve tags
Browse files Browse the repository at this point in the history
  • Loading branch information
@arichardson
arichardson committed Oct 5, 2022
1 parent 3cdd801 commit 3712d60
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 21 deletions.
14 changes: 14 additions & 0 deletions clang/lib/CodeGen/CodeGenTypes.cpp
View file
Expand Up @@ -952,6 +952,12 @@ bool CodeGenTypes::isZeroInitializable(QualType T) {
return true;
}

static bool isLessThanCapSize(const ASTContext &Context,
Optional<CharUnits> Size) {
return Size && *Size < Context.toCharUnitsFromBits(
Context.getTargetInfo().getCHERICapabilityWidth());
}

static bool copiesAtMostTypeSize(const QualType Ty, const ASTContext &Context,
Optional<CharUnits> Size) {
if (!Size)
Expand All @@ -967,6 +973,10 @@ CodeGenTypes::copyShouldPreserveTags(const Expr *DestPtr, const Expr *SrcPtr,
// 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, Size);
if (DstPreserve == llvm::PreserveCheriTags::Unnecessary) {
// If the destination does not need to preserve tags, we know that we don't
Expand Down Expand Up @@ -1019,6 +1029,10 @@ llvm::PreserveCheriTags CodeGenTypes::copyShouldPreserveTagsForPointee(
// 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;
}
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
Expand Down
18 changes: 7 additions & 11 deletions clang/test/CodeGen/cheri/memcpy-unaligned.c
View file
Expand Up @@ -36,7 +36,7 @@ void test_dst_unliagned_src_cap_memcpy(void *align1, align2_ptr align2, align4_p
// expected-note@-2{{use __builtin_assume_aligned() or cast to (u)intptr_t*}}
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
// CHECK-SAME: align 1 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 16, i1 false)
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_A:#[0-9]+]]
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_A:#[0-9]+]]{{$}}

memcpy(align2, src, sizeof(*src));
// expected-warning@-1{{memcpy operation with capability argument 'a' (aka 'unsigned __intcap') and underaligned destination (aligned to 2 bytes) may be inefficient or result in CHERI tags bits being stripped}}
Expand Down Expand Up @@ -78,27 +78,23 @@ void test_no_warn_for_non_caps(short *align2, align2_ptr align2_not_short, int n

memcpy(align2, &not_a_cap, sizeof(not_a_cap)); // no warning
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 4 %{{.+}}, i64 4, i1 false){{$}}
// FIXME-SAME: [[NO_PRESERVE_TAGS_ATTRIB:#[0-9]+]]{{$}}
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 4 %{{.+}}, i64 4, i1 false) [[NO_PRESERVE_TAGS_ATTRIB:#[0-9]+]]{{$}}

memcpy(align2, struct_without_cap, sizeof(*struct_without_cap)); // no warning
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 4 %{{.+}}, i64 8, i1 false){{$}}
// FIXME-SAME: [[NO_PRESERVE_TAGS_ATTRIB:#[0-9]+]]{{$}}
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 4 %{{.+}}, i64 8, i1 false) [[NO_PRESERVE_TAGS_ATTRIB:#[0-9]+]]{{$}}

memcpy(align2, capptr, sizeof(*capptr));
// expected-warning@-1{{memcpy operation with capability argument 'unsigned __intcap' and underaligned destination (aligned to 2 bytes) may be inefficient or result in CHERI tags bits being stripped}}
// expected-note@-2{{use __builtin_assume_aligned() or cast to (u)intptr_t*}}
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 16, i1 false)
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_UINTCAP:#[0-9]+]]{{$}}
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 16, i1 false) [[PRESERVE_TAGS_ATTRIB_TYPE_UINTCAP:#[0-9]+]]{{$}}

memcpy(align2_not_short, struct_with_cap, sizeof(*struct_with_cap));
memcpy(align2, struct_with_cap, sizeof(*struct_with_cap));
// expected-warning@-1{{memcpy operation with capability argument 'struct with_cap' and underaligned destination (aligned to 2 bytes) may be inefficient or result in CHERI tags bits being stripped}}
// expected-note@-2{{use __builtin_assume_aligned() or cast to (u)intptr_t*}}
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
// CHECK-SAME: align 1 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 32, i1 false)
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_STRUCT_WITH_CAP:#[0-9]+]]{{$}}
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 32, i1 false) [[PRESERVE_TAGS_ATTRIB_TYPE_STRUCT_WITH_CAP:#[0-9]+]]{{$}}
}

void test_dst_unliagned_src_cap_memmove(void *align1, align2_ptr align2, align4_ptr align4, align8_ptr align8, align_cap_ptr align_cap, a *src) {
Expand Down Expand Up @@ -328,7 +324,7 @@ void test_builtin_assume_aligned_memmove_intermediate_var(char *align1, char *al
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_A]]{{$}}
}

// FIXME-DAG: attributes [[NO_PRESERVE_TAGS_ATTRIB]] = { no_preserve_cheri_tags }
// CHECK-DAG: attributes [[NO_PRESERVE_TAGS_ATTRIB]] = { no_preserve_cheri_tags }
// CHECK-DAG: attributes [[PRESERVE_TAGS_ATTRIB_TYPE_A]] = { must_preserve_cheri_tags "frontend-memtransfer-type"="'a' (aka 'unsigned __intcap')" }
// CHECK-DAG: attributes [[PRESERVE_TAGS_ATTRIB_TYPE_STRUCT_WITH_CAP]] = { must_preserve_cheri_tags "frontend-memtransfer-type"="'struct with_cap'" }
// CHECK-DAG: attributes [[PRESERVE_TAGS_ATTRIB_TYPE_UINTCAP]] = { must_preserve_cheri_tags "frontend-memtransfer-type"="'unsigned __intcap'" }
17 changes: 7 additions & 10 deletions clang/test/CodeGen/cheri/no-tag-copy-attribute-with-caps.c
View file
Expand Up @@ -14,19 +14,17 @@ void test_addrof_char(struct OneCap *cap, char c, __uint128_t u) {
// the source does not contain tags
__builtin_memmove(cap, &c, sizeof(c));
// 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 1, i1 false) [[MUST_PRESERVE_ATTR:#[0-9]+]]{{$}}
// FIXME-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR:#[0-9]+]]{{$}}
// CHECK-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR:#[0-9]+]]
__builtin_memmove(&c, cap, sizeof(c));
// 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) [[MUST_PRESERVE_WITH_TYPE_ATTR:#[0-9]+]]{{$}}
// FIXME-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR:#[0-9]+]]{{$}}
// CHECK-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR]]{{$}}

// 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]+}}
// FIXME: We can see the underlying decl, this should not need to preserve tags
// CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR]]{{$}}
// FIXME-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}}
// CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR:#[0-9]+]]{{$}}
// FIXME-SAME: , i64 16, i1 false) [[NO_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]+}}
// FIXME: We can see the underlying decl, this should not need to preserve tags
Expand All @@ -41,10 +39,9 @@ void test_small_copy(struct OneCap *cap1, struct OneCap *cap2) {
// 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) [[MUST_PRESERVE_WITH_TYPE_ATTR]]{{$}}
__builtin_memmove(cap1, cap2, 2);
// TODO :This copy is too small -> should not preserve tags
// This copy is too small -> no need to preserve tags
// 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 2, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR]]{{$}}
// FIXME-SAME: , i64 2, i1 false) [[NO_PRESERVE_ATTR]]{{$}}
// CHECK-SAME: , i64 2, i1 false) [[NO_PRESERVE_ATTR]]{{$}}
}

struct strbuf {
Expand Down Expand Up @@ -194,4 +191,4 @@ void test_int_buffer(struct OneCap *cap, int *buf) {
// CHECK: attributes #0 = {
// CHECK-DAG: attributes [[MUST_PRESERVE_ATTR]] = { must_preserve_cheri_tags }
// CHECK-DAG: attributes [[MUST_PRESERVE_WITH_TYPE_ATTR]] = { must_preserve_cheri_tags "frontend-memtransfer-type"="'struct OneCap'" }
// FIXME-DAG: attributes [[NO_PRESERVE_ATTR]] = { no_preserve_cheri_tags }
// CHECK-DAG: attributes [[NO_PRESERVE_ATTR]] = { no_preserve_cheri_tags }

0 comments on commit 3712d60

Please sign in to comment.