[clang] [TBAA] Don't emit pointer-tbaa for void pointers. (PR #122116)
Florian Hahn via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 28 05:15:50 PST 2025
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/122116
>From adbcc4fc5044d82bd983671ef5442142a3f498fe Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 8 Jan 2025 13:07:01 +0000
Subject: [PATCH 1/5] [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 1a1dc0ad9ee1047b4cfd69129a158ce9e3987430 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Thu, 9 Jan 2025 11:49:55 +0000
Subject: [PATCH 2/5] !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 b98ab9913f13f660182cf89d408ac584eb071b2e Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Thu, 9 Jan 2025 19:47:12 +0000
Subject: [PATCH 3/5] !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 0446809d396873c5d360567cec91163e3c114e88 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 28 Jan 2025 10:11:53 +0000
Subject: [PATCH 4/5] !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 260e84910c6f78..badd16c9e11bce 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -2486,6 +2486,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 0979a212ba091bf00dcfdf17a2592b8605922635 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 28 Jan 2025 13:15:03 +0000
Subject: [PATCH 5/5] !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 badd16c9e11bce..505dd5496cd24b 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -2494,6 +2494,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
@@ -2501,7 +2507,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
---------------------------
@@ -5286,6 +5293,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
^^^^^^^^^^^^^^^
More information about the cfe-commits
mailing list