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

Author: tshortli

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());

lib/SILGen/SILGen.h

@@ -66,7 +66,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
   llvm::DenseMap<SILDeclRef, SILDeclRef> delayedFunctions;
 
   /// Queue of delayed SILFunctions that need to be forced.
-  std::deque<SILDeclRef> forcedFunctions;
+  std::deque<SILDeclRef> pendingForcedFunctions;
+
+  /// Delayed SILFunctions that need to be forced.
+  llvm::DenseSet<SILDeclRef> forcedFunctions;
 
   /// Mapping global VarDecls to their onceToken and onceFunc, respectively.
   llvm::DenseMap<VarDecl *, std::pair<SILGlobalVariable *,

test/attr/Inputs/BackDeployHelper.swift

@@ -151,6 +151,13 @@ extension IntArray {
   public var rawValues: [Int] {
     _read { yield _values }
   }
+
+  @available(BackDeploy 1.0, *)
+  @_backDeploy(before: BackDeploy 2.0)
+  public mutating func removeLast() -> 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 {

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)
 }