ibookstein created this revision.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
The purpose of this change is to fix the following codegen bug:
// main.c
__attribute__((cpu_specific(generic)))
int *foo(void) { static int z; return &z;}
int main() { return *foo() = 5; }
// other.c
__attribute__((cpu_dispatch(generic))) int *foo(void);
// run:
clang main.c other.c -o main; ./main
This will segfault prior to the change, and return the correct
exit code 5 after the change.
The underlying cause is that when a translation unit contains
a cpu_specific function without the corresponding cpu_dispatch
the generated code binds the reference to foo() against a
GlobalIFunc whose resolver is undefined. This is invalid: the
resolver must be defined in the same translation unit as the
ifunc, but historically the LLVM bitcode verifier did not check
that. The generated code then binds against the resolver rather
than the ifunc, so it ends up calling the resolver rather than
the resolvee. In the example above it treats its return value as
an int *, therefore trying to write to program text.
The root issue at the representation level is that GlobalIFunc,
like GlobalAlias, does not support a "declaration" state. The
object which provides the correct semantics in these cases
is a Function declaration, but unlike Functions, changing a
declaration to a definition in the GlobalIFunc case constitutes
a change of the object type, as opposed to simply emitting code
into a Function.
I think this limitation is unlikely to change, so I implemented
the fix by returning a function declaration rather than an ifunc
when encountering cpu_specific, and upgrading it to an ifunc
when emitting cpu_dispatch.
This uses `takeName` + `replaceAllUsesWith` in similar vein to
other places where the correct IR object type cannot be known
locally/up-front, like in `CodeGenModule::EmitAliasDefinition`.
Signed-off-by: Itay Bookstein <ibookst...@gmail.com>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120567
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/attr-cpuspecific.c
Index: clang/test/CodeGen/attr-cpuspecific.c
===================================================================
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,6 +8,7 @@
#endif // _MSC_VER
// Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
// LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
// LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
// LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
@@ -18,8 +19,8 @@
// LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
// LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
// LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
// LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
// LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
// LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
@@ -34,6 +35,21 @@
// LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
// WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
ATTR(cpu_specific(ivybridge))
void NotCalled(void){}
// LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +96,10 @@
// CHECK: define {{.*}}void @ThreeVersionsSameAttr.S() #[[S]]
// CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
+ATTR(cpu_specific(knl))
+int CpuSpecificNoDispatch(void) { return 1; }
+// CHECK: define {{.*}}i32 @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+
void usages(void) {
SingleVersion();
// LINUX: @SingleVersion.ifunc()
@@ -93,6 +113,9 @@
ThreeVersionsSameAttr();
// LINUX: @ThreeVersionsSameAttr.ifunc()
// WINDOWS: @ThreeVersionsSameAttr()
+ CpuSpecificNoDispatch();
+ // LINUX: @CpuSpecificNoDispatch.ifunc()
+ // WINDOWS: @CpuSpecificNoDispatch()
}
// has an extra config to emit!
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3593,16 +3593,28 @@
CGF.EmitMultiVersionResolver(ResolverFunc, Options);
if (getTarget().supportsIFunc()) {
+ llvm::GlobalValue::LinkageTypes Linkage = getMultiversionLinkage(*this, GD);
+ auto *IFunc = cast<llvm::GlobalValue>(
+ GetOrCreateMultiVersionResolver(GD, DeclTy, FD));
+
+ // Fix up function declarations that were created for cpu_specific before
+ // cpu_dispatch was known
+ if (!dyn_cast<llvm::GlobalIFunc>(IFunc)) {
+ assert(cast<llvm::Function>(IFunc)->isDeclaration());
+ auto *GI = llvm::GlobalIFunc::create(DeclTy, 0, Linkage, "", ResolverFunc,
+ &getModule());
+ GI->takeName(IFunc);
+ IFunc->replaceAllUsesWith(GI);
+ IFunc->eraseFromParent();
+ IFunc = GI;
+ }
+
std::string AliasName = getMangledNameImpl(
*this, GD, FD, /*OmitMultiVersionMangling=*/true);
llvm::Constant *AliasFunc = GetGlobalValue(AliasName);
if (!AliasFunc) {
- auto *IFunc = cast<llvm::GlobalIFunc>(GetOrCreateLLVMFunction(
- AliasName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,
- /*IsThunk=*/false, llvm::AttributeList(), NotForDefinition));
- auto *GA = llvm::GlobalAlias::create(DeclTy, 0,
- getMultiversionLinkage(*this, GD),
- AliasName, IFunc, &getModule());
+ auto *GA = llvm::GlobalAlias::create(DeclTy, 0, Linkage, AliasName, IFunc,
+ &getModule());
SetCommonAttributes(GD, GA);
}
}
@@ -3650,7 +3662,9 @@
}
}
- if (getTarget().supportsIFunc()) {
+ // 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()) {
llvm::Type *ResolverType = llvm::FunctionType::get(
llvm::PointerType::get(
DeclTy, getContext().getTargetAddressSpace(FD->getType())),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
