[clang] [clang][FMV] Fix crash with cpu_specific attribute. (PR #115762)
Alexandros Lamprineas via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 25 13:21:14 PST 2024
https://github.com/labrinea updated https://github.com/llvm/llvm-project/pull/115762
>From aff962d795e56f7b41af44860feb77e656091b78 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Mon, 11 Nov 2024 20:10:01 +0000
Subject: [PATCH 1/3] [clang][FMV] Fix crash with cpu_specific attribute.
Raised here https://github.com/llvm/llvm-project/issues/115299.
The commit a2d3099 introduced `replaceDeclarationWith`, but it
shouldn't be called by the fallthrough code which handles the
cpu_specific attribute.
---
clang/lib/CodeGen/CodeGenModule.cpp | 2 --
clang/test/CodeGen/attr-cpuspecific.c | 12 ++++++------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index ba376f9ecfacde..27b1ccb8137356 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4597,8 +4597,6 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
assert(isa<llvm::GlobalValue>(Resolver) &&
"Resolver should be created for the first time");
SetCommonAttributes(FD, cast<llvm::GlobalValue>(Resolver));
- if (ResolverGV)
- replaceDeclarationWith(ResolverGV, Resolver);
return Resolver;
}
diff --git a/clang/test/CodeGen/attr-cpuspecific.c b/clang/test/CodeGen/attr-cpuspecific.c
index 628892d5809b4e..91f6c9e9e06b88 100644
--- a/clang/test/CodeGen/attr-cpuspecific.c
+++ b/clang/test/CodeGen/attr-cpuspecific.c
@@ -114,8 +114,8 @@ void ThreeVersionsSameAttr(void){}
// CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
ATTR(cpu_specific(knl))
-void CpuSpecificNoDispatch(void) {}
-// CHECK: define {{.*}}void @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+void CpuSpecificNoDispatch(void (*f)(void)) {}
+// CHECK: define {{.*}}void @CpuSpecificNoDispatch.Z(ptr noundef %f) #[[K:[0-9]+]]
ATTR(cpu_dispatch(knl))
void OrderDispatchUsageSpecific(void);
@@ -151,9 +151,9 @@ void usages(void) {
ThreeVersionsSameAttr();
// LINUX: @ThreeVersionsSameAttr.ifunc()
// WINDOWS: @ThreeVersionsSameAttr()
- CpuSpecificNoDispatch();
- // LINUX: @CpuSpecificNoDispatch.ifunc()
- // WINDOWS: @CpuSpecificNoDispatch()
+ CpuSpecificNoDispatch((void (*)(void))CpuSpecificNoDispatch);
+ // LINUX: @CpuSpecificNoDispatch.ifunc(ptr noundef @CpuSpecificNoDispatch.ifunc)
+ // WINDOWS: @CpuSpecificNoDispatch(ptr noundef @CpuSpecificNoDispatch)
OrderDispatchUsageSpecific();
// LINUX: @OrderDispatchUsageSpecific.ifunc()
// WINDOWS: @OrderDispatchUsageSpecific()
@@ -162,7 +162,7 @@ void usages(void) {
// WINDOWS: @OrderSpecificUsageDispatch()
}
-// LINUX: declare void @CpuSpecificNoDispatch.ifunc()
+// LINUX: declare void @CpuSpecificNoDispatch.ifunc(ptr)
// has an extra config to emit!
ATTR(cpu_dispatch(ivybridge, knl, atom))
>From 7e61a41da7f2bc58b320f88f767db8312ccc1e1a Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Fri, 15 Nov 2024 15:14:11 +0000
Subject: [PATCH 2/3] changes from last revision:
* changed the condition for early exit
* assert that the first lookup returned null if you are at the fallthrough
---
clang/lib/CodeGen/CodeGenModule.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 27b1ccb8137356..f0f5f52e0eecab 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4554,6 +4554,9 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
ResolverName += ".resolver";
}
+ bool ShouldReturnIFunc =
+ getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion();
+
// If the resolver has already been created, just return it. This lookup may
// yield a function declaration instead of a resolver on AArch64. That is
// because we didn't know whether a resolver will be generated when we first
@@ -4561,8 +4564,7 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
// targets which support ifuncs should not return here unless we actually
// found an ifunc.
llvm::GlobalValue *ResolverGV = GetGlobalValue(ResolverName);
- if (ResolverGV &&
- (isa<llvm::GlobalIFunc>(ResolverGV) || !getTarget().supportsIFunc()))
+ if (ResolverGV && (isa<llvm::GlobalIFunc>(ResolverGV) || !ShouldReturnIFunc))
return ResolverGV;
const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD);
@@ -4575,7 +4577,7 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
// For cpu_specific, don't create an ifunc yet because we don't know if the
// cpu_dispatch will be emitted in this translation unit.
- if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
+ if (ShouldReturnIFunc) {
unsigned AS = getTypes().getTargetAddressSpace(FD->getType());
llvm::Type *ResolverType =
llvm::FunctionType::get(llvm::PointerType::get(DeclTy, AS), false);
@@ -4594,7 +4596,7 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
llvm::Constant *Resolver = GetOrCreateLLVMFunction(
ResolverName, DeclTy, GlobalDecl{}, /*ForVTable=*/false);
- assert(isa<llvm::GlobalValue>(Resolver) &&
+ assert(isa<llvm::GlobalValue>(Resolver) && !ResolverGV &&
"Resolver should be created for the first time");
SetCommonAttributes(FD, cast<llvm::GlobalValue>(Resolver));
return Resolver;
>From e0854c296ce503135d5ed8b3a1624ceb76e04f18 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Fri, 15 Nov 2024 17:10:17 +0000
Subject: [PATCH 3/3] Retain the original test and add a new one to reproduce
the crash.
---
clang/test/CodeGen/attr-cpuspecific.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/clang/test/CodeGen/attr-cpuspecific.c b/clang/test/CodeGen/attr-cpuspecific.c
index 91f6c9e9e06b88..6eb2fb27587383 100644
--- a/clang/test/CodeGen/attr-cpuspecific.c
+++ b/clang/test/CodeGen/attr-cpuspecific.c
@@ -114,8 +114,8 @@ void ThreeVersionsSameAttr(void){}
// CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
ATTR(cpu_specific(knl))
-void CpuSpecificNoDispatch(void (*f)(void)) {}
-// CHECK: define {{.*}}void @CpuSpecificNoDispatch.Z(ptr noundef %f) #[[K:[0-9]+]]
+void CpuSpecificNoDispatch(void) {}
+// CHECK: define {{.*}}void @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
ATTR(cpu_dispatch(knl))
void OrderDispatchUsageSpecific(void);
@@ -151,9 +151,15 @@ void usages(void) {
ThreeVersionsSameAttr();
// LINUX: @ThreeVersionsSameAttr.ifunc()
// WINDOWS: @ThreeVersionsSameAttr()
- CpuSpecificNoDispatch((void (*)(void))CpuSpecificNoDispatch);
- // LINUX: @CpuSpecificNoDispatch.ifunc(ptr noundef @CpuSpecificNoDispatch.ifunc)
- // WINDOWS: @CpuSpecificNoDispatch(ptr noundef @CpuSpecificNoDispatch)
+ CpuSpecificNoDispatch();
+ // LINUX: @CpuSpecificNoDispatch.ifunc()
+ // WINDOWS: @CpuSpecificNoDispatch()
+ //
+ // Adding another use of CpuSpecificNoDispatch reproduces the
+ // crash in https://github.com/llvm/llvm-project/issues/115299
+ CpuSpecificNoDispatch();
+ // LINUX: @CpuSpecificNoDispatch.ifunc()
+ // WINDOWS: @CpuSpecificNoDispatch()
OrderDispatchUsageSpecific();
// LINUX: @OrderDispatchUsageSpecific.ifunc()
// WINDOWS: @OrderDispatchUsageSpecific()
@@ -162,7 +168,7 @@ void usages(void) {
// WINDOWS: @OrderSpecificUsageDispatch()
}
-// LINUX: declare void @CpuSpecificNoDispatch.ifunc(ptr)
+// LINUX: declare void @CpuSpecificNoDispatch.ifunc()
// has an extra config to emit!
ATTR(cpu_dispatch(ivybridge, knl, atom))
More information about the cfe-commits
mailing list