From 94742cf865d955c8e809369af227ac816e6b6400 Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Sat, 3 Feb 2024 21:51:04 -0800 Subject: [PATCH 1/2] [CHERI-RISC-V] Add a baseline test for incorrect union arguments Unions that contain an int128 and a capability are passed as a single i128 instead of as a capability or indirectly. --- clang/test/CodeGen/cheri/cheri-union-i128.c | 215 ++++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 clang/test/CodeGen/cheri/cheri-union-i128.c diff --git a/clang/test/CodeGen/cheri/cheri-union-i128.c b/clang/test/CodeGen/cheri/cheri-union-i128.c new file mode 100644 index 000000000000..70f525a0dbd7 --- /dev/null +++ b/clang/test/CodeGen/cheri/cheri-union-i128.c @@ -0,0 +1,215 @@ +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature +// RUN: %riscv32_cheri_purecap_cc1 %s -fforce-enable-int128 -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-32 +// RUN: %riscv64_cheri_purecap_cc1 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-64 + +// UTC_ARGS: --disable +// CHECK-32: %union.u64 = type { i64 } +// CHECK-64: %union.u64 = type { ptr addrspace(200) } +// CHECK: %union.u128 = type { i128 } +// CHECK: %union.nested = type { %struct.anon } +// CHECK: %struct.anon = type { i32, ptr addrspace(200) } +// CHECK: %struct.anon.0 = type { ptr addrspace(200), i32 } +// UTC_ARGS: --enable + +union u64 { + __INT64_TYPE__ x; + __intcap cap; +}; +extern union u64 global64; + +// CHECK-32-LABEL: define {{[^@]+}}@arg64 +// CHECK-32-SAME: (i64 [[U_COERCE:%.*]]) addrspace(200) #[[ATTR0:[0-9]+]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[U:%.*]] = alloca [[UNION_U64:%.*]], align 8, addrspace(200) +// CHECK-32-NEXT: [[COERCE_DIVE:%.*]] = getelementptr inbounds [[UNION_U64]], ptr addrspace(200) [[U]], i32 0, i32 0 +// CHECK-32-NEXT: store i64 [[U_COERCE]], ptr addrspace(200) [[COERCE_DIVE]], align 8 +// CHECK-32-NEXT: [[TMP0:%.*]] = load i64, ptr addrspace(200) [[U]], align 8 +// CHECK-32-NEXT: ret i64 [[TMP0]] +// +// CHECK-64-LABEL: define {{[^@]+}}@arg64 +// CHECK-64-SAME: (ptr addrspace(200) [[U_COERCE:%.*]]) addrspace(200) #[[ATTR0:[0-9]+]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[U:%.*]] = alloca [[UNION_U64:%.*]], align 16, addrspace(200) +// CHECK-64-NEXT: [[COERCE_DIVE:%.*]] = getelementptr inbounds [[UNION_U64]], ptr addrspace(200) [[U]], i32 0, i32 0 +// CHECK-64-NEXT: store ptr addrspace(200) [[U_COERCE]], ptr addrspace(200) [[COERCE_DIVE]], align 16 +// CHECK-64-NEXT: [[TMP0:%.*]] = load i64, ptr addrspace(200) [[U]], align 16 +// CHECK-64-NEXT: ret i64 [[TMP0]] +// +__INT64_TYPE__ arg64(union u64 u) { + return u.x; +} +// CHECK-32-LABEL: define {{[^@]+}}@call64 +// CHECK-32-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[TMP0:%.*]] = load i64, ptr addrspace(200) @global64, align 8 +// CHECK-32-NEXT: [[CALL:%.*]] = call i64 @arg64(i64 [[TMP0]]) +// CHECK-32-NEXT: ret void +// +// CHECK-64-LABEL: define {{[^@]+}}@call64 +// CHECK-64-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[TMP0:%.*]] = load ptr addrspace(200), ptr addrspace(200) @global64, align 16 +// CHECK-64-NEXT: [[CALL:%.*]] = call i64 @arg64(ptr addrspace(200) [[TMP0]]) +// CHECK-64-NEXT: ret void +// +void call64(void) { + arg64(global64); +} + +// CHECK-32-LABEL: define {{[^@]+}}@ret64 +// CHECK-32-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[RETVAL:%.*]] = alloca [[UNION_U64:%.*]], align 8, addrspace(200) +// CHECK-32-NEXT: call void @llvm.memcpy.p200.p200.i32(ptr addrspace(200) align 8 [[RETVAL]], ptr addrspace(200) align 8 @global64, i32 8, i1 false) +// CHECK-32-NEXT: [[TMP0:%.*]] = load [[UNION_U64]], ptr addrspace(200) [[RETVAL]], align 8 +// CHECK-32-NEXT: ret [[UNION_U64]] [[TMP0]] +// +// CHECK-64-LABEL: define {{[^@]+}}@ret64 +// CHECK-64-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[RETVAL:%.*]] = alloca [[UNION_U64:%.*]], align 16, addrspace(200) +// CHECK-64-NEXT: call void @llvm.memcpy.p200.p200.i64(ptr addrspace(200) align 16 [[RETVAL]], ptr addrspace(200) align 16 @global64, i64 16, i1 false) +// CHECK-64-NEXT: [[TMP0:%.*]] = load [[UNION_U64]], ptr addrspace(200) [[RETVAL]], align 16 +// CHECK-64-NEXT: ret [[UNION_U64]] [[TMP0]] +// +union u64 ret64(void) { + return global64; +} + +// FIXME: Unions with int128+capability members should not be passed as i128! +union u128 { + __int128 x; + __intcap cap; +}; +extern union u128 global128; + +// CHECK-32-LABEL: define {{[^@]+}}@arg128 +// CHECK-32-SAME: (ptr addrspace(200) noundef [[U:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[TMP0:%.*]] = load i128, ptr addrspace(200) [[U]], align 16 +// CHECK-32-NEXT: ret i128 [[TMP0]] +// +// CHECK-64-LABEL: define {{[^@]+}}@arg128 +// CHECK-64-SAME: (i128 [[U_COERCE:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[U:%.*]] = alloca [[UNION_U128:%.*]], align 16, addrspace(200) +// CHECK-64-NEXT: [[COERCE_DIVE:%.*]] = getelementptr inbounds [[UNION_U128]], ptr addrspace(200) [[U]], i32 0, i32 0 +// CHECK-64-NEXT: store i128 [[U_COERCE]], ptr addrspace(200) [[COERCE_DIVE]], align 16 +// CHECK-64-NEXT: [[TMP0:%.*]] = load i128, ptr addrspace(200) [[U]], align 16 +// CHECK-64-NEXT: ret i128 [[TMP0]] +// +__int128 arg128(union u128 u) { + return u.x; +} +// CHECK-32-LABEL: define {{[^@]+}}@call128 +// CHECK-32-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[BYVAL_TEMP:%.*]] = alloca [[UNION_U128:%.*]], align 16, addrspace(200) +// CHECK-32-NEXT: call void @llvm.memcpy.p200.p200.i32(ptr addrspace(200) align 16 [[BYVAL_TEMP]], ptr addrspace(200) align 16 @global128, i32 16, i1 false) +// CHECK-32-NEXT: [[CALL:%.*]] = call i128 @arg128(ptr addrspace(200) noundef [[BYVAL_TEMP]]) +// CHECK-32-NEXT: ret void +// +// CHECK-64-LABEL: define {{[^@]+}}@call128 +// CHECK-64-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[TMP0:%.*]] = load i128, ptr addrspace(200) @global128, align 16 +// CHECK-64-NEXT: [[CALL:%.*]] = call i128 @arg128(i128 [[TMP0]]) +// CHECK-64-NEXT: ret void +// +void call128(void) { + arg128(global128); +} + +// CHECK-32-LABEL: define {{[^@]+}}@ret128 +// CHECK-32-SAME: (ptr addrspace(200) noalias sret([[UNION_U128:%.*]]) align 16 [[AGG_RESULT:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[RESULT_PTR:%.*]] = alloca ptr addrspace(200), align 8, addrspace(200) +// CHECK-32-NEXT: store ptr addrspace(200) [[AGG_RESULT]], ptr addrspace(200) [[RESULT_PTR]], align 8 +// CHECK-32-NEXT: call void @llvm.memcpy.p200.p200.i32(ptr addrspace(200) align 16 [[AGG_RESULT]], ptr addrspace(200) align 16 @global128, i32 16, i1 false) +// CHECK-32-NEXT: ret void +// +// CHECK-64-LABEL: define {{[^@]+}}@ret128 +// CHECK-64-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[RETVAL:%.*]] = alloca [[UNION_U128:%.*]], align 16, addrspace(200) +// CHECK-64-NEXT: call void @llvm.memcpy.p200.p200.i64(ptr addrspace(200) align 16 [[RETVAL]], ptr addrspace(200) align 16 @global128, i64 16, i1 false) +// CHECK-64-NEXT: [[TMP0:%.*]] = load [[UNION_U128]], ptr addrspace(200) [[RETVAL]], align 16 +// CHECK-64-NEXT: ret [[UNION_U128]] [[TMP0]] +// +union u128 ret128(void) { + return global128; +} + +// TODO: Unions with capabilities inside structs should use capability fields +// at the appropriate offsets rather than just one of the struct types. +// This is less likely to be an issue since such structs are generally passed +// indirectly but could be a problem in the future. +// Recored as https://github.com/CTSRD-CHERI/llvm-project/issues/725 +union nested { + struct { + int a; + __intcap b; + } c; + struct { + __intcap d; + int e; + } f; +}; +extern union nested global_nested; + +// CHECK-32-LABEL: define {{[^@]+}}@arg_nested +// CHECK-32-SAME: (ptr addrspace(200) noundef [[U:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[E:%.*]] = getelementptr inbounds [[STRUCT_ANON_0:%.*]], ptr addrspace(200) [[U]], i32 0, i32 1 +// CHECK-32-NEXT: [[TMP0:%.*]] = load i32, ptr addrspace(200) [[E]], align 8 +// CHECK-32-NEXT: ret i32 [[TMP0]] +// +// CHECK-64-LABEL: define {{[^@]+}}@arg_nested +// CHECK-64-SAME: (ptr addrspace(200) noundef [[U:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[E:%.*]] = getelementptr inbounds [[STRUCT_ANON_0:%.*]], ptr addrspace(200) [[U]], i32 0, i32 1 +// CHECK-64-NEXT: [[TMP0:%.*]] = load i32, ptr addrspace(200) [[E]], align 16 +// CHECK-64-NEXT: ret i32 [[TMP0]] +// +int arg_nested(union nested u) { + return u.f.e; +} +// CHECK-32-LABEL: define {{[^@]+}}@call_nested +// CHECK-32-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[BYVAL_TEMP:%.*]] = alloca [[UNION_NESTED:%.*]], align 8, addrspace(200) +// CHECK-32-NEXT: call void @llvm.memcpy.p200.p200.i32(ptr addrspace(200) align 8 [[BYVAL_TEMP]], ptr addrspace(200) align 8 @global_nested, i32 16, i1 false) +// CHECK-32-NEXT: [[CALL:%.*]] = call i32 @arg_nested(ptr addrspace(200) noundef [[BYVAL_TEMP]]) +// CHECK-32-NEXT: ret void +// +// CHECK-64-LABEL: define {{[^@]+}}@call_nested +// CHECK-64-SAME: () addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[BYVAL_TEMP:%.*]] = alloca [[UNION_NESTED:%.*]], align 16, addrspace(200) +// CHECK-64-NEXT: call void @llvm.memcpy.p200.p200.i64(ptr addrspace(200) align 16 [[BYVAL_TEMP]], ptr addrspace(200) align 16 @global_nested, i64 32, i1 false) +// CHECK-64-NEXT: [[CALL:%.*]] = call signext i32 @arg_nested(ptr addrspace(200) noundef [[BYVAL_TEMP]]) +// CHECK-64-NEXT: ret void +// +void call_nested(void) { + arg_nested(global_nested); +} + +// CHECK-32-LABEL: define {{[^@]+}}@ret_nested +// CHECK-32-SAME: (ptr addrspace(200) noalias sret([[UNION_NESTED:%.*]]) align 8 [[AGG_RESULT:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-32-NEXT: entry: +// CHECK-32-NEXT: [[RESULT_PTR:%.*]] = alloca ptr addrspace(200), align 8, addrspace(200) +// CHECK-32-NEXT: store ptr addrspace(200) [[AGG_RESULT]], ptr addrspace(200) [[RESULT_PTR]], align 8 +// CHECK-32-NEXT: call void @llvm.memcpy.p200.p200.i32(ptr addrspace(200) align 8 [[AGG_RESULT]], ptr addrspace(200) align 8 @global_nested, i32 16, i1 false) +// CHECK-32-NEXT: ret void +// +// CHECK-64-LABEL: define {{[^@]+}}@ret_nested +// CHECK-64-SAME: (ptr addrspace(200) noalias sret([[UNION_NESTED:%.*]]) align 16 [[AGG_RESULT:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-64-NEXT: entry: +// CHECK-64-NEXT: [[RESULT_PTR:%.*]] = alloca ptr addrspace(200), align 16, addrspace(200) +// CHECK-64-NEXT: store ptr addrspace(200) [[AGG_RESULT]], ptr addrspace(200) [[RESULT_PTR]], align 16 +// CHECK-64-NEXT: call void @llvm.memcpy.p200.p200.i64(ptr addrspace(200) align 16 [[AGG_RESULT]], ptr addrspace(200) align 16 @global_nested, i64 32, i1 false) +// CHECK-64-NEXT: ret void +// +union nested ret_nested(void) { + return global_nested; +} From e80c5f55a2da52f3392b39c3d3d9f64546490625 Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Sat, 3 Feb 2024 22:21:17 -0800 Subject: [PATCH 2/2] [CHERI] Prefer capability types when lowering unions to LLVM IR types This ensures that a union contain a __int128 and a capability will be lowered as a struct with one capability field instead of as an i128. This fixes tag stripping when passing such unions to functions by value instead of by reference. --- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 7 ++++--- clang/test/CodeGen/cheri/cheri-union-i128.c | 23 ++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index 0a993ffef072..436c1c795c78 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -349,10 +349,11 @@ void CGRecordLowering::lowerUnion() { if (!IsZeroInitializable) continue; // Conditionally update our storage type if we've got a new "better" one. - if (!StorageType || - getAlignment(FieldType) > getAlignment(StorageType) || + if (!StorageType || getAlignment(FieldType) > getAlignment(StorageType) || (getAlignment(FieldType) == getAlignment(StorageType) && - getSize(FieldType) > getSize(StorageType))) + getSize(FieldType) > getSize(StorageType)) || + (DataLayout.isFatPointer(FieldType) && + !DataLayout.isFatPointer(StorageType))) StorageType = FieldType; } // If we have no storage type just pad to the appropriate size and return. diff --git a/clang/test/CodeGen/cheri/cheri-union-i128.c b/clang/test/CodeGen/cheri/cheri-union-i128.c index 70f525a0dbd7..5b66b4e486dc 100644 --- a/clang/test/CodeGen/cheri/cheri-union-i128.c +++ b/clang/test/CodeGen/cheri/cheri-union-i128.c @@ -3,9 +3,9 @@ // RUN: %riscv64_cheri_purecap_cc1 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-64 // UTC_ARGS: --disable -// CHECK-32: %union.u64 = type { i64 } -// CHECK-64: %union.u64 = type { ptr addrspace(200) } -// CHECK: %union.u128 = type { i128 } +// CHECK: %union.u64 = type { ptr addrspace(200) } +// CHECK-32: %union.u128 = type { ptr addrspace(200), [8 x i8] } +// CHECK-64: %union.u128 = type { ptr addrspace(200) } // CHECK: %union.nested = type { %struct.anon } // CHECK: %struct.anon = type { i32, ptr addrspace(200) } // CHECK: %struct.anon.0 = type { ptr addrspace(200), i32 } @@ -18,11 +18,11 @@ union u64 { extern union u64 global64; // CHECK-32-LABEL: define {{[^@]+}}@arg64 -// CHECK-32-SAME: (i64 [[U_COERCE:%.*]]) addrspace(200) #[[ATTR0:[0-9]+]] { +// CHECK-32-SAME: (ptr addrspace(200) [[U_COERCE:%.*]]) addrspace(200) #[[ATTR0:[0-9]+]] { // CHECK-32-NEXT: entry: // CHECK-32-NEXT: [[U:%.*]] = alloca [[UNION_U64:%.*]], align 8, addrspace(200) // CHECK-32-NEXT: [[COERCE_DIVE:%.*]] = getelementptr inbounds [[UNION_U64]], ptr addrspace(200) [[U]], i32 0, i32 0 -// CHECK-32-NEXT: store i64 [[U_COERCE]], ptr addrspace(200) [[COERCE_DIVE]], align 8 +// CHECK-32-NEXT: store ptr addrspace(200) [[U_COERCE]], ptr addrspace(200) [[COERCE_DIVE]], align 8 // CHECK-32-NEXT: [[TMP0:%.*]] = load i64, ptr addrspace(200) [[U]], align 8 // CHECK-32-NEXT: ret i64 [[TMP0]] // @@ -41,8 +41,8 @@ __INT64_TYPE__ arg64(union u64 u) { // CHECK-32-LABEL: define {{[^@]+}}@call64 // CHECK-32-SAME: () addrspace(200) #[[ATTR0]] { // CHECK-32-NEXT: entry: -// CHECK-32-NEXT: [[TMP0:%.*]] = load i64, ptr addrspace(200) @global64, align 8 -// CHECK-32-NEXT: [[CALL:%.*]] = call i64 @arg64(i64 [[TMP0]]) +// CHECK-32-NEXT: [[TMP0:%.*]] = load ptr addrspace(200), ptr addrspace(200) @global64, align 8 +// CHECK-32-NEXT: [[CALL:%.*]] = call i64 @arg64(ptr addrspace(200) [[TMP0]]) // CHECK-32-NEXT: ret void // // CHECK-64-LABEL: define {{[^@]+}}@call64 @@ -76,7 +76,6 @@ union u64 ret64(void) { return global64; } -// FIXME: Unions with int128+capability members should not be passed as i128! union u128 { __int128 x; __intcap cap; @@ -90,11 +89,11 @@ extern union u128 global128; // CHECK-32-NEXT: ret i128 [[TMP0]] // // CHECK-64-LABEL: define {{[^@]+}}@arg128 -// CHECK-64-SAME: (i128 [[U_COERCE:%.*]]) addrspace(200) #[[ATTR0]] { +// CHECK-64-SAME: (ptr addrspace(200) [[U_COERCE:%.*]]) addrspace(200) #[[ATTR0]] { // CHECK-64-NEXT: entry: // CHECK-64-NEXT: [[U:%.*]] = alloca [[UNION_U128:%.*]], align 16, addrspace(200) // CHECK-64-NEXT: [[COERCE_DIVE:%.*]] = getelementptr inbounds [[UNION_U128]], ptr addrspace(200) [[U]], i32 0, i32 0 -// CHECK-64-NEXT: store i128 [[U_COERCE]], ptr addrspace(200) [[COERCE_DIVE]], align 16 +// CHECK-64-NEXT: store ptr addrspace(200) [[U_COERCE]], ptr addrspace(200) [[COERCE_DIVE]], align 16 // CHECK-64-NEXT: [[TMP0:%.*]] = load i128, ptr addrspace(200) [[U]], align 16 // CHECK-64-NEXT: ret i128 [[TMP0]] // @@ -112,8 +111,8 @@ __int128 arg128(union u128 u) { // CHECK-64-LABEL: define {{[^@]+}}@call128 // CHECK-64-SAME: () addrspace(200) #[[ATTR0]] { // CHECK-64-NEXT: entry: -// CHECK-64-NEXT: [[TMP0:%.*]] = load i128, ptr addrspace(200) @global128, align 16 -// CHECK-64-NEXT: [[CALL:%.*]] = call i128 @arg128(i128 [[TMP0]]) +// CHECK-64-NEXT: [[TMP0:%.*]] = load ptr addrspace(200), ptr addrspace(200) @global128, align 16 +// CHECK-64-NEXT: [[CALL:%.*]] = call i128 @arg128(ptr addrspace(200) [[TMP0]]) // CHECK-64-NEXT: ret void // void call128(void) {