From bffe9a669f10ea8c61b86896dbc571c5d44fff0c Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Fri, 2 Dec 2022 16:14:12 -0800 Subject: [PATCH 1/2] Tests: Add a SILGen test for async functions with the @_backDeploy attribute. --- .../back_deploy_attribute_async_func.swift | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 test/SILGen/back_deploy_attribute_async_func.swift diff --git a/test/SILGen/back_deploy_attribute_async_func.swift b/test/SILGen/back_deploy_attribute_async_func.swift new file mode 100644 index 0000000000000..59b777176698d --- /dev/null +++ b/test/SILGen/back_deploy_attribute_async_func.swift @@ -0,0 +1,59 @@ +// RUN: %target-swift-emit-sil -parse-as-library -module-name back_deploy %s -target %target-cpu-apple-macosx10.50 -verify +// RUN: %target-swift-emit-silgen -parse-as-library -module-name back_deploy %s | %FileCheck %s +// RUN: %target-swift-emit-silgen -parse-as-library -module-name back_deploy %s -target %target-cpu-apple-macosx10.50 | %FileCheck %s +// RUN: %target-swift-emit-silgen -parse-as-library -module-name back_deploy %s -target %target-cpu-apple-macosx10.60 | %FileCheck %s + +// REQUIRES: OS=macosx +// REQUIRES: concurrency + +@available(macOS 10.51, *) +@usableFromInline func otherFunc() async {} + +// -- Fallback definition of asyncFunc() +// CHECK: sil non_abi [serialized] [ossa] @$s11back_deploy9asyncFuncyyYaFTwB : $@convention(thin) @async () -> () +// CHECK: bb0: +// CHECK: [[FNREF:%.*]] = function_ref @$s11back_deploy9otherFuncyyYaF : $@convention(thin) @async () -> () +// CHECK: [[APPLY:%.*]] = apply [[FNREF]]() : $@convention(thin) @async () -> () +// CHECK: [[RESULT:%.*]] = tuple () +// CHECK: return [[RESULT]] : $() + +// -- Back deployment thunk for trivialFunc() +// CHECK-LABEL: sil non_abi [serialized] [thunk] [ossa] @$s11back_deploy9asyncFuncyyYaFTwb : $@convention(thin) @async () -> () +// CHECK: bb0: +// CHECK: [[MAJOR:%.*]] = integer_literal $Builtin.Word, 10 +// CHECK: [[MINOR:%.*]] = integer_literal $Builtin.Word, 52 +// CHECK: [[PATCH:%.*]] = integer_literal $Builtin.Word, 0 +// CHECK: [[OSVFN:%.*]] = function_ref @$ss26_stdlib_isOSVersionAtLeastyBi1_Bw_BwBwtF : $@convention(thin) (Builtin.Word, Builtin.Word, Builtin.Word) -> Builtin.Int1 +// CHECK: [[AVAIL:%.*]] = apply [[OSVFN]]([[MAJOR]], [[MINOR]], [[PATCH]]) : $@convention(thin) (Builtin.Word, Builtin.Word, Builtin.Word) -> Builtin.Int1 +// CHECK: cond_br [[AVAIL]], [[AVAIL_BB:bb[0-9]+]], [[UNAVAIL_BB:bb[0-9]+]] +// +// CHECK: [[UNAVAIL_BB]]: +// CHECK: [[FALLBACKFN:%.*]] = function_ref @$s11back_deploy9asyncFuncyyYaFTwB : $@convention(thin) @async () -> () +// CHECK: {{%.*}} = apply [[FALLBACKFN]]() : $@convention(thin) @async () -> () +// CHECK: br [[RETURN_BB:bb[0-9]+]] +// +// CHECK: [[AVAIL_BB]]: +// CHECK: [[ORIGFN:%.*]] = function_ref @$s11back_deploy9asyncFuncyyYaF : $@convention(thin) @async () -> () +// CHECK: {{%.*}} = apply [[ORIGFN]]() : $@convention(thin) @async () -> () +// CHECK: br [[RETURN_BB]] +// +// CHECK: [[RETURN_BB]] +// CHECK: [[RESULT:%.*]] = tuple () +// CHECK: return [[RESULT]] : $() + +// -- Original definition of trivialFunc() +// CHECK-LABEL: sil [available 10.52] [ossa] @$s11back_deploy9asyncFuncyyYaF : $@convention(thin) @async () -> () +@available(macOS 10.51, *) +@_backDeploy(before: macOS 10.52) +public func asyncFunc() async { + await otherFunc() +} + +// CHECK-LABEL: sil hidden [available 10.51] [ossa] @$s11back_deploy6calleryyYaF : $@convention(thin) @async () -> () +@available(macOS 10.51, *) +func caller() async { + // -- Verify the thunk is called + // CHECK: {{%.*}} = function_ref @$s11back_deploy9asyncFuncyyYaFTwb : $@convention(thin) @async () -> () + await asyncFunc() +} + From 95a9310e11ad6946b5898d3add1bf545b1b21117 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Tue, 6 Dec 2022 13:14:21 -0800 Subject: [PATCH 2/2] SILGen: Fix 'multiple definitions of symbol' error for functions with `@_backDeploy` containing defer blocks. If the emission of a function is delayed via `emitOrDelayFunction()`, then the function is recorded on a list of delayed functions. If the delayed function is later referenced, then the function moves from the delayed list to the "forced" list which will cause it to actually be emitted later. The implementation of `emitOrDelayFunction()` had a bug when called twice for the same delayable function - it would emit that function prematurely if the function moved to the forced list between the two invocations. Later, during forced function emission a multiple definitions of symbol diagnostic would be emitted since the function was not empty. This issue could be triggered by `@_backDeploy` functions that have auxilary delayable helper functions (e.g. defer blocks). SILGen visits `@_backDeploy` functions twice (once for the copy of the function emitted into the client and once for the resilient copy of the function) so `emitOrDelayFunction()` is called twice for each of the helper functions and the helper functions could become referenced between the two calls. The fix is to check for an existing entry in the forced functions list before delaying or emitting a delayable function. Resolves rdar://102909684 --- lib/SILGen/SILGen.cpp | 53 +++++++++++++---------- lib/SILGen/SILGen.h | 5 ++- test/attr/Inputs/BackDeployHelper.swift | 14 ++++++ test/attr/attr_backDeploy_evolution.swift | 8 +++- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 60de8cdd66405..3a2c0869f1cb6 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -732,15 +732,16 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant, emittedFunctions[constant] = F; - if (!delayedFunctions.count(constant)) { + auto foundDelayed = delayedFunctions.find(constant); + if (foundDelayed == delayedFunctions.end()) { if (isEmittedOnDemand(M, constant)) { - forcedFunctions.push_back(constant); + if (forcedFunctions.insert(constant).second) + pendingForcedFunctions.push_back(constant); return F; } } // If we delayed emitting this function previously, we need it now. - auto foundDelayed = delayedFunctions.find(constant); if (foundDelayed != delayedFunctions.end()) { // Move the function to its proper place within the module. M.functions.remove(F); @@ -752,7 +753,8 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant, M.functions.insertAfter(insertAfter->getIterator(), F); } - forcedFunctions.push_back(constant); + if (forcedFunctions.insert(constant).second) + pendingForcedFunctions.push_back(constant); delayedFunctions.erase(foundDelayed); } else { // We would have registered a delayed function as "last emitted" when we @@ -770,7 +772,6 @@ bool SILGenModule::hasFunction(SILDeclRef constant) { void SILGenModule::visitFuncDecl(FuncDecl *fd) { emitFunction(fd); } void SILGenModule::emitFunctionDefinition(SILDeclRef constant, SILFunction *f) { - if (!f->empty()) { diagnose(constant.getAsRegularLocation(), diag::sil_function_redefinition, f->getName()); @@ -1150,24 +1151,28 @@ static void emitOrDelayFunction(SILGenModule &SGM, SILDeclRef constant) { !constant.isDynamicallyReplaceable() && !isPossiblyUsedExternally(linkage, SGM.M.isWholeModule()); - // Avoid emitting a delayable definition if it hasn't already been referenced. - SILFunction *f = nullptr; - if (mayDelay) - f = SGM.getEmittedFunction(constant, ForDefinition); - else - f = SGM.getFunction(constant, ForDefinition); - - // If we don't want to emit now, remember how for later. - if (!f) { - SGM.delayedFunctions.insert({constant, emitAfter}); - // Even though we didn't emit the function now, update the - // lastEmittedFunction so that we preserve the original ordering that - // the symbols would have been emitted in. - SGM.lastEmittedFunction = constant; + if (!mayDelay) { + SGM.emitFunctionDefinition(constant, SGM.getFunction(constant, ForDefinition)); + return; + } + + // If the function is already forced then it was previously delayed and then + // referenced. We don't need to emit or delay it again. + if (SGM.forcedFunctions.contains(constant)) + return; + + if (auto *f = SGM.getEmittedFunction(constant, ForDefinition)) { + SGM.emitFunctionDefinition(constant, f); return; } - SGM.emitFunctionDefinition(constant, f); + // This is a delayable function so remember how to emit it in case it gets + // referenced later. + SGM.delayedFunctions.insert({constant, emitAfter}); + // Even though we didn't emit the function now, update the + // lastEmittedFunction so that we preserve the original ordering that + // the symbols would have been emitted in. + SGM.lastEmittedFunction = constant; } void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F, @@ -2222,13 +2227,13 @@ class SILGenModuleRAII { // Emit any delayed definitions that were forced. // Emitting these may in turn force more definitions, so we have to take // care to keep pumping the queues. - while (!SGM.forcedFunctions.empty() + while (!SGM.pendingForcedFunctions.empty() || !SGM.pendingConformances.empty()) { - while (!SGM.forcedFunctions.empty()) { - auto &front = SGM.forcedFunctions.front(); + while (!SGM.pendingForcedFunctions.empty()) { + auto &front = SGM.pendingForcedFunctions.front(); SGM.emitFunctionDefinition( front, SGM.getEmittedFunction(front, ForDefinition)); - SGM.forcedFunctions.pop_front(); + SGM.pendingForcedFunctions.pop_front(); } while (!SGM.pendingConformances.empty()) { SGM.getWitnessTable(SGM.pendingConformances.front()); diff --git a/lib/SILGen/SILGen.h b/lib/SILGen/SILGen.h index 122b757405778..ac4e66c71e736 100644 --- a/lib/SILGen/SILGen.h +++ b/lib/SILGen/SILGen.h @@ -66,7 +66,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor { llvm::DenseMap delayedFunctions; /// Queue of delayed SILFunctions that need to be forced. - std::deque forcedFunctions; + std::deque pendingForcedFunctions; + + /// Delayed SILFunctions that need to be forced. + llvm::DenseSet forcedFunctions; /// Mapping global VarDecls to their onceToken and onceFunc, respectively. llvm::DenseMap Int? { + defer { _values.removeLast() } + return _values.last + } } extension ReferenceIntArray { @@ -192,6 +199,13 @@ extension ReferenceIntArray { public final var rawValues: [Int] { _read { yield _values } } + + @available(BackDeploy 1.0, *) + @_backDeploy(before: BackDeploy 2.0) + public final func removeLast() -> Int? { + defer { _values.removeLast() } + return _values.last + } } extension Array { diff --git a/test/attr/attr_backDeploy_evolution.swift b/test/attr/attr_backDeploy_evolution.swift index e90cbdb8e7f08..9bf1c78a92fea 100644 --- a/test/attr/attr_backDeploy_evolution.swift +++ b/test/attr/attr_backDeploy_evolution.swift @@ -162,7 +162,9 @@ do { // CHECK-ABI: library: [5, 43, 3] // CHECK-BD: client: [5, 43, 3] - print(array.rawValues.print()) + array.rawValues.print() + + precondition(array.removeLast() == 3) } do { @@ -194,5 +196,7 @@ do { // CHECK-ABI: library: [7, 40, 1] // CHECK-BD: client: [7, 40, 1] - print(array.rawValues.print()) + array.rawValues.print() + + precondition(array.removeLast() == 1) }