Skip to content

[concurrency] Ban associated objects from being set on instances of actor classes. #34136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion include/swift/ABI/Class.h
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
5 changes: 5 additions & 0 deletions include/swift/AST/SemanticAttrs.def
Original file line number Diff line number Diff line change
Expand Up @@ -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

32 changes: 32 additions & 0 deletions lib/IRGen/GenClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
25 changes: 25 additions & 0 deletions test/IRGen/actor_class_forbid_objc_assoc_objects.swift
Original file line number Diff line number Diff line change
@@ -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 }
}
79 changes: 79 additions & 0 deletions test/IRGen/class_forbid_objc_assoc_objects.swift
Original file line number Diff line number Diff line change
@@ -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> {
}
190 changes: 190 additions & 0 deletions test/Interpreter/actor_class_forbid_objc_assoc_objects.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
Loading