Author: Erich Keane Date: 2022-04-01T13:00:59-07:00 New Revision: 9ba8c4024b52bb8d0491fa1d15790ce7e9a9eae1
URL: https://github.com/llvm/llvm-project/commit/9ba8c4024b52bb8d0491fa1d15790ce7e9a9eae1 DIFF: https://github.com/llvm/llvm-project/commit/9ba8c4024b52bb8d0491fa1d15790ce7e9a9eae1.diff LOG: Fix behavior of ifuncs with 'used' extern "C" static functions We expect that `extern "C"` static functions to be usable in things like inline assembly, as well as ifuncs: See the bug report here: https://github.com/llvm/llvm-project/issues/54549 However, we were diagnosing this as 'not defined', because the ifunc's attempt to look up its resolver would generate a declared IR function. Additionally, as background, the way we allow these static extern "C" functions to work in inline assembly is by making an alias with the C mangling in MOST situations to the version we emit with internal-linkage/mangling. The problem here was multi-fold: First- We generated the alias after the ifunc was checked, so the function by that name didn't exist yet. Second, the ifunc's generation caused a symbol to exist under the name of the alias already (the declared function above), which suppressed the alias generation. This patch fixes all of this by moving the checking of ifuncs/CFE aliases until AFTER we have generated the extern-C alias. Then, it does a 'fixup' around the GlobalIFunc to make sure we correct the reference. Differential Revision: https://reviews.llvm.org/D122608 Added: clang/test/CodeGenCXX/externc-ifunc-resolver.cpp clang/test/SemaCXX/externc-ifunc-resolver.cpp Modified: clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 08117809c56fb..d637ab6ec288d 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -507,7 +507,6 @@ void CodeGenModule::Release() { EmitVTablesOpportunistically(); applyGlobalValReplacements(); applyReplacements(); - checkAliases(); emitMultiVersionFunctions(); EmitCXXGlobalInitFunc(); EmitCXXGlobalCleanUpFunc(); @@ -539,6 +538,7 @@ void CodeGenModule::Release() { EmitCtorList(GlobalDtors, "llvm.global_dtors"); EmitGlobalAnnotations(); EmitStaticExternCAliases(); + checkAliases(); EmitDeferredUnusedCoverageMappings(); CodeGenPGO(*this).setValueProfilingFlag(getModule()); if (CoverageMapping) @@ -6348,6 +6348,68 @@ static void EmitGlobalDeclMetadata(CodeGenModule &CGM, 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 @@ -6359,7 +6421,19 @@ void CodeGenModule::EmitStaticExternCAliases() { 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 the 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)); } } diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index d8ec8f0c09e65..a95949109c371 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1576,6 +1576,16 @@ class CodeGenModule : public CodeGenTypeCache { /// Emit the link options introduced by imported modules. void EmitModuleLinkOptions(); + /// Helper function for EmitStaticExternCAliases() to redirect ifuncs that + /// have a resolver name that matches 'Elem' to instead resolve to the name of + /// 'CppFunc'. This redirection is necessary in cases where 'Elem' has a name + /// that will be emitted as an alias of the name bound to 'CppFunc'; ifuncs + /// may not reference aliases. Redirection is only performed if 'Elem' is only + /// used by ifuncs in which case, 'Elem' is destroyed. 'true' is returned if + /// redirection is successful, and 'false' is returned otherwise. + 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(); diff --git a/clang/test/CodeGenCXX/externc-ifunc-resolver.cpp b/clang/test/CodeGenCXX/externc-ifunc-resolver.cpp new file mode 100644 index 0000000000000..11bd8f77cf5e1 --- /dev/null +++ b/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() diff --git a/clang/test/SemaCXX/externc-ifunc-resolver.cpp b/clang/test/SemaCXX/externc-ifunc-resolver.cpp new file mode 100644 index 0000000000000..70c21b0ac7f9b --- /dev/null +++ b/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}} +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
