SILOptimizer: prevent illegal load forwarding of function references in global variables.

We cannot replace a load from a global let-variable with a function_ref, if the referenced function would violate the resilience rules.
That means if a non-public function_ref would be inlined into a function which is serialized.

Author: eeckstein

include/swift/SILOptimizer/Utils/BasicBlockOptUtils.h

@@ -320,7 +320,9 @@ class StaticInitCloner : public SILCloner<StaticInitCloner> {
   /// Add \p InitVal and all its operands (transitively) for cloning.
   ///
   /// Note: all init values must are added, before calling clone().
-  void add(SILInstruction *initVal);
+  /// Returns false if cloning is not possible, e.g. if we would end up cloning
+  /// a reference to a private function into a function which is serialized.
+  bool add(SILInstruction *initVal);
 
   /// Clone \p InitVal and all its operands into the initializer of the
   /// SILGlobalVariable.
@@ -332,7 +334,9 @@ class StaticInitCloner : public SILCloner<StaticInitCloner> {
   static void appendToInitializer(SILGlobalVariable *gVar,
                                   SingleValueInstruction *initVal) {
     StaticInitCloner cloner(gVar);
-    cloner.add(initVal);
+    bool success = cloner.add(initVal);
+    (void)success;
+    assert(success && "adding initVal cannot fail for a global variable");
     cloner.clone(initVal);
   }
 

lib/SILOptimizer/IPO/GlobalOpt.cpp

@@ -388,7 +388,8 @@ replaceLoadsByKnownValue(SILFunction *InitF, SILGlobalVariable *SILG,
     StaticInitCloner cloner(initCall);
     SmallVector<SILInstruction *, 8> insertedInsts;
     cloner.setTrackingList(&insertedInsts);
-    cloner.add(initVal);
+    if (!cloner.add(initVal))
+      continue;
 
     // Replace all loads from the addressor with the initial value of the global.
     replaceLoadsFromGlobal(initCall, initVal, cloner);
@@ -693,7 +694,8 @@ void SILGlobalOpt::optimizeGlobalAccess(SILGlobalVariable *SILG,
       continue;
 
     StaticInitCloner cloner(globalAddr);
-    cloner.add(initVal);
+    if (!cloner.add(initVal))
+      continue;
 
     // Replace all loads from the addressor with the initial value of the global.
     replaceLoadsFromGlobal(globalAddr, initVal, cloner);

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -959,8 +959,9 @@ SILInstruction *SILCombiner::visitLoadInst(LoadInst *LI) {
   // globals, which can occur with cross-module optimization.
   if (SingleValueInstruction *initVal = getValueFromStaticLet(LI->getOperand())) {
     StaticInitCloner cloner(LI);
-    cloner.add(initVal);
-    return cloner.clone(initVal);
+    if (cloner.add(initVal)) {
+      return cloner.clone(initVal);
+    }
   }
 
   // If we have a load [copy] whose only non-debug users are destroy_value, just

lib/SILOptimizer/Utils/BasicBlockOptUtils.cpp

@@ -247,10 +247,19 @@ bool SinkAddressProjections::cloneProjections() {
   return true;
 }
 
-void StaticInitCloner::add(SILInstruction *initVal) {
+bool StaticInitCloner::add(SILInstruction *initVal) {
   // Don't schedule an instruction twice for cloning.
   if (numOpsToClone.count(initVal) != 0)
-    return;
+    return true;
+
+  if (auto *funcRef = dyn_cast<FunctionRefInst>(initVal)) {
+    // We cannot inline non-public functions into functions which are serialized.
+    if (!getBuilder().isInsertingIntoGlobal() &&
+        getBuilder().getFunction().isSerialized() &&
+        !funcRef->getReferencedFunction()->hasValidLinkageForFragileRef()) {
+      return false;
+    }
+  }
 
   ArrayRef<Operand> operands = initVal->getAllOperands();
   numOpsToClone[initVal] = operands.size();
@@ -261,9 +270,11 @@ void StaticInitCloner::add(SILInstruction *initVal) {
   } else {
     // Recursively add all operands.
     for (const Operand &operand : operands) {
-      add(cast<SingleValueInstruction>(operand.get()));
+      if (!add(cast<SingleValueInstruction>(operand.get())))
+        return false;
     }
   }
+  return true;
 }
 
 SingleValueInstruction *

test/SILOptimizer/sil_combine_ossa.sil

@@ -4497,6 +4497,31 @@ bb0:
   return %1 : $Int64
 }
 
+sil private [ossa] @somePrivateFunction : $@convention(thin) () -> Int64 {
+bb0:
+  %0 = integer_literal $Builtin.Int64, 27
+  %1 = struct $Int64 (%0 : $Builtin.Int64)
+  return %1 : $Int64
+}
+
+sil_global [let] @pointerToSomePrivateFunction : $@callee_guaranteed () -> Int64 = {
+  %0 = function_ref @somePrivateFunction: $@convention(thin) () -> Int64
+  %initval = thin_to_thick_function %0 : $@convention(thin) () -> Int64 to $@callee_guaranteed () -> Int64
+}
+
+// CHECK-LABEL: sil [serialized] [ossa] @dontInlinePrivateFunctionRefIntoSerializedFunction
+// CHECK:         [[GA:%[0-9]+]] = global_addr
+// CHECK:         [[L:%[0-9]+]] = load [copy] [[GA]]
+// CHECK:         return [[L]]
+// CHECK:       } // end sil function 'dontInlinePrivateFunctionRefIntoSerializedFunction'
+sil [serialized] [ossa] @dontInlinePrivateFunctionRefIntoSerializedFunction : $@convention(thin) () -> @owned @callee_guaranteed () -> Int64 {
+bb0:
+  %0 = global_addr @pointerToSomePrivateFunction : $*@callee_guaranteed () -> Int64
+  %1 = load [copy] %0 : $*@callee_guaranteed () -> Int64
+  return %1 : $@callee_guaranteed () -> Int64
+}
+
+
 // Check for disabled optimization of unchecked_bitwise_cast to unchecked_ref_cast in ossa
 // This test can be optimized when ossa is supported in the SILCombine for unchecked_bitwise_cast
 // CHECK-LABEL: sil [ossa] @refcast :