[clang] [clang][FMV] Fix crash with cpu_specific attribute. (PR #115762)

[Alexandros Lamprineas via cfe-commits]

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/2] [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/2] 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;