[sil] Ban casting AnyObject with a guaranteed ownership forwarding checked_cast_br and fix up semantic-arc-opts given that.

The reason why I am doing this is that currently checked_cast_br of an AnyObject
may return a different value due to boxing. As an example, this can occur when
performing a checked_cast_br of a __SwiftValue(AnyHashable(Klass())).

To model this in SIL, I added to OwnershipForwardingMixin a bit of information
that states if the instruction directly forwards ownership with a default value
of true. In checked_cast_br's constructor though I specialize this behavior if
the source type is AnyObject and thus mark the checked_cast_br as not directly
forwarding its operand. If an OwnershipForwardingMixin directly forwards
ownership, one can assume that if it forwards ownership, it will always do so in
a way that ensures that each forwarded value is rc-identical to whatever result
it algebraically forwards ownership to. So in the context of checked_cast_br, it
means that the source value is rc-identical to the argument of the success and
default blocks.

I added a verifier check to CheckedCastBr that makes sure that it can never
forward guaranteed ownership and have a source type of AnyObject.

In SemanticARCOpts, I modified the code that builds extended live ranges of
owned values (*) to check if a OwnershipForwardingMixin is directly
forwarding. If it is not directly forwarding, then we treat the use just as an
unknown consuming use. This will then prevent us from converting such an owned
value to guaranteed appropriately in such a case.

I also in SILGen needed to change checked_cast_br emission in SILGenBuilder to
perform an ensurePlusOne on the input operand (converting it to +1 with an
appropriate cleanup) if the source type is an AnyObject. I found this via the
verifier check catching this behavior from SILGen when compiling libswift. This
just shows how important IR verification of invariants can be.

(*) For those unaware, SemanticARCOpts contains a model of an owned value and
all forwarding uses of it called an OwnershipLiveRange.

rdar://85710101

Author: gottesmm

include/swift/SIL/SILInstruction.h

@@ -1057,16 +1057,40 @@ class CopyLikeInstruction {
 /// initializes the kind field on our object is run before our constructor runs.
 class OwnershipForwardingMixin {
   ValueOwnershipKind ownershipKind;
+  bool directlyForwards;
 
 protected:
   OwnershipForwardingMixin(SILInstructionKind kind,
-                           ValueOwnershipKind ownershipKind)
-      : ownershipKind(ownershipKind) {
+                           ValueOwnershipKind ownershipKind,
+                           bool isDirectlyForwarding = true)
+      : ownershipKind(ownershipKind), directlyForwards(isDirectlyForwarding) {
     assert(isa(kind) && "Invalid subclass?!");
     assert(ownershipKind && "invalid forwarding ownership");
+    assert((directlyForwards || ownershipKind != OwnershipKind::Guaranteed) &&
+           "Non directly forwarding instructions can not forward guaranteed "
+           "ownership");
   }
 
 public:
+  /// If an instruction is directly forwarding, then any operand op whose
+  /// ownership it forwards into a result r must have the property that op and r
+  /// are "rc identical". This means that they are representing the same set of
+  /// underlying lifetimes (plural b/c of aggregates).
+  ///
+  /// An instruction that is not directly forwarding, can not have guaranteed
+  /// ownership since without direct forwarding, there isn't necessarily any
+  /// connection in between the operand's lifetime and the value's lifetime.
+  ///
+  /// An example of this is checked_cast_br where when performing the following:
+  ///
+  ///   __SwiftValue(AnyHashable(Klass())) to OtherKlass()
+  ///
+  /// we will look through the __SwiftValue(AnyHashable(X)) any just cast Klass
+  /// to OtherKlass. This means that the result argument would no longer be
+  /// rc-identical to the operand and default case and thus we can not propagate
+  /// forward any form of guaranteed ownership.
+  bool isDirectlyForwarding() const { return directlyForwards; }
+
   /// Forwarding ownership is determined by the forwarding instruction's
   /// constant ownership attribute. If forwarding ownership is owned, then the
   /// instruction moves an owned operand to its result, ending its lifetime. If
@@ -1081,6 +1105,9 @@ class OwnershipForwardingMixin {
     return ownershipKind;
   }
   void setForwardingOwnershipKind(ValueOwnershipKind newKind) {
+    assert((isDirectlyForwarding() || newKind != OwnershipKind::Guaranteed) &&
+           "Non directly forwarding instructions can not forward guaranteed "
+           "ownership");
     ownershipKind = newKind;
   }
 
@@ -7927,9 +7954,10 @@ class OwnershipForwardingTermInst : public TermInst,
 protected:
   OwnershipForwardingTermInst(SILInstructionKind kind,
                               SILDebugLocation debugLoc,
-                              ValueOwnershipKind ownershipKind)
+                              ValueOwnershipKind ownershipKind,
+                              bool isDirectlyForwarding = true)
       : TermInst(kind, debugLoc),
-        OwnershipForwardingMixin(kind, ownershipKind) {
+        OwnershipForwardingMixin(kind, ownershipKind, isDirectlyForwarding) {
     assert(classof(kind));
   }
 
@@ -8909,7 +8937,13 @@ class CheckedCastBranchInst final
                         ValueOwnershipKind forwardingOwnershipKind)
       : UnaryInstructionWithTypeDependentOperandsBase(
             DebugLoc, Operand, TypeDependentOperands, SuccessBB, FailureBB,
-            Target1Count, Target2Count, forwardingOwnershipKind),
+            Target1Count, Target2Count, forwardingOwnershipKind,
+            // We are always directly forwarding unless we are casting an
+            // AnyObject. This is b/c an AnyObject could contain a boxed
+            // AnyObject(Class()) that we unwrap as part of the cast. In such a
+            // case, we would return a different value and potentially end the
+            // lifetime of the operand value.
+            !Operand->getType().isAnyObject()),
         DestLoweredTy(DestLoweredTy), DestFormalTy(DestFormalTy),
         IsExact(IsExact) {}
 

lib/SIL/Utils/MemAccessUtils.cpp

@@ -877,9 +877,11 @@ SILValue swift::findOwnershipReferenceAggregate(SILValue ref) {
     if (auto *arg = dyn_cast<SILArgument>(root)) {
       if (auto *term = arg->getSingleTerminator()) {
         if (term->isTransformationTerminator()) {
-          assert(OwnershipForwardingTermInst::isa(term));
-          root = term->getOperand(0);
-          continue;
+          auto *ti = cast<OwnershipForwardingTermInst>(term);
+          if (ti->isDirectlyForwarding()) {
+            root = term->getOperand(0);
+            continue;
+          }
         }
       }
     }

lib/SIL/Utils/OwnershipUtils.cpp

@@ -29,121 +29,51 @@ bool swift::isValueAddressOrTrivial(SILValue v) {
          v.getOwnershipKind() == OwnershipKind::None;
 }
 
-// These operations forward both owned and guaranteed ownership.
-static bool isOwnershipForwardingInstructionKind(SILInstructionKind kind) {
-  switch (kind) {
-  case SILInstructionKind::TupleInst:
-  case SILInstructionKind::StructInst:
-  case SILInstructionKind::EnumInst:
-  case SILInstructionKind::DifferentiableFunctionInst:
-  case SILInstructionKind::LinearFunctionInst:
-  case SILInstructionKind::OpenExistentialRefInst:
-  case SILInstructionKind::UpcastInst:
-  case SILInstructionKind::UncheckedValueCastInst:
-  case SILInstructionKind::UncheckedRefCastInst:
-  case SILInstructionKind::ConvertFunctionInst:
-  case SILInstructionKind::RefToBridgeObjectInst:
-  case SILInstructionKind::BridgeObjectToRefInst:
-  case SILInstructionKind::UnconditionalCheckedCastInst:
-  case SILInstructionKind::UncheckedEnumDataInst:
-  case SILInstructionKind::SelectEnumInst:
-  case SILInstructionKind::SwitchEnumInst:
-  case SILInstructionKind::CheckedCastBranchInst:
-  case SILInstructionKind::DestructureStructInst:
-  case SILInstructionKind::DestructureTupleInst:
-  case SILInstructionKind::MarkDependenceInst:
-  case SILInstructionKind::InitExistentialRefInst:
-    return true;
-  default:
-    return false;
-  }
-}
-
-// These operations forward guaranteed ownership, but don't necessarily forward
-// owned values.
-static bool isGuaranteedForwardingInstructionKind(SILInstructionKind kind) {
-  switch (kind) {
-  case SILInstructionKind::TupleExtractInst:
-  case SILInstructionKind::StructExtractInst:
-  case SILInstructionKind::DifferentiableFunctionExtractInst:
-  case SILInstructionKind::LinearFunctionExtractInst:
-  case SILInstructionKind::OpenExistentialValueInst:
-  case SILInstructionKind::OpenExistentialBoxValueInst:
-    return true;
-  default:
-    return isOwnershipForwardingInstructionKind(kind);
-  }
-}
-
 bool swift::canOpcodeForwardGuaranteedValues(SILValue value) {
   // If we have an argument from a transforming terminator, we can forward
   // guaranteed.
   if (auto *arg = dyn_cast<SILArgument>(value))
     if (auto *ti = arg->getSingleTerminator())
-      if (ti->isTransformationTerminator()) {
-        assert(OwnershipForwardingMixin::isa(ti));
-        return true;
-      }
+      if (ti->isTransformationTerminator())
+        return OwnershipForwardingMixin::get(ti)->isDirectlyForwarding();
 
-  auto *inst = value->getDefiningInstruction();
-  if (!inst)
-    return false;
+  if (auto *inst = value->getDefiningInstruction())
+    if (auto *mixin = OwnershipForwardingMixin::get(inst))
+      return mixin->isDirectlyForwarding() &&
+             !isa<OwnedFirstArgForwardingSingleValueInst>(inst);
 
-  bool result = isGuaranteedForwardingInstructionKind(inst->getKind());
-  if (result) {
-    assert(!isa<OwnedFirstArgForwardingSingleValueInst>(inst));
-    assert(OwnershipForwardingMixin::isa(inst));
-  }
-  return result;
+  return false;
 }
 
 bool swift::canOpcodeForwardGuaranteedValues(Operand *use) {
-  auto *user = use->getUser();
-  bool result = isOwnershipForwardingInstructionKind(user->getKind());
-  if (result) {
-    assert(!isa<GuaranteedFirstArgForwardingSingleValueInst>(user));
-    assert(OwnershipForwardingMixin::isa(user));
-  }
-  return result;
-}
-
-static bool isOwnedForwardingValueKind(SILInstructionKind kind) {
-  switch (kind) {
-  case SILInstructionKind::MarkUninitializedInst:
-    return true;
-  default:
-    return isOwnershipForwardingInstructionKind(kind);
-  }
+  if (auto *mixin = OwnershipForwardingMixin::get(use->getUser()))
+    return mixin->isDirectlyForwarding() &&
+           !isa<OwnedFirstArgForwardingSingleValueInst>(use->getUser());
+  return false;
 }
 
 bool swift::canOpcodeForwardOwnedValues(SILValue value) {
   // If we have a SILArgument and we are the successor block of a transforming
   // terminator, we are fine.
   if (auto *arg = dyn_cast<SILPhiArgument>(value))
     if (auto *predTerm = arg->getSingleTerminator())
-      if (predTerm->isTransformationTerminator()) {
-        assert(OwnershipForwardingMixin::isa(predTerm));
-        return true;
-      }
-  auto *inst = value->getDefiningInstruction();
-  if (!inst)
-    return false;
+      if (predTerm->isTransformationTerminator())
+        return OwnershipForwardingMixin::get(predTerm)->isDirectlyForwarding();
 
-  bool result = isOwnedForwardingValueKind(inst->getKind());
-  if (result) {
-    assert(!isa<GuaranteedFirstArgForwardingSingleValueInst>(inst));
-    assert(OwnershipForwardingMixin::isa(inst));
-  }
-  return result;
+  if (auto *inst = value->getDefiningInstruction())
+    if (auto *mixin = OwnershipForwardingMixin::get(inst))
+      return mixin->isDirectlyForwarding() &&
+             !isa<GuaranteedFirstArgForwardingSingleValueInst>(inst);
+
+  return false;
 }
 
 bool swift::canOpcodeForwardOwnedValues(Operand *use) {
   auto *user = use->getUser();
-  bool result = isOwnershipForwardingInstructionKind(user->getKind());
-  if (result) {
-    assert(OwnershipForwardingMixin::isa(user));
-  }
-  return result;
+  if (auto *mixin = OwnershipForwardingMixin::get(user))
+    return mixin->isDirectlyForwarding() &&
+           !isa<GuaranteedFirstArgForwardingSingleValueInst>(user);
+  return false;
 }
 
 //===----------------------------------------------------------------------===//
@@ -980,7 +910,8 @@ bool swift::getAllBorrowIntroducingValues(SILValue inputValue,
       // predecessor terminator.
       auto *arg = cast<SILPhiArgument>(value);
       auto *termInst = arg->getSingleTerminator();
-      assert(termInst && termInst->isTransformationTerminator());
+      assert(termInst && termInst->isTransformationTerminator() &&
+             OwnershipForwardingMixin::get(termInst)->isDirectlyForwarding());
       assert(termInst->getNumOperands() == 1 &&
              "Transforming terminators should always have a single operand");
       worklist.push_back(termInst->getAllOperands()[0].get());
@@ -1031,7 +962,8 @@ BorrowedValue swift::getSingleBorrowIntroducingValue(SILValue inputValue) {
       // predecessor terminator.
       auto *arg = cast<SILPhiArgument>(currentValue);
       auto *termInst = arg->getSingleTerminator();
-      assert(termInst && termInst->isTransformationTerminator());
+      assert(termInst && termInst->isTransformationTerminator() &&
+             OwnershipForwardingMixin::get(termInst)->isDirectlyForwarding());
       assert(termInst->getNumOperands() == 1 &&
              "Transformation terminators should only have single operands");
       currentValue = termInst->getAllOperands()[0].get();
@@ -1083,7 +1015,8 @@ bool swift::getAllOwnedValueIntroducers(
       // predecessor terminator.
       auto *arg = cast<SILPhiArgument>(value);
       auto *termInst = arg->getSingleTerminator();
-      assert(termInst && termInst->isTransformationTerminator());
+      assert(termInst && termInst->isTransformationTerminator() &&
+             OwnershipForwardingMixin::get(termInst)->isDirectlyForwarding());
       assert(termInst->getNumOperands() == 1 &&
              "Transforming terminators should always have a single operand");
       worklist.push_back(termInst->getAllOperands()[0].get());
@@ -1127,10 +1060,11 @@ OwnedValueIntroducer swift::getSingleOwnedValueIntroducer(SILValue inputValue) {
       }
 
       // Otherwise, we should have a block argument that is defined by a single
-      // predecessor terminator.
+      // predecessor terminator and is directly forwarding.
       auto *arg = cast<SILPhiArgument>(currentValue);
       auto *termInst = arg->getSingleTerminator();
-      assert(termInst && termInst->isTransformationTerminator());
+      assert(termInst && termInst->isTransformationTerminator() &&
+             OwnershipForwardingMixin::get(termInst)->isDirectlyForwarding());
       assert(termInst->getNumOperands()
              - termInst->getNumTypeDependentOperands() == 1 &&
              "Transformation terminators should only have single operands");

lib/SIL/Verifier/SILVerifier.cpp

@@ -4048,6 +4048,24 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
                   CBI->getOperand().getOwnershipKind()),
               "failure dest block argument must have ownership compatible with "
               "the checked_cast_br operand");
+
+      // Do not allow for checked_cast_br to forward guaranteed ownership if the
+      // source type is an AnyObject.
+      //
+      // EXPLANATION: A checked_cast_br from an AnyObject may return a different
+      // object. This occurs for instance if one has an AnyObject that is a
+      // boxed AnyHashable (ClassType). This breaks with the guarantees of
+      // checked_cast_br guaranteed, so we ban it.
+      require(!CBI->getOperand()->getType().isAnyObject() ||
+                  CBI->getOperand().getOwnershipKind() !=
+                      OwnershipKind::Guaranteed,
+              "checked_cast_br with an AnyObject typed source cannot forward "
+              "guaranteed ownership");
+      require(CBI->isDirectlyForwarding() ||
+                  CBI->getOperand().getOwnershipKind() !=
+                      OwnershipKind::Guaranteed,
+              "If checked_cast_br is not directly forwarding, it can not have "
+              "guaranteed ownership");
     } else {
       require(CBI->getFailureBB()->args_empty(),
               "Failure dest of checked_cast_br must not take any argument in "

lib/SILGen/SILGenBuilder.cpp

@@ -536,6 +536,12 @@ void SILGenBuilder::createCheckedCastBranch(SILLocation loc, bool isExact,
                                             SILBasicBlock *falseBlock,
                                             ProfileCounter Target1Count,
                                             ProfileCounter Target2Count) {
+  // Check if our source type is AnyObject. In such a case, we need to ensure
+  // plus one our operand since SIL does not support guaranteed casts from an
+  // AnyObject.
+  if (op.getType().isAnyObject()) {
+    op = op.ensurePlusOne(SGF, loc);
+  }
   createCheckedCastBranch(loc, isExact, op.forward(SGF),
                           destLoweredTy, destFormalTy,
                           trueBlock, falseBlock,

lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp

@@ -14,6 +14,7 @@
 #include "OwnershipPhiOperand.h"
 
 #include "swift/SIL/BasicBlockUtils.h"
+#include "swift/SIL/OwnershipUtils.h"
 
 using namespace swift;
 using namespace swift::semanticarc;
@@ -83,6 +84,20 @@ OwnershipLiveRange::OwnershipLiveRange(SILValue value)
       continue;
     }
 
+    // If we have a subclass of OwnershipForwardingMixin that doesnt directly
+    // forward its operand to the result, treat the use as an unknown consuming
+    // use.
+    //
+    // If we do not directly forward and we have an owned value (which we do
+    // here), we could get back a different value. Thus we can not transform
+    // such a thing from owned to guaranteed.
+    if (auto *i = OwnershipForwardingMixin::get(op->getUser())) {
+      if (!i->isDirectlyForwarding()) {
+        tmpUnknownConsumingUses.push_back(op);
+        continue;
+      }
+    }
+
     // Ok, this is a forwarding instruction whose ownership we can flip from
     // owned -> guaranteed.
     tmpForwardingConsumingUses.push_back(op);

test/SILGen/checked_cast_br_anyobject.swift

@@ -0,0 +1,44 @@
+// RUN: %target-swift-emit-silgen -module-name checked_cast_br_anyobject -parse-as-library -Xllvm -sil-full-demangle -enforce-exclusivity=checked %s
+
+public struct BridgedSwiftObject {
+    var swiftMetatype : UnsafeRawPointer
+    var refCounts : Int64
+}
+
+public typealias SwiftObject = UnsafeMutablePointer<BridgedSwiftObject>
+
+extension UnsafeMutablePointer where Pointee == BridgedSwiftObject {
+  init<T: AnyObject>(_ object: T) {
+    let ptr = Unmanaged.passUnretained(object).toOpaque()
+    self = ptr.bindMemory(to: BridgedSwiftObject.self, capacity: 1)
+  }
+  
+  func getAs<T: AnyObject>(_ objectType: T.Type) -> T {
+    return Unmanaged<T>.fromOpaque(self).takeUnretainedValue()
+  }
+}
+
+extension Optional where Wrapped == UnsafeMutablePointer<BridgedSwiftObject> {
+  func getAs<T: AnyObject>(_ objectType: T.Type) -> T? {
+    if let pointer = self {
+      return Unmanaged<T>.fromOpaque(pointer).takeUnretainedValue()
+    }
+    return nil
+  }
+}
+
+public class Klass {}
+public class Klass2 {}
+
+// Make sure that we do not crash when emitting this code!
+public func getValue(_ obj: UnsafeMutablePointer<BridgedSwiftObject>) -> AnyObject {
+    let v = obj.getAs(AnyObject.self)
+    switch v {
+    case let k as Klass:
+        return k
+    case let k as Klass2:
+        return k
+    default:
+        fatalError("unknown type")
+    }
+}