https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/122116
>From 9a637043f00ad388971cd6d4247510510fca31c8 Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Wed, 8 Jan 2025 13:07:01 +0000 Subject: [PATCH 1/6] [TBAA] Don't emit pointer-tbaa for void pointers. While there are no special rules in the standards regarding void pointers and strict aliasing, emitting distinct tags for void pointers break some common idioms and there is no good alternative to re-write the code without strict-aliasing violations. An example is to count the entries in an array of pointers: int count_elements(void * values) { void **seq = values; int count; for (count = 0; seq && seq[count]; count++); return count; } https://clang.godbolt.org/z/8dTv51v8W An example in the wild is from https://github.com/llvm/llvm-project/issues/119099 This patch avoids emitting distinct tags for void pointers, to avoid those idioms causing mis-compiles for now. --- clang/lib/CodeGen/CodeGenTBAA.cpp | 8 ++++++++ clang/test/CodeGen/tbaa-pointers.c | 13 +++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp index 75e66bae79afdc..3f1a24791ddd81 100644 --- a/clang/lib/CodeGen/CodeGenTBAA.cpp +++ b/clang/lib/CodeGen/CodeGenTBAA.cpp @@ -226,6 +226,14 @@ llvm::MDNode *CodeGenTBAA::getTypeInfoHelper(const Type *Ty) { PtrDepth++; Ty = Ty->getPointeeType()->getBaseElementTypeUnsafe(); } while (Ty->isPointerType()); + + // While there are no special rules in the standards regarding void pointers + // and strict aliasing, emitting distinct tags for void pointers break some + // common idioms and there is no good alternative to re-write the code + // without strict-aliasing violations. + if (Ty->isVoidType()) + return AnyPtr; + assert(!isa<VariableArrayType>(Ty)); // When the underlying type is a builtin type, we compute the pointee type // string recursively, which is implicitly more forgiving than the standards diff --git a/clang/test/CodeGen/tbaa-pointers.c b/clang/test/CodeGen/tbaa-pointers.c index 4aae2552f107a3..48adac503357f0 100644 --- a/clang/test/CodeGen/tbaa-pointers.c +++ b/clang/test/CodeGen/tbaa-pointers.c @@ -208,12 +208,9 @@ int void_ptrs(void **ptr) { // COMMON-LABEL: define i32 @void_ptrs( // COMMON-SAME: ptr noundef [[PTRA:%.+]]) // COMMON: [[PTR_ADDR:%.+]] = alloca ptr, align 8 -// DISABLE-NEXT: store ptr [[PTRA]], ptr [[PTR_ADDR]], align 8, !tbaa [[ANYPTR]] -// DISABLE-NEXT: [[L0:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8, !tbaa [[ANYPTR]] -// DISABLE-NEXT: [[L1:%.+]] = load ptr, ptr [[L0]], align 8, !tbaa [[ANYPTR]] -// DEFAULT-NEXT: store ptr [[PTRA]], ptr [[PTR_ADDR]], align 8, !tbaa [[P2VOID:!.+]] -// DEFAULT-NEXT: [[L0:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8, !tbaa [[P2VOID]] -// DEFAULT-NEXT: [[L1:%.+]] = load ptr, ptr [[L0]], align 8, !tbaa [[P1VOID:!.+]] +// COMMON-NEXT: store ptr [[PTRA]], ptr [[PTR_ADDR]], align 8, !tbaa [[ANYPTR]] +// COMMON-NEXT: [[L0:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8, !tbaa [[ANYPTR]] +// COMMON-NEXT: [[L1:%.+]] = load ptr, ptr [[L0]], align 8, !tbaa [[ANYPTR]] // COMMON-NEXT: [[BOOL:%.+]] = icmp ne ptr [[L1]], null // COMMON-NEXT: [[BOOL_EXT:%.+]] = zext i1 [[BOOL]] to i64 // COMMON-NEXT: [[COND:%.+]] = select i1 [[BOOL]], i32 0, i32 1 @@ -254,7 +251,3 @@ int void_ptrs(void **ptr) { // COMMON: [[INT_TAG]] = !{[[INT_TY:!.+]], [[INT_TY]], i64 0} // COMMON: [[INT_TY]] = !{!"int", [[CHAR]], i64 0} // DEFAULT: [[ANYPTR]] = !{[[ANY_POINTER]], [[ANY_POINTER]], i64 0} -// DEFAULT: [[P2VOID]] = !{[[P2VOID_TY:!.+]], [[P2VOID_TY]], i64 0} -// DEFAULT: [[P2VOID_TY]] = !{!"p2 void", [[ANY_POINTER]], i64 0} -// DEFAULT: [[P1VOID]] = !{[[P1VOID_TY:!.+]], [[P1VOID_TY]], i64 0} -// DEFAULT: [[P1VOID_TY]] = !{!"p1 void", [[ANY_POINTER]], i64 0} >From e72016567fab2b9a4673cf03ec7d7ec155c12f2e Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Thu, 9 Jan 2025 11:49:55 +0000 Subject: [PATCH 2/6] !fixup update remaining tests accessing void * --- .../CodeGenOpenCL/amdgpu-enqueue-kernel.cl | 25 +++++++++---------- clang/unittests/CodeGen/TBAAMetadataTest.cpp | 5 +--- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl b/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl index 5599f4dd50f042..ace34dd0ca6dce 100644 --- a/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl +++ b/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl @@ -651,7 +651,7 @@ kernel void test_target_features_kernel(global int *i) { // // GFX900: Function Attrs: convergent nounwind // GFX900-LABEL: define {{[^@]+}}@__test_block_invoke_3_kernel -// GFX900-SAME: (<{ i32, i32, ptr, ptr addrspace(1), ptr addrspace(1), i64, i8 }> [[TMP0:%.*]], ptr addrspace(3) [[TMP1:%.*]]) #[[ATTR6]] !kernel_arg_addr_space [[META28:![0-9]+]] !kernel_arg_access_qual [[META29:![0-9]+]] !kernel_arg_type [[META30:![0-9]+]] !kernel_arg_base_type [[META30]] !kernel_arg_type_qual [[META31:![0-9]+]] { +// GFX900-SAME: (<{ i32, i32, ptr, ptr addrspace(1), ptr addrspace(1), i64, i8 }> [[TMP0:%.*]], ptr addrspace(3) [[TMP1:%.*]]) #[[ATTR6]] !kernel_arg_addr_space [[META27:![0-9]+]] !kernel_arg_access_qual [[META28:![0-9]+]] !kernel_arg_type [[META29:![0-9]+]] !kernel_arg_base_type [[META29]] !kernel_arg_type_qual [[META30:![0-9]+]] { // GFX900-NEXT: entry: // GFX900-NEXT: [[TMP2:%.*]] = alloca <{ i32, i32, ptr, ptr addrspace(1), ptr addrspace(1), i64, i8 }>, align 8, addrspace(5) // GFX900-NEXT: store <{ i32, i32, ptr, ptr addrspace(1), ptr addrspace(1), i64, i8 }> [[TMP0]], ptr addrspace(5) [[TMP2]], align 8 @@ -688,7 +688,7 @@ kernel void test_target_features_kernel(global int *i) { // // GFX900: Function Attrs: convergent norecurse nounwind // GFX900-LABEL: define {{[^@]+}}@test_target_features_kernel -// GFX900-SAME: (ptr addrspace(1) noundef align 4 [[I:%.*]]) #[[ATTR2]] !kernel_arg_addr_space [[META32:![0-9]+]] !kernel_arg_access_qual [[META23]] !kernel_arg_type [[META33:![0-9]+]] !kernel_arg_base_type [[META33]] !kernel_arg_type_qual [[META25]] { +// GFX900-SAME: (ptr addrspace(1) noundef align 4 [[I:%.*]]) #[[ATTR2]] !kernel_arg_addr_space [[META31:![0-9]+]] !kernel_arg_access_qual [[META23]] !kernel_arg_type [[META32:![0-9]+]] !kernel_arg_base_type [[META32]] !kernel_arg_type_qual [[META25]] { // GFX900-NEXT: entry: // GFX900-NEXT: [[I_ADDR:%.*]] = alloca ptr addrspace(1), align 8, addrspace(5) // GFX900-NEXT: [[DEFAULT_QUEUE:%.*]] = alloca ptr addrspace(1), align 8, addrspace(5) @@ -700,7 +700,7 @@ kernel void test_target_features_kernel(global int *i) { // GFX900-NEXT: [[FLAGS_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[FLAGS]] to ptr // GFX900-NEXT: [[NDRANGE_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[NDRANGE]] to ptr // GFX900-NEXT: [[TMP_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[TMP]] to ptr -// GFX900-NEXT: store ptr addrspace(1) [[I]], ptr [[I_ADDR_ASCAST]], align 8, !tbaa [[TBAA34:![0-9]+]] +// GFX900-NEXT: store ptr addrspace(1) [[I]], ptr [[I_ADDR_ASCAST]], align 8, !tbaa [[TBAA33:![0-9]+]] // GFX900-NEXT: call void @llvm.lifetime.start.p5(i64 8, ptr addrspace(5) [[DEFAULT_QUEUE]]) #[[ATTR8]] // GFX900-NEXT: call void @llvm.lifetime.start.p5(i64 4, ptr addrspace(5) [[FLAGS]]) #[[ATTR8]] // GFX900-NEXT: store i32 0, ptr [[FLAGS_ASCAST]], align 4, !tbaa [[TBAA17]] @@ -803,16 +803,15 @@ kernel void test_target_features_kernel(global int *i) { // GFX900: [[META23]] = !{!"none"} // GFX900: [[META24]] = !{!"__block_literal"} // GFX900: [[META25]] = !{!""} -// GFX900: [[TBAA26]] = !{[[META27:![0-9]+]], [[META27]], i64 0} -// GFX900: [[META27]] = !{!"p1 void", [[META9]], i64 0} -// GFX900: [[META28]] = !{i32 0, i32 3} -// GFX900: [[META29]] = !{!"none", !"none"} -// GFX900: [[META30]] = !{!"__block_literal", !"void*"} -// GFX900: [[META31]] = !{!"", !""} -// GFX900: [[META32]] = !{i32 1} -// GFX900: [[META33]] = !{!"int*"} -// GFX900: [[TBAA34]] = !{[[META35:![0-9]+]], [[META35]], i64 0} -// GFX900: [[META35]] = !{!"p1 int", [[META9]], i64 0} +// GFX900: [[TBAA26]] = !{[[META9]], [[META9]], i64 0} +// GFX900: [[META27]] = !{i32 0, i32 3} +// GFX900: [[META28]] = !{!"none", !"none"} +// GFX900: [[META29]] = !{!"__block_literal", !"void*"} +// GFX900: [[META30]] = !{!"", !""} +// GFX900: [[META31]] = !{i32 1} +// GFX900: [[META32]] = !{!"int*"} +// GFX900: [[TBAA33]] = !{[[META34:![0-9]+]], [[META34]], i64 0} +// GFX900: [[META34]] = !{!"p1 int", [[META9]], i64 0} //. //// NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line: // CHECK: {{.*}} diff --git a/clang/unittests/CodeGen/TBAAMetadataTest.cpp b/clang/unittests/CodeGen/TBAAMetadataTest.cpp index cad8783ea73fba..35e68a577e08f6 100644 --- a/clang/unittests/CodeGen/TBAAMetadataTest.cpp +++ b/clang/unittests/CodeGen/TBAAMetadataTest.cpp @@ -120,10 +120,7 @@ TEST(TBAAMetadataTest, BasicTypes) { MInstruction(Instruction::Store, MValType(PointerType::getUnqual(Compiler.Context)), MMTuple( - MMTuple( - MMString("p1 void"), - AnyPtr, - MConstInt(0)), + AnyPtr, MSameAs(0), MConstInt(0)))); ASSERT_TRUE(I); >From 059c1268d43d31a04a012993ae0ebe2aa431d9a2 Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Thu, 9 Jan 2025 19:47:12 +0000 Subject: [PATCH 3/6] !fixup fix formatting --- clang/unittests/CodeGen/TBAAMetadataTest.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/clang/unittests/CodeGen/TBAAMetadataTest.cpp b/clang/unittests/CodeGen/TBAAMetadataTest.cpp index 35e68a577e08f6..f05c9787c63eaf 100644 --- a/clang/unittests/CodeGen/TBAAMetadataTest.cpp +++ b/clang/unittests/CodeGen/TBAAMetadataTest.cpp @@ -117,12 +117,9 @@ TEST(TBAAMetadataTest, BasicTypes) { ASSERT_TRUE(I); I = matchNext(I, - MInstruction(Instruction::Store, - MValType(PointerType::getUnqual(Compiler.Context)), - MMTuple( - AnyPtr, - MSameAs(0), - MConstInt(0)))); + MInstruction(Instruction::Store, + MValType(PointerType::getUnqual(Compiler.Context)), + MMTuple(AnyPtr, MSameAs(0), MConstInt(0)))); ASSERT_TRUE(I); I = matchNext(I, >From dcd27ec0b9cffbfcb219d66128641874d48c680c Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Tue, 28 Jan 2025 10:11:53 +0000 Subject: [PATCH 4/6] !fixup add Strict Aliasing section to docs. --- clang/docs/UsersManual.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index a56c9425ebb757..f5b113d28db035 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -2489,6 +2489,23 @@ are listed below. $ clang -fuse-ld=lld -Oz -Wl,--icf=safe -fcodegen-data-use code.cc +Strict Aliasing +--------------- + +Clang by default applies C/C++'s strict aliasing rules during optimizations. In +cases C and C++ rules diverge, the more conservative rules are used. Clang does +not make use of strict aliasing rules in all cases yet, including unions and +variable-sized arrays. That may change in the future. + +As of Clang 20, strict aliasing rules are also applied to nested pointers. The +new behavior can be disabled using ``-fno-pointer-tbaa``. Note that Clang does +not apply strict aliasing rules to `void*` pointers to avoid breaking existing +code, even though this is not required by the standard. + +Strict aliasing violations in the source may change program behavior and +``-fno-strict-aliasing`` disables use of the strict aliasing rules. There also +is an experimental :doc:`TypeSanitizer` to detect strict aliasing voliations. + Profile Guided Optimization --------------------------- >From 2c896bc9834a5298c66881598fd1e5060f54c892 Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Tue, 28 Jan 2025 13:15:03 +0000 Subject: [PATCH 5/6] !fixpup add info about clang-cl, remove dep on Tysan docs --- clang/docs/UsersManual.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index f5b113d28db035..a825e9db851ab7 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -2497,6 +2497,12 @@ cases C and C++ rules diverge, the more conservative rules are used. Clang does not make use of strict aliasing rules in all cases yet, including unions and variable-sized arrays. That may change in the future. +Internally Clang encodes the strict aliasing rules in LLVM IR using type-based +alias analysis (TBAA) metadata. + +Note that clang-cl disables strict aliasing by default, see +:ref:`Strict aliasing in clang-cl. <clang_cl_strict_aliasing>` + As of Clang 20, strict aliasing rules are also applied to nested pointers. The new behavior can be disabled using ``-fno-pointer-tbaa``. Note that Clang does not apply strict aliasing rules to `void*` pointers to avoid breaking existing @@ -2504,7 +2510,8 @@ code, even though this is not required by the standard. Strict aliasing violations in the source may change program behavior and ``-fno-strict-aliasing`` disables use of the strict aliasing rules. There also -is an experimental :doc:`TypeSanitizer` to detect strict aliasing voliations. +is an experimental TypeSanitizer to detect strict aliasing voliations, which +can be enabled by ``-fsanitize=type``. Profile Guided Optimization --------------------------- @@ -5289,6 +5296,7 @@ The Visual C++ Toolset has a slightly more elaborate mechanism for detection. Restrictions and Limitations compared to Clang ---------------------------------------------- +.. _clang_cl_strict_aliasing: Strict Aliasing ^^^^^^^^^^^^^^^ >From 0e421126dc73b7b0efd435488be6cb6aa8b3177b Mon Sep 17 00:00:00 2001 From: Florian Hahn <f...@fhahn.com> Date: Tue, 28 Jan 2025 20:43:22 +0000 Subject: [PATCH 6/6] !fixup add missing newline, TypeSanitizer doc reference. --- clang/docs/UsersManual.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index a825e9db851ab7..05909f5486367e 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -2510,8 +2510,8 @@ code, even though this is not required by the standard. Strict aliasing violations in the source may change program behavior and ``-fno-strict-aliasing`` disables use of the strict aliasing rules. There also -is an experimental TypeSanitizer to detect strict aliasing voliations, which -can be enabled by ``-fsanitize=type``. +is an experimental :ref:`TypeSanitizer <TypeSanitizer>` to detect strict +aliasing violations. Profile Guided Optimization --------------------------- @@ -5297,6 +5297,7 @@ Restrictions and Limitations compared to Clang ---------------------------------------------- .. _clang_cl_strict_aliasing: + Strict Aliasing ^^^^^^^^^^^^^^^ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits