[concurrency] Ban associated objects from being set on instances of actor classes that do not inherit from NSObject.

gottesmm · gottesmm · commit 91209d311d21 · 2020-11-27T14:48:51.000-08:00
Associated objects are actively dangerous there because they’re non-isolated
actor state, and it’s “new” code wher no backward compatibility concerns that
make it more difficult to ban this on other forms of classes.

rdar://69769048
diff --git a/include/swift/ABI/Class.h b/include/swift/ABI/Class.h
@@ -53,7 +53,12 @@ enum class ObjCClassFlags : uint32_t {
   /// This class provides a non-trivial .cxx_destruct method, but
   /// its .cxx_construct is trivial.  For backwards compatibility,
   /// when setting this flag, HasCXXStructors must be set as well.
-  HasCXXDestructorOnly = 0x00100
+  HasCXXDestructorOnly = 0x00100,
+
+  /// This class does not allow associated objects on instances.
+  ///
+  /// Will cause the objc runtime to trap in objc_setAssociatedObject.
+  ForbidsAssociatedObjects = 0x00400,
 };
 inline ObjCClassFlags &operator|=(ObjCClassFlags &lhs, ObjCClassFlags rhs) {
   lhs = ObjCClassFlags(uint32_t(lhs) | uint32_t(rhs));
diff --git a/include/swift/AST/SemanticAttrs.def b/include/swift/AST/SemanticAttrs.def
@@ -106,5 +106,10 @@ SEMANTICS_ATTR(KEYPATH_KVC_KEY_PATH_STRING, "keypath.kvcKeyPathString")
 /// consider inlining where to put these.
 SEMANTICS_ATTR(FORCE_EMIT_OPT_REMARK_PREFIX, "optremark")
 
+/// An attribute that when attached to a class causes instances of the class to
+/// be forbidden from having associated objects set upon them. This is only used
+/// for testing purposes.
+SEMANTICS_ATTR(OBJC_FORBID_ASSOCIATED_OBJECTS, "objc.forbidAssociatedObjects")
+
 #undef SEMANTICS_ATTR
 
diff --git a/lib/IRGen/GenClass.cpp b/lib/IRGen/GenClass.cpp
@@ -25,6 +25,7 @@
 #include "swift/AST/Module.h"
 #include "swift/AST/Pattern.h"
 #include "swift/AST/PrettyStackTrace.h"
+#include "swift/AST/SemanticAttrs.h"
 #include "swift/AST/TypeMemberVisitor.h"
 #include "swift/AST/Types.h"
 #include "swift/ClangImporter/ClangModule.h"
@@ -1443,6 +1444,32 @@ namespace {
     }
 
   private:
+    /// If we should set the forbids associated objects on instances metadata
+    /// flag.
+    ///
+    /// We currently do this on:
+    ///
+    /// * Actor classes.
+    /// * classes marked with @_semantics("objc.forbidAssociatedObjects")
+    ///   (for testing purposes)
+    ///
+    /// TODO: Expand this as appropriate over time.
+    bool doesClassForbidAssociatedObjectsOnInstances() const {
+      auto *clsDecl = getClass();
+
+      // We ban this on actors without objc ancestry.
+      if (clsDecl->isActor() && !clsDecl->checkAncestry(AncestryFlags::ObjC))
+        return true;
+
+      // Otherwise, we only do it if our special semantics attribute is on the
+      // relevant class. This is for testing purposes.
+      if (clsDecl->hasSemanticsAttr(semantics::OBJC_FORBID_ASSOCIATED_OBJECTS))
+        return true;
+
+      // TODO: Add new cases here as appropriate over time.
+      return false;
+    }
+
     ObjCClassFlags buildFlags(ForMetaClass_t forMeta,
                               HasUpdateCallback_t hasUpdater) {
       ObjCClassFlags flags = ObjCClassFlags::CompiledByARC;
@@ -1463,6 +1490,11 @@ namespace {
       if (hasUpdater)
         flags |= ObjCClassFlags::HasMetadataUpdateCallback;
 
+      // If we know that our class does not support having associated objects
+      // placed upon instances, set the forbid associated object flag.
+      if (doesClassForbidAssociatedObjectsOnInstances())
+        flags |= ObjCClassFlags::ForbidsAssociatedObjects;
+
       // FIXME: set ObjCClassFlags::Hidden when appropriate
       return flags;
     }
diff --git a/test/IRGen/actor_class_forbid_objc_assoc_objects.swift b/test/IRGen/actor_class_forbid_objc_assoc_objects.swift
@@ -0,0 +1,25 @@
+// RUN: %target-swift-frontend -enable-experimental-concurrency -emit-ir %s | %FileCheck %s
+
+// REQUIRES: concurrency
+// REQUIRES: objc_interop
+
+import _Concurrency
+
+// CHECK: @_METACLASS_DATA__TtC37actor_class_forbid_objc_assoc_objects5Actor = internal constant { {{.*}} } { i32 [[METAFLAGS:1153]],
+// CHECK: @_DATA__TtC37actor_class_forbid_objc_assoc_objects5Actor = internal constant { {{.*}} } { i32 [[OBJECTFLAGS:1152|1216]],
+final actor class Actor {
+}
+
+// CHECK: @_METACLASS_DATA__TtC37actor_class_forbid_objc_assoc_objects6Actor2 = internal constant { {{.*}} } { i32 [[METAFLAGS]],
+// CHECK: @_DATA__TtC37actor_class_forbid_objc_assoc_objects6Actor2 = internal constant { {{.*}} } { i32 [[OBJECTFLAGS]],
+actor class Actor2 {
+}
+
+// CHECK: @_METACLASS_DATA__TtC37actor_class_forbid_objc_assoc_objects6Actor3 = internal constant { {{.*}} } { i32 [[METAFLAGS]],
+// CHECK: @_DATA__TtC37actor_class_forbid_objc_assoc_objects6Actor3 = internal constant { {{.*}} } { i32 [[OBJECTFLAGS]],
+class Actor3 : Actor2 {}
+
+actor class GenericActor<T> {
+    var state: T
+    init(state: T) { self.state = state }
+}
diff --git a/test/IRGen/class_forbid_objc_assoc_objects.swift b/test/IRGen/class_forbid_objc_assoc_objects.swift
@@ -0,0 +1,79 @@
+// RUN: %target-swift-frontend -emit-ir %s | %FileCheck %s
+
+// REQUIRES: objc_interop
+
+// CHECK: @_METACLASS_DATA__TtC31class_forbid_objc_assoc_objects24AllowedToHaveAssocObject = internal constant { {{.*}} } { i32 129,
+// CHECK: @_DATA__TtC31class_forbid_objc_assoc_objects24AllowedToHaveAssocObject = internal constant { {{.*}} } { i32 128,
+final class AllowedToHaveAssocObject {
+}
+
+// CHECK: @_METACLASS_DATA__TtC31class_forbid_objc_assoc_objects24UnableToHaveAssocObjects = internal constant { {{.*}} } { i32 1153,
+// CHECK: @_DATA__TtC31class_forbid_objc_assoc_objects24UnableToHaveAssocObjects = internal constant { {{.*}} } { i32 1152,
+@_semantics("objc.forbidAssociatedObjects")
+final class UnableToHaveAssocObjects {
+}
+
+// Class Metadata For Generic Metadata
+//
+// CHECK: [[CLASS_METADATA:@[0-9][0-9]*]] = internal constant <{ {{.*}} }> <{ {{.*}} { i32 1152,
+//
+// Generic Metadata Pattern
+//
+// CHECK: @"$s31class_forbid_objc_assoc_objects31UnableToHaveAssocObjectsGenericCMP" = internal constant {{.*}}[[CLASS_METADATA]]
+@_semantics("objc.forbidAssociatedObjects")
+final class UnableToHaveAssocObjectsGeneric<T> {
+    var state: T
+    init(state: T) { self.state = state }
+}
+
+// This should be normal.
+//
+// CHECK: @_METACLASS_DATA__TtC31class_forbid_objc_assoc_objects40UnsoundAbleToHaveAssocObjectsParentClass = internal constant { {{.*}} } { i32 129,
+// CHECK: @_DATA__TtC31class_forbid_objc_assoc_objects40UnsoundAbleToHaveAssocObjectsParentClass = internal constant { {{.*}} } { i32 128,
+class UnsoundAbleToHaveAssocObjectsParentClass {
+}
+
+// This should have assoc object constraints
+//
+// CHECK: @_METACLASS_DATA__TtC31class_forbid_objc_assoc_objects39UnsoundUnableToHaveAssocObjectsSubClass = internal constant { {{.*}} } { i32 1153,
+// CHECK: @_DATA__TtC31class_forbid_objc_assoc_objects39UnsoundUnableToHaveAssocObjectsSubClass = internal constant { {{.*}} } { i32 1152,
+@_semantics("objc.forbidAssociatedObjects")
+final class UnsoundUnableToHaveAssocObjectsSubClass : UnsoundAbleToHaveAssocObjectsParentClass {
+}
+
+// CHECK: @_DATA__TtC31class_forbid_objc_assoc_objects41UnsoundAbleToHaveAssocObjectsParentClass2 = internal constant { {{.*}} } { i32 1152,
+@_semantics("objc.forbidAssociatedObjects")
+class UnsoundAbleToHaveAssocObjectsParentClass2 {
+}
+
+// This has normal metadata. We must at runtime add the flags of the subclass to
+// the child.
+//
+// CHECK: @_DATA__TtC31class_forbid_objc_assoc_objects40UnsoundUnableToHaveAssocObjectsSubClass2 = internal constant { {{.*}} } { i32 128, 
+final class UnsoundUnableToHaveAssocObjectsSubClass2 : UnsoundAbleToHaveAssocObjectsParentClass2 {
+}
+
+// CHECK: @_DATA__TtC31class_forbid_objc_assoc_objects40UnsoundUnableToHaveAssocObjectsSubClass3 = internal constant { {{.*}} } { i32 128,
+class UnsoundUnableToHaveAssocObjectsSubClass3 : UnsoundAbleToHaveAssocObjectsParentClass2 {
+}
+
+class GenericAbleToHaveAssocObjectsParentClass<T> {
+  public var state: T
+  init(state: T) { self.state = state }
+}
+
+@_semantics("objc.forbidAssociatedObjects")
+final class GenericUnableToHaveAssocObjectsSubClass<T> : GenericAbleToHaveAssocObjectsParentClass<T> {
+}
+
+@_semantics("objc.forbidAssociatedObjects")
+class GenericAbleToHaveAssocObjectsParentClass2<T> {
+  public var state: T
+  init(state: T) { self.state = state }
+}
+
+final class GenericUnableToHaveAssocObjectsSubClass2<T> : GenericAbleToHaveAssocObjectsParentClass2<T> {
+}
+
+class GenericUnableToHaveAssocObjectsSubClass3<T> : GenericAbleToHaveAssocObjectsParentClass2<T> {
+}
diff --git a/test/Interpreter/actor_class_forbid_objc_assoc_objects.swift b/test/Interpreter/actor_class_forbid_objc_assoc_objects.swift
@@ -0,0 +1,190 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swiftc_driver -Xfrontend -enable-experimental-concurrency %s -o %t/out
+// RUN: %target-run %t/out
+
+// REQUIRES: concurrency
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+
+import ObjectiveC
+import _Concurrency
+import StdlibUnittest
+
+defer { runAllTests() }
+
+var Tests = TestSuite("Actor.AssocObject")
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+final actor class Actor {
+}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("final class crash when set assoc object")
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor()
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+actor class Actor2 {
+}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("non-final class crash when set assoc object")
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor2()
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+class Actor3 : Actor2 {}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("non-final subclass crash when set assoc object")
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor3()
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+final class Actor3Final : Actor2 {}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("final subclass crash when set assoc object")
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor3Final()
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+class Actor4<T> : Actor2 {
+  var state: T
+  init(state: T) { self.state = state }
+}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("generic subclass crash when set assoc object")
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor4(state: 5)
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+actor class Actor5<T> {
+  var state: T
+  init(state: T) { self.state = state }
+}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("base generic class crash when set assoc object")
+  .xfail(
+    .custom({ true }, reason: "We appear to be stomping on isa pointers during " +
+      "actor generic class isa initialization: rdar://70589739"))
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor5(state: 5)
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+
+  Tests.test("base generic class metatype crash when set assoc object")
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor5<Int>.self
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+class Actor6<T> : Actor5<T> {
+  override init(state: T) { super.init(state: state) }
+}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("sub-generic class base generic class crash when set assoc object")
+  .xfail(
+    .custom({ true }, reason: "We appear to be stomping on isa pointers during " +
+      "actor generic class isa initialization: rdar://70589739"))
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor6(state: 5)
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+final class Actor6Final<T> : Actor5<T> {
+  override init(state: T) { super.init(state: state) }
+}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("final sub-generic class base generic class crash when set assoc object")
+  .xfail(
+    .custom({ true }, reason: "We appear to be stomping on isa pointers during " +
+      "actor generic class isa initialization: rdar://70589739"))
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor6Final(state: 5)
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+
+  Tests.test("final sub-generic class base generic class crash when set assoc object2")
+  .xfail(
+    .custom({ true }, reason: "We appear to be stomping on isa pointers during " +
+      "actor generic class isa initialization: rdar://70589739"))
+  .code {
+    let x = Actor6Final(state: 5)
+    print(type(of: x))
+  }
+
+  Tests.test("final sub-generic class metatype, base generic class crash when set assoc object")
+  .crashOutputMatches("objc_setAssociatedObject called on instance")
+  .code {
+    expectCrashLater()
+    let x = Actor6Final<Int>.self
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+actor class ActorNSObjectSubKlass : NSObject {}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("no crash when inherit from nsobject")
+  .code {
+    let x = ActorNSObjectSubKlass()
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
+
+@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
+actor class ActorNSObjectSubKlassGeneric<T> : NSObject {
+  var state: T
+  init(state: T) { self.state = state }
+}
+
+if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
+  Tests.test("no crash when generic inherit from nsobject")
+  .code {
+    let x = ActorNSObjectSubKlassGeneric(state: 5)
+    objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
+  }
+}
diff --git a/test/Interpreter/actor_subclass_metatypes.swift b/test/Interpreter/actor_subclass_metatypes.swift
diff --git a/test/Interpreter/class_forbid_objc_assoc_objects.swift b/test/Interpreter/class_forbid_objc_assoc_objects.swift
