Skip to content

Commit c98ce0c

Browse files
committed
IRGen: Fix Dispatch overlay for non-optimized builds
I apologize in advance to @jrose-apple, who is not a fan of this fix ;-) In unoptimized builds, the convenience initializers on DispatchQueue allocate and immediately deallocate an instance of OS_dispatch_queue prior to calling the C function that returns the "real" instance. This is because we don't have a way to write user-defined factory initializers yet; convenience initializers still have an 'initializing' entry point that takes an existing instance, which we have no choice but to throw away. Unfortunately, when we perform the fake allocation, we look up class metadata by calling the wrong Swift runtime function, causing a crash when we send +allocWithZone:. Fix this so that the metadata is accessed via a lookup from the Objective-C runtime, instead of making a totally fake 'foreign metadata' object -- it looks like there was code for this already, it just wasn't used in all cases. While getting metadata for a runtime-only class should be rare, this feels like a real bug fix, to me. Second, we would ultimately free the fake object by sending -release, however OS_dispatch_queue has an override of -dealloc which doesn't like to be called with a completely uninitialized instance. Here, I'm going to drop all pretense of sanity. The patch just changes IRGen to lower the dealloc_partial_ref instruction as a call to the object_dispose() Objective-C runtime function when the class in question is a runtime-only class. This frees the object without running -dealloc, which *happens* to work for OS_dispatch_queue. Fixes <rdar://problem/27226313>.
1 parent 960b8e9 commit c98ce0c

File tree

7 files changed

+39
-8
lines changed

7 files changed

+39
-8
lines changed

include/swift/Runtime/RuntimeFunctions.def

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,12 @@ FUNCTION(GetObjectClass, object_getClass, C_CC,
841841
ARGS(ObjCPtrTy),
842842
ATTRS(NoUnwind, ReadOnly))
843843

844+
// id object_dispose(id object);
845+
FUNCTION(ObjectDispose, object_dispose, C_CC,
846+
RETURNS(ObjCPtrTy),
847+
ARGS(ObjCPtrTy),
848+
ATTRS(NoUnwind))
849+
844850
// Class objc_lookUpClass(const char *name);
845851
FUNCTION(LookUpClass, objc_lookUpClass, C_CC,
846852
RETURNS(ObjCClassPtrTy),

lib/IRGen/GenClass.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,22 @@ void irgen::emitPartialClassDeallocation(IRGenFunction &IGF,
726726
llvm::Value *metadataValue) {
727727
auto *theClass = selfType.getClassOrBoundGenericClass();
728728

729+
// Foreign classes should not be freed by sending -release.
730+
// They should also probably not be freed with object_dispose(),
731+
// either.
732+
//
733+
// However, in practice, the only time we should try to free an
734+
// instance of a foreign class here is inside an initializer
735+
// delegating to a factory initializer. In this case, the object
736+
// was allocated with +allocWithZone:, so calling object_dispose()
737+
// should be OK.
738+
if (theClass->getForeignClassKind() == ClassDecl::ForeignKind::RuntimeOnly) {
739+
selfValue = IGF.Builder.CreateBitCast(selfValue, IGF.IGM.ObjCPtrTy);
740+
IGF.Builder.CreateCall(IGF.IGM.getObjectDisposeFn(),
741+
{selfValue});
742+
return;
743+
}
744+
729745
llvm::Value *size, *alignMask;
730746
getInstanceSizeAndAlignMask(IGF, selfType, theClass, selfValue,
731747
size, alignMask);

lib/IRGen/GenMeta.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1340,7 +1340,7 @@ static llvm::Value *emitTypeMetadataAccessFunctionBody(IRGenFunction &IGF,
13401340
// Non-native types are just wrapped in various ways.
13411341
if (auto classDecl = dyn_cast<ClassDecl>(typeDecl)) {
13421342
// We emit a completely different pattern for foreign classes.
1343-
if (classDecl->isForeign()) {
1343+
if (classDecl->getForeignClassKind() == ClassDecl::ForeignKind::CFType) {
13441344
return emitForeignTypeMetadataRef(IGF, type);
13451345
}
13461346

test/1_stdlib/Dispatch.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@
33

44
// REQUIRES: objc_interop
55

6-
// rdar://27226313
7-
// REQUIRES: optimized_stdlib
8-
96
import Dispatch
107
import Foundation
118
import StdlibUnittest

test/IRGen/objc_protocol_conversion.sil

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ entry:
2929
// CHECK-LABEL: define{{( protected)?}} %swift.type* @protocol_metatype()
3030
sil @protocol_metatype : $@convention(thin) () -> @thick Protocol.Type {
3131
entry:
32-
// CHECK: call %swift.type* @swift_getForeignTypeMetadata
32+
// CHECK: call %objc_class* @objc_lookUpClass
33+
// CHECK: call %swift.type* @swift_getObjCClassMetadata
3334
%t = metatype $@thick Protocol.Type
3435
return %t : $@thick Protocol.Type
3536
}

test/IRGen/objc_runtime_visible.sil

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,17 @@ bb0:
2020
// CHECK-NEXT: ret %objc_class*
2121
return %0 : $@objc_metatype A.Type
2222
}
23+
24+
// CHECK: define void @deallocA(%CSo1A*) #0 {
25+
sil @deallocA : $@convention(thin) (@owned A) -> () {
26+
bb0(%0 : $A):
27+
%1 = metatype $@thick A.Type
28+
29+
// CHECK: call %objc_object* @object_dispose
30+
dealloc_partial_ref %0 : $A, %1 : $@thick A.Type
31+
32+
%2 = tuple ()
33+
34+
// CHECK-NEXT: ret
35+
return %2 : $()
36+
}

test/Runtime/weak-reference-racetests-dispatch.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
// RUN: %target-run-simple-swift
22
// REQUIRES: executable_test
33

4-
// Disabled because of a crash with a debug stdlib - rdar://problem/27226313
5-
// REQUIRES: optimized_stdlib
6-
74
// REQUIRES: objc_interop
85

96
import StdlibUnittest

0 commit comments

Comments
 (0)