erichkeane updated this revision to Diff 419741.
erichkeane marked 9 inline comments as done.
erichkeane added a comment.
Fix all the things @tahonermann found.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122608/new/
https://reviews.llvm.org/D122608
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
clang/test/SemaCXX/externc-ifunc-resolver.cpp
Index: clang/test/SemaCXX/externc-ifunc-resolver.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/externc-ifunc-resolver.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple x86_64-linux-gnu -verify %s
+
+extern "C" {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+namespace NS {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+} // namespace NS
+
+// FIXME: This diagnostic is pretty confusing, the issue is that the existence
+// of the two functions suppresses the 'alias' creation, and thus the ifunc
+// resolution via the alias as well. In the future we should probably find
+// some way to improve this diagnostic (likely by diagnosing when we decide
+// this case suppresses alias creation).
+__attribute__((ifunc("resolve_foo"))) void foo(); // expected-error{{ifunc must point to a defined function}}
+}
+
Index: clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/externc-ifunc-resolver.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+extern "C" {
+__attribute__((used)) static void *resolve_foo() { return 0; }
+__attribute__((ifunc("resolve_foo"))) char *foo();
+__attribute__((ifunc("resolve_foo"))) void foo2(int);
+__attribute__((ifunc("resolve_foo"))) char foo3(float);
+__attribute__((ifunc("resolve_foo"))) char foo4(float);
+}
+
+// CHECK: @resolve_foo = internal alias i8* (), i8* ()* @_ZL11resolve_foov
+// CHECK: @foo = ifunc i8* (), bitcast (i8* ()* @_ZL11resolve_foov to i8* ()* ()*)
+// CHECK: @foo2 = ifunc void (i32), bitcast (i8* ()* @_ZL11resolve_foov to void (i32)* ()*)
+// CHECK: @foo3 = ifunc i8 (float), bitcast (i8* ()* @_ZL11resolve_foov to i8 (float)* ()*)
+// CHECK: @foo4 = ifunc i8 (float), bitcast (i8* ()* @_ZL11resolve_foov to i8 (float)* ()*)
+// CHECK: define internal noundef i8* @_ZL11resolve_foov()
Index: clang/lib/CodeGen/CodeGenModule.h
===================================================================
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1570,6 +1570,12 @@
/// Emit the link options introduced by imported modules.
void EmitModuleLinkOptions();
+ /// Helper function for EmitStaticExternCAliases that clears the uses of
+ /// 'Elem' if it is used exclusively by ifunc resolvers. Returns 'true' if it
+ /// was successful erases Elem.
+ bool CheckAndReplaceExternCIFuncs(llvm::GlobalValue *Elem,
+ llvm::GlobalValue *CppFunc);
+
/// Emit aliases for internal-linkage declarations inside "C" language
/// linkage specifications, giving them the "expected" name where possible.
void EmitStaticExternCAliases();
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -507,7 +507,6 @@
EmitVTablesOpportunistically();
applyGlobalValReplacements();
applyReplacements();
- checkAliases();
emitMultiVersionFunctions();
EmitCXXGlobalInitFunc();
EmitCXXGlobalCleanUpFunc();
@@ -539,6 +538,7 @@
EmitCtorList(GlobalDtors, "llvm.global_dtors");
EmitGlobalAnnotations();
EmitStaticExternCAliases();
+ checkAliases();
EmitDeferredUnusedCoverageMappings();
CodeGenPGO(*this).setValueProfilingFlag(getModule());
if (CoverageMapping)
@@ -6315,6 +6315,68 @@
GlobalMetadata->addOperand(llvm::MDNode::get(CGM.getLLVMContext(), Ops));
}
+bool CodeGenModule::CheckAndReplaceExternCIFuncs(llvm::GlobalValue *Elem,
+ llvm::GlobalValue *CppFunc) {
+ // Store the list of ifuncs we need to replace uses in.
+ llvm::SmallVector<llvm::GlobalIFunc *> IFuncs;
+ // List of ConstantExprs that we should be able to delete when we're done
+ // here.
+ llvm::SmallVector<llvm::ConstantExpr *> CEs;
+
+ // First make sure that all users of this are ifuncs (or ifuncs via a
+ // bitcast), and collect the list of ifuncs and CEs so we can work on them
+ // later.
+ for (llvm::User *User : Elem->users()) {
+ // Users can either be a bitcast ConstExpr that is used by the ifuncs, OR an
+ // ifunc directly. In any other case, just give up, as we don't know what we
+ // could break by changing those.
+ if (auto *ConstExpr = dyn_cast<llvm::ConstantExpr>(User)) {
+ if (ConstExpr->getOpcode() != llvm::Instruction::BitCast)
+ return false;
+
+ for (llvm::User *CEUser : ConstExpr->users()) {
+ if (auto *IFunc = dyn_cast<llvm::GlobalIFunc>(CEUser)) {
+ IFuncs.push_back(IFunc);
+ } else {
+ return false;
+ }
+ }
+ CEs.push_back(ConstExpr);
+ } else if (auto *IFunc = dyn_cast<llvm::GlobalIFunc>(User)) {
+ IFuncs.push_back(IFunc);
+ } else {
+ // This user is one we don't know how to handle, so fail redirection. This
+ // will result in an ifunc retaining a resolver name that will ultimately
+ // fail to be resolved to a defined function.
+ return false;
+ }
+ }
+
+ // Now we know this is a valid case where we can do this alias replacement, we
+ // need to remove all of the references to Elem (and the bitcasts!) so we can
+ // delete it.
+ for (llvm::GlobalIFunc *IFunc : IFuncs)
+ IFunc->setResolver(nullptr);
+ for (llvm::ConstantExpr *ConstExpr : CEs)
+ ConstExpr->destroyConstant();
+
+ // We should now be out of uses for the 'old' version of this function, so we
+ // can erase it as well.
+ Elem->eraseFromParent();
+
+ for (llvm::GlobalIFunc *IFunc : IFuncs) {
+ // The type of the resolver is always just a function-type that returns the
+ // type of the IFunc, so create that here. If the type of the actual
+ // resolver doesn't match, it just gets bitcast to the right thing.
+ auto *ResolverTy =
+ llvm::FunctionType::get(IFunc->getType(), /*isVarArg*/ false);
+ llvm::Constant *Resolver = GetOrCreateLLVMFunction(
+ CppFunc->getName(), ResolverTy, {}, /*ForVTable*/ false);
+ IFunc->setResolver(Resolver);
+ }
+ return true;
+}
+
/// For each function which is declared within an extern "C" region and marked
/// as 'used', but has internal linkage, create an alias from the unmangled
/// name to the mangled name if possible. People expect to be able to refer
@@ -6326,7 +6388,19 @@
for (auto &I : StaticExternCValues) {
IdentifierInfo *Name = I.first;
llvm::GlobalValue *Val = I.second;
- if (Val && !getModule().getNamedValue(Name->getName()))
+
+ // If Val is null, that implies there were multiple declarations that each
+ // had a claim to the unmangled name. In this case, generation of hte alias
+ // is suppressed. See CodeGenModule::MaybeHandleStaticInExterC.
+ if (!Val)
+ break;
+
+ llvm::GlobalValue *ExistingElem =
+ getModule().getNamedValue(Name->getName());
+
+ // If there is either not something already by this name, or we were able to
+ // replace all uses from IFuncs, create the alias.
+ if (!ExistingElem || CheckAndReplaceExternCIFuncs(ExistingElem, Val))
addCompilerUsedGlobal(llvm::GlobalAlias::create(Name->getName(), Val));
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
