erichkeane created this revision. erichkeane added reviewers: tahonermann, aaron.ballman, cor3ntin. Herald added a project: All. erichkeane requested review of this revision.
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. https://reviews.llvm.org/D122608 Files: clang/lib/CodeGen/CodeGenModule.cpp 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,10 @@ +// RUN: %clang_cc1 -emit-llvm-only -verify %s + +extern "C" { + __attribute__((used)) static void *resolve_foo() { return 0; } + namespace NS { + __attribute__((used)) static void *resolve_foo() { return 0; } + } + __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,10 @@ +// 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"))) void foo(); +} + +// CHECK: @resolve_foo = internal alias i8* (), i8* ()* @_ZL11resolve_foov +// CHECK: @foo = ifunc void (), bitcast (i8* ()* @_ZL11resolve_foov to void ()* ()*) +// CHECK: define internal noundef i8* @_ZL11resolve_foov() 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,27 @@ GlobalMetadata->addOperand(llvm::MDNode::get(CGM.getLLVMContext(), Ops)); } +/// IF all uses of the GV are from an IFunc resolver, which can happen when the +/// IFunc resolver is a static-function, but the name ends up being different, +/// return the IFunc so it can have its resolver replaced with the correct +/// 'alias' from EmitStaticExternCAliases. +static llvm::GlobalIFunc *getIFuncFromResolver(llvm::GlobalValue *GV) { + // If there is more than 1 use, I don't really know what we can do. + // EmitStaticExternCAliases won't be able to do anything, and the ifunc + // diagnostic will have to catch this. + if (!GV || std::distance(GV->use_begin(), GV->use_end()) != 1) + return nullptr; + + llvm::User *U = GV->use_begin()->getUser(); + if (auto *IFunc = dyn_cast_or_null<llvm::GlobalIFunc>(U)) { + assert(IFunc->getResolver() == GV && "Didn't find the resolver use?"); + assert(GV->isDeclaration() && isa<llvm::Function>(GV) && + "Resolver is defined or not a function?"); + return IFunc; + } + return nullptr; +} + /// 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,8 +6347,35 @@ for (auto &I : StaticExternCValues) { IdentifierInfo *Name = I.first; llvm::GlobalValue *Val = I.second; - if (Val && !getModule().getNamedValue(Name->getName())) - addCompilerUsedGlobal(llvm::GlobalAlias::create(Name->getName(), Val)); + llvm::GlobalValue *ExistingElem = + getModule().getNamedValue(Name->getName()); + llvm::GlobalIFunc *IFunc = getIFuncFromResolver(ExistingElem); + + if (Val && (!ExistingElem || IFunc)) { + // If the IFunc refers to this existing element we have to delete it + // FIRST, otherwise the 'name' of the new element will be wrong. + llvm::Type *ResolverTy = nullptr; + if (IFunc) { + ResolverTy = IFunc->getResolverFunction()->getFunctionType(); + IFunc->setResolver(nullptr); + assert(ExistingElem && "Got IFunc without an existing element?"); + ExistingElem->eraseFromParent(); + } + + auto *Alias = llvm::GlobalAlias::create(Name->getName(), Val); + addCompilerUsedGlobal(Alias); + + // Make sure we 'fix up' the IFunc's resolver. Despite it having been the + // 'right' name, we have to make sure we reassign the resolver to the + // original static function, since IFuncs aren't allowed to refer to the + // alias directly. SO, we have to de-alias it here. We have to make sure + // the correct bitcasts happen, so we have to re-look up the function. + if (IFunc) { + llvm::Constant *Resolver = GetOrCreateLLVMFunction( + Val->getName(), ResolverTy, {}, /*ForVTable*/ false); + IFunc->setResolver(Resolver); + } + } } }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits