From 0fdca11e461a4943b43d208f16582d48762ec60c 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 | 9 +++++ lib/IRGen/GenDecl.cpp | 40 +++++++++++--------- test/IRGen/async/partial_apply_forwarder.sil | 5 +++ 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/include/swift/IRGen/Linking.h b/include/swift/IRGen/Linking.h index 6c811bf11dedf..cdc6c7799e7e7 100644 --- a/include/swift/IRGen/Linking.h +++ b/include/swift/IRGen/Linking.h @@ -1861,6 +1861,15 @@ class LinkEntity { 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; + } + /// Whether the link entity's definitions must be considered non-unique. /// /// This applies only in the Embedded Swift linkage model, and is used for diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp index 1a6921606165b..4759c49ac5a27 100644 --- a/lib/IRGen/GenDecl.cpp +++ b/lib/IRGen/GenDecl.cpp @@ -2324,11 +2324,11 @@ void IRGenerator::emitEntryPointInfo() { IGM.addUsedGlobal(var); } -static IRLinkage -getIRLinkage(StringRef name, const UniversalLinkageInfo &info, - SILLinkage linkage, ForDefinition_t isDefinition, - bool isWeakImported, bool isKnownLocal, - bool hasNonUniqueDefinition) { +static IRLinkage getIRLinkage(StringRef name, const UniversalLinkageInfo &info, + SILLinkage linkage, ForDefinition_t isDefinition, + bool isWeakImported, bool isKnownLocal, + bool hasNonUniqueDefinition, + bool privateMeansPrivate) { #define RESULT(LINKAGE, VISIBILITY, DLL_STORAGE) \ IRLinkage{llvm::GlobalValue::LINKAGE##Linkage, \ llvm::GlobalValue::VISIBILITY##Visibility, \ @@ -2390,12 +2390,15 @@ getIRLinkage(StringRef name, const UniversalLinkageInfo &info, case SILLinkage::Private: { if (info.forcePublicDecls() && !isDefinition) return getIRLinkage(name, info, SILLinkage::PublicExternal, isDefinition, - isWeakImported, isKnownLocal, hasNonUniqueDefinition); - - auto linkage = info.needLinkerToMergeDuplicateSymbols() - ? llvm::GlobalValue::LinkOnceODRLinkage - : llvm::GlobalValue::InternalLinkage; - auto visibility = info.shouldAllPrivateDeclsBeVisibleFromOtherFiles() + isWeakImported, isKnownLocal, hasNonUniqueDefinition, + 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}; @@ -2446,8 +2449,8 @@ void irgen::updateLinkageForDefinition(IRGenModule &IGM, auto IRL = getIRLinkage(global->hasName() ? global->getName() : StringRef(), linkInfo, entity.getLinkage(ForDefinition), ForDefinition, - weakImported, isKnownLocal, - entity.hasNonUniqueDefinition()); + weakImported, isKnownLocal, entity.hasNonUniqueDefinition(), + entity.privateMeansPrivate()); ApplyIRLinkage(IRL).to(global); LinkInfo link = LinkInfo::get(IGM, entity, ForDefinition); @@ -2498,10 +2501,10 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo, } bool weakImported = entity.isWeakImported(swiftModule); - result.IRL = getIRLinkage(result.Name, linkInfo, - entity.getLinkage(isDefinition), isDefinition, - weakImported, isKnownLocal, - entity.hasNonUniqueDefinition()); + result.IRL = getIRLinkage( + result.Name, linkInfo, entity.getLinkage(isDefinition), isDefinition, + weakImported, isKnownLocal, entity.hasNonUniqueDefinition(), + entity.privateMeansPrivate()); result.ForDefinition = isDefinition; return result; } @@ -2513,7 +2516,8 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo, StringRef name, result.Name += name; result.IRL = getIRLinkage(name, linkInfo, linkage, isDefinition, isWeakImported, linkInfo.Internalize, - /*hasNonUniqueDefinition=*/false); + /*hasNonUniqueDefinition=*/false, + /*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 () -> () {