From 92d8e063e8d95118b8ebe44ed6fc24743923ed1f Mon Sep 17 00:00:00 2001 From: Dario Rexin Date: Mon, 8 Dec 2025 11:30:22 -0800 Subject: [PATCH] [IRGen] Use proper linkage for async function pointers to partial apply forwarders rdar://163631865 Under certain circumstances the same symbol can be used for different implementations of the forwarders. The forwarders themselves are already emitted with "internal" LLVM linkage, but some particularities in the mapping from SILLinkage to LLVM linkage caused async function pointers to partial apply forwarders to be emitted with linkonce_odr instead, which caused the wrong forwarder to be called at runtime, causing unexpected behavior and crashes. --- include/swift/IRGen/Linking.h | 10 ++++++ lib/IRGen/GenDecl.cpp | 33 +++++++++++--------- test/IRGen/async/partial_apply_forwarder.sil | 5 +++ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/include/swift/IRGen/Linking.h b/include/swift/IRGen/Linking.h index e9673973b5163..fe8a53a4b05d6 100644 --- a/include/swift/IRGen/Linking.h +++ b/include/swift/IRGen/Linking.h @@ -1850,6 +1850,16 @@ class LinkEntity { bool isTypeKind() const { return isTypeKind(getKind()); } bool isAlwaysSharedLinkage() const; + + /// Partial apply forwarders always need real private linkage, + /// to ensure the correct implementation is used in case of + /// colliding symbols. + bool privateMeansPrivate() const { + return getKind() == Kind::PartialApplyForwarder || + getKind() == Kind::PartialApplyForwarderAsyncFunctionPointer || + getKind() == Kind::PartialApplyForwarderCoroFunctionPointer; + } + #undef LINKENTITY_GET_FIELD #undef LINKENTITY_SET_FIELD diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp index 4fd5cc0827a91..55913922b4f9c 100644 --- a/lib/IRGen/GenDecl.cpp +++ b/lib/IRGen/GenDecl.cpp @@ -2242,10 +2242,10 @@ void IRGenerator::emitEntryPointInfo() { IGM.addUsedGlobal(var); } -static IRLinkage -getIRLinkage(StringRef name, const UniversalLinkageInfo &info, - SILLinkage linkage, ForDefinition_t isDefinition, - bool isWeakImported, bool isKnownLocal = false) { +static IRLinkage getIRLinkage(StringRef name, const UniversalLinkageInfo &info, + SILLinkage linkage, ForDefinition_t isDefinition, + bool isWeakImported, bool isKnownLocal, + bool privateMeansPrivate) { #define RESULT(LINKAGE, VISIBILITY, DLL_STORAGE) \ IRLinkage{llvm::GlobalValue::LINKAGE##Linkage, \ llvm::GlobalValue::VISIBILITY##Visibility, \ @@ -2302,12 +2302,14 @@ getIRLinkage(StringRef name, const UniversalLinkageInfo &info, case SILLinkage::Private: { if (info.forcePublicDecls() && !isDefinition) return getIRLinkage(name, info, SILLinkage::PublicExternal, isDefinition, - isWeakImported, isKnownLocal); - - auto linkage = info.needLinkerToMergeDuplicateSymbols() - ? llvm::GlobalValue::LinkOnceODRLinkage - : llvm::GlobalValue::InternalLinkage; - auto visibility = info.shouldAllPrivateDeclsBeVisibleFromOtherFiles() + isWeakImported, isKnownLocal, privateMeansPrivate); + + auto linkage = + info.needLinkerToMergeDuplicateSymbols() && !privateMeansPrivate + ? llvm::GlobalValue::LinkOnceODRLinkage + : llvm::GlobalValue::InternalLinkage; + auto visibility = info.shouldAllPrivateDeclsBeVisibleFromOtherFiles() && + !privateMeansPrivate ? llvm::GlobalValue::HiddenVisibility : llvm::GlobalValue::DefaultVisibility; return {linkage, visibility, llvm::GlobalValue::DefaultStorageClass}; @@ -2358,7 +2360,7 @@ void irgen::updateLinkageForDefinition(IRGenModule &IGM, auto IRL = getIRLinkage(global->hasName() ? global->getName() : StringRef(), linkInfo, entity.getLinkage(ForDefinition), ForDefinition, - weakImported, isKnownLocal); + weakImported, isKnownLocal, entity.privateMeansPrivate()); ApplyIRLinkage(IRL).to(global); LinkInfo link = LinkInfo::get(IGM, entity, ForDefinition); @@ -2409,9 +2411,9 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo, } bool weakImported = entity.isWeakImported(swiftModule); - result.IRL = getIRLinkage(result.Name, linkInfo, - entity.getLinkage(isDefinition), isDefinition, - weakImported, isKnownLocal); + result.IRL = getIRLinkage( + result.Name, linkInfo, entity.getLinkage(isDefinition), isDefinition, + weakImported, isKnownLocal, entity.privateMeansPrivate()); result.ForDefinition = isDefinition; return result; } @@ -2422,7 +2424,8 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo, StringRef name, LinkInfo result; result.Name += name; result.IRL = getIRLinkage(name, linkInfo, linkage, isDefinition, - isWeakImported, linkInfo.Internalize); + isWeakImported, linkInfo.Internalize, + /*privateMeansPrivate=*/false); result.ForDefinition = isDefinition; return result; } diff --git a/test/IRGen/async/partial_apply_forwarder.sil b/test/IRGen/async/partial_apply_forwarder.sil index 672a733ae157e..c0022e129befa 100644 --- a/test/IRGen/async/partial_apply_forwarder.sil +++ b/test/IRGen/async/partial_apply_forwarder.sil @@ -89,6 +89,11 @@ entry(%e : $EmptyType, %b : $WeakBox<τ_0_0>): unreachable } +// CHECK-DAG: @"$s7takingQTATu" = internal +// CHECK-DAG: @"$s11takingQAndSTATu" = internal +// CHECK-DAG: @"$s13inner_closureTATu" = internal +// CHECK-DAG: @"$s15returns_closureTATu" = internal + // CHECK-LABEL: define{{( dllexport)?}}{{( protected)?}} swift{{(tail)?}}cc void @bind_polymorphic_param_from_context( // CHECK-LABEL: define internal swift{{(tail)?}}cc void @"$s7takingQTA"( sil public @bind_polymorphic_param_from_context : $@async @convention(thin) <τ_0_1>(@in τ_0_1) -> @owned @async @callee_guaranteed () -> () {