Skip to content

Commit 5ee4c99

Browse files
arichardsonarichardson
committed
Memory copies of less than cap size do not need to preserve tags
1 parent fa97741 commit 5ee4c99

File tree

3 files changed

+28
-21
lines changed

3 files changed

+28
-21
lines changed

‎clang/lib/CodeGen/CodeGenTypes.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,12 @@ bool CodeGenTypes::isZeroInitializable(QualType T) {
952952
return true;
953953
}
954954

955+
static bool isLessThanCapSize(const ASTContext &Context,
956+
Optional<CharUnits> Size) {
957+
return Size && *Size < Context.toCharUnitsFromBits(
958+
Context.getTargetInfo().getCHERICapabilityWidth());
959+
}
960+
955961
static bool copiesAtMostTypeSize(const QualType Ty, const ASTContext &Context,
956962
Optional<CharUnits> Size) {
957963
if (!Size)
@@ -967,6 +973,10 @@ CodeGenTypes::copyShouldPreserveTags(const Expr *DestPtr, const Expr *SrcPtr,
967973
// targets to avoid changing tests and to avoid compile-time impact.
968974
if (!Context.getTargetInfo().SupportsCapabilities())
969975
return llvm::PreserveCheriTags::Unknown;
976+
if (isLessThanCapSize(Context, Size)) {
977+
// Copies smaller than capability size do not need to preserve tag bits.
978+
return llvm::PreserveCheriTags::Unnecessary;
979+
}
970980
auto DstPreserve = copyShouldPreserveTags(DestPtr, Size);
971981
if (DstPreserve == llvm::PreserveCheriTags::Unnecessary) {
972982
// If the destination does not need to preserve tags, we know that we don't
@@ -1019,6 +1029,10 @@ llvm::PreserveCheriTags CodeGenTypes::copyShouldPreserveTagsForPointee(
10191029
// targets to avoid changing tests and to avoid compile-time impact.
10201030
if (!Context.getTargetInfo().SupportsCapabilities())
10211031
return llvm::PreserveCheriTags::Unknown;
1032+
if (isLessThanCapSize(Context, Size)) {
1033+
// Copies smaller than capability size do not need to preserve tag bits.
1034+
return llvm::PreserveCheriTags::Unnecessary;
1035+
}
10221036
assert(!Pointee.isNull() && "Should only be called for valid types");
10231037
if (Context.containsCapabilities(Pointee)) {
10241038
// If this is a capability type or a structure/union containing

‎clang/test/CodeGen/cheri/memcpy-unaligned.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void test_dst_unliagned_src_cap_memcpy(void *align1, align2_ptr align2, align4_p
3636
// expected-note@-2{{use __builtin_assume_aligned() or cast to (u)intptr_t*}}
3737
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
3838
// CHECK-SAME: align 1 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 16, i1 false)
39-
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_A:#[0-9]+]]
39+
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_A:#[0-9]+]]{{$}}
4040

4141
memcpy(align2, src, sizeof(*src));
4242
// 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}}
@@ -78,27 +78,23 @@ void test_no_warn_for_non_caps(short *align2, align2_ptr align2_not_short, int n
7878

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

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

8987
memcpy(align2, capptr, sizeof(*capptr));
9088
// 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}}
9189
// expected-note@-2{{use __builtin_assume_aligned() or cast to (u)intptr_t*}}
9290
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
93-
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 16, i1 false)
94-
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_UINTCAP:#[0-9]+]]{{$}}
91+
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 16, i1 false) [[PRESERVE_TAGS_ATTRIB_TYPE_UINTCAP:#[0-9]+]]{{$}}
9592

96-
memcpy(align2_not_short, struct_with_cap, sizeof(*struct_with_cap));
93+
memcpy(align2, struct_with_cap, sizeof(*struct_with_cap));
9794
// 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}}
9895
// expected-note@-2{{use __builtin_assume_aligned() or cast to (u)intptr_t*}}
9996
// CHECK: call void @llvm.memcpy.p200i8.p200i8.i64(i8 addrspace(200)*
100-
// CHECK-SAME: align 1 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 32, i1 false)
101-
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_STRUCT_WITH_CAP:#[0-9]+]]{{$}}
97+
// CHECK-SAME: align 2 %{{.+}}, i8 addrspace(200)* align 16 %{{.+}}, i64 32, i1 false) [[PRESERVE_TAGS_ATTRIB_TYPE_STRUCT_WITH_CAP:#[0-9]+]]{{$}}
10298
}
10399

104100
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) {
@@ -328,7 +324,7 @@ void test_builtin_assume_aligned_memmove_intermediate_var(char *align1, char *al
328324
// CHECK-SAME: [[PRESERVE_TAGS_ATTRIB_TYPE_A]]{{$}}
329325
}
330326

331-
// FIXME-DAG: attributes [[NO_PRESERVE_TAGS_ATTRIB]] = { no_preserve_cheri_tags }
327+
// CHECK-DAG: attributes [[NO_PRESERVE_TAGS_ATTRIB]] = { no_preserve_cheri_tags }
332328
// CHECK-DAG: attributes [[PRESERVE_TAGS_ATTRIB_TYPE_A]] = { must_preserve_cheri_tags "frontend-memtransfer-type"="'a' (aka 'unsigned __intcap')" }
333329
// CHECK-DAG: attributes [[PRESERVE_TAGS_ATTRIB_TYPE_STRUCT_WITH_CAP]] = { must_preserve_cheri_tags "frontend-memtransfer-type"="'struct with_cap'" }
334330
// CHECK-DAG: attributes [[PRESERVE_TAGS_ATTRIB_TYPE_UINTCAP]] = { must_preserve_cheri_tags "frontend-memtransfer-type"="'unsigned __intcap'" }

‎clang/test/CodeGen/cheri/no-tag-copy-attribute-with-caps.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,17 @@ void test_addrof_char(struct OneCap *cap, char c, __uint128_t u) {
1414
// the source does not contain tags
1515
__builtin_memmove(cap, &c, sizeof(c));
1616
// CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 1 {{%[a-z0-9.]+}}
17-
// CHECK-SAME: , i64 1, i1 false) [[MUST_PRESERVE_ATTR:#[0-9]+]]{{$}}
18-
// FIXME-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR:#[0-9]+]]{{$}}
17+
// CHECK-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR:#[0-9]+]]
1918
__builtin_memmove(&c, cap, sizeof(c));
2019
// 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]+}}
21-
// CHECK-SAME: , i64 1, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR:#[0-9]+]]{{$}}
22-
// FIXME-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR:#[0-9]+]]{{$}}
20+
// CHECK-SAME: , i64 1, i1 false) [[NO_PRESERVE_ATTR]]{{$}}
2321

2422
// uint128_t cannot not hold tags -> no need to preserve them since we can see the underlying allocation.
2523
__builtin_memmove(cap, &u, sizeof(u));
2624
// CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}}
2725
// FIXME: We can see the underlying decl, this should not need to preserve tags
28-
// CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR]]{{$}}
29-
// FIXME-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR]]{{$}}
26+
// CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_ATTR:#[0-9]+]]{{$}}
27+
// FIXME-SAME: , i64 16, i1 false) [[NO_PRESERVE_ATTR:#[0-9]+]]{{$}}
3028
__builtin_memmove(&u, cap, sizeof(u));
3129
// CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}}
3230
// FIXME: We can see the underlying decl, this should not need to preserve tags
@@ -41,10 +39,9 @@ void test_small_copy(struct OneCap *cap1, struct OneCap *cap2) {
4139
// CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}}
4240
// CHECK-SAME: , i64 16, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR]]{{$}}
4341
__builtin_memmove(cap1, cap2, 2);
44-
// TODO :This copy is too small -> should not preserve tags
42+
// This copy is too small -> no need to preserve tags
4543
// CHECK: call void @llvm.memmove.p200i8.p200i8.i64(i8 addrspace(200)* align 16 {{%[a-z0-9]+}}, i8 addrspace(200)* align 16 {{%[a-z0-9]+}}
46-
// CHECK-SAME: , i64 2, i1 false) [[MUST_PRESERVE_WITH_TYPE_ATTR]]{{$}}
47-
// FIXME-SAME: , i64 2, i1 false) [[NO_PRESERVE_ATTR]]{{$}}
44+
// CHECK-SAME: , i64 2, i1 false) [[NO_PRESERVE_ATTR]]{{$}}
4845
}
4946

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

0 commit comments

Comments
 (0)