Skip to content

Commit bd6da5d

Browse files
committed
[cxx-interop][IRGen] Do not try to retain/release a null pointer
This teaches IRGen to only emit a lifetime operation (retain or release) for a C++ foreign reference type if the pointer is not `nullptr`. Previously the compiler would in some cases emit a release call for `nullptr`, which breaks the assumption that the argument to a custom release function is `_Nonnull`. For instance: ``` var globalOptional: MyRefType? = nil func foo() { globalOptional = MyRefType.create() } ``` When emitting IR for the assignment operation to `globalOptional`, the compiler would emit code to first retrieve the existing value of `globalOptional` and release it. If the value is `nil`, it does not need to be released. rdar://97532642
1 parent d7426e2 commit bd6da5d

File tree

6 files changed

+134
-23
lines changed

6 files changed

+134
-23
lines changed

lib/IRGen/ClassTypeInfo.h

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -99,38 +99,56 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
9999
HeapTypeInfo::emitScalarRetain(IGF, value, atomicity);
100100
}
101101

102+
void strongCustomRetain(IRGenFunction &IGF, Explosion &e,
103+
bool needsNullCheck) const {
104+
assert(getReferenceCounting() == ReferenceCounting::Custom &&
105+
"only supported for custom ref-counting");
106+
107+
llvm::Value *value = e.claimNext();
108+
auto retainFn =
109+
evaluateOrDefault(
110+
getClass()->getASTContext().evaluator,
111+
CustomRefCountingOperation(
112+
{getClass(), CustomRefCountingOperationKind::retain}),
113+
{})
114+
.operation;
115+
IGF.emitForeignReferenceTypeLifetimeOperation(retainFn, value,
116+
needsNullCheck);
117+
}
118+
102119
// Implement the primary retain/release operations of ReferenceTypeInfo
103120
// using basic reference counting.
104121
void strongRetain(IRGenFunction &IGF, Explosion &e,
105122
Atomicity atomicity) const override {
106123
if (getReferenceCounting() == ReferenceCounting::Custom) {
107-
llvm::Value *value = e.claimNext();
108-
auto retainFn =
109-
evaluateOrDefault(
110-
getClass()->getASTContext().evaluator,
111-
CustomRefCountingOperation(
112-
{getClass(), CustomRefCountingOperationKind::retain}),
113-
{})
114-
.operation;
115-
IGF.emitForeignReferenceTypeLifetimeOperation(retainFn, value);
124+
strongCustomRetain(IGF, e, /*needsNullCheck*/ false);
116125
return;
117126
}
118127

119128
HeapTypeInfo::strongRetain(IGF, e, atomicity);
120129
}
121130

131+
void strongCustomRelease(IRGenFunction &IGF, Explosion &e,
132+
bool needsNullCheck) const {
133+
assert(getReferenceCounting() == ReferenceCounting::Custom &&
134+
"only supported for custom ref-counting");
135+
136+
llvm::Value *value = e.claimNext();
137+
auto releaseFn =
138+
evaluateOrDefault(
139+
getClass()->getASTContext().evaluator,
140+
CustomRefCountingOperation(
141+
{getClass(), CustomRefCountingOperationKind::release}),
142+
{})
143+
.operation;
144+
IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value,
145+
needsNullCheck);
146+
}
147+
122148
void strongRelease(IRGenFunction &IGF, Explosion &e,
123149
Atomicity atomicity) const override {
124150
if (getReferenceCounting() == ReferenceCounting::Custom) {
125-
llvm::Value *value = e.claimNext();
126-
auto releaseFn =
127-
evaluateOrDefault(
128-
getClass()->getASTContext().evaluator,
129-
CustomRefCountingOperation(
130-
{getClass(), CustomRefCountingOperationKind::release}),
131-
{})
132-
.operation;
133-
IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
151+
strongCustomRelease(IGF, e, /*needsNullCheck*/ false);
134152
return;
135153
}
136154

lib/IRGen/GenEnum.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2668,7 +2668,8 @@ namespace {
26682668
if (Refcounting == ReferenceCounting::Custom) {
26692669
Explosion e;
26702670
e.add(ptr);
2671-
getPayloadTypeInfo().as<ClassTypeInfo>().strongRetain(IGF, e, IGF.getDefaultAtomicity());
2671+
getPayloadTypeInfo().as<ClassTypeInfo>().strongCustomRetain(
2672+
IGF, e, /*needsNullCheck*/ true);
26722673
return;
26732674
}
26742675

@@ -2704,7 +2705,8 @@ namespace {
27042705
if (Refcounting == ReferenceCounting::Custom) {
27052706
Explosion e;
27062707
e.add(ptr);
2707-
getPayloadTypeInfo().as<ClassTypeInfo>().strongRelease(IGF, e, IGF.getDefaultAtomicity());
2708+
getPayloadTypeInfo().as<ClassTypeInfo>().strongCustomRelease(
2709+
IGF, e, /*needsNullCheck*/ true);
27082710
return;
27092711
}
27102712

lib/IRGen/GenObjC.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,7 +1712,7 @@ void IRGenFunction::emitBlockRelease(llvm::Value *value) {
17121712
}
17131713

17141714
void IRGenFunction::emitForeignReferenceTypeLifetimeOperation(
1715-
ValueDecl *fn, llvm::Value *value) {
1715+
ValueDecl *fn, llvm::Value *value, bool needsNullCheck) {
17161716
assert(fn->getClangDecl() && isa<clang::FunctionDecl>(fn->getClangDecl()));
17171717

17181718
auto clangFn = cast<clang::FunctionDecl>(fn->getClangDecl());
@@ -1723,6 +1723,28 @@ void IRGenFunction::emitForeignReferenceTypeLifetimeOperation(
17231723
cast<llvm::FunctionType>(llvmFn->getFunctionType())->getParamType(0);
17241724
value = Builder.CreateBitCast(value, argType);
17251725

1726-
auto call = Builder.CreateCall(llvmFn->getFunctionType(), llvmFn, value);
1726+
llvm::CallInst *call = nullptr;
1727+
if (needsNullCheck) {
1728+
// Check if the pointer is null.
1729+
auto nullValue = llvm::Constant::getNullValue(argType);
1730+
auto hasValue = Builder.CreateICmpNE(value, nullValue);
1731+
1732+
auto nonNullValueBB = createBasicBlock("lifetime.nonnull-value");
1733+
auto contBB = createBasicBlock("lifetime.cont");
1734+
1735+
// If null, just continue.
1736+
Builder.CreateCondBr(hasValue, nonNullValueBB, contBB);
1737+
1738+
// If non-null, emit a call to release/retain function.
1739+
Builder.emitBlock(nonNullValueBB);
1740+
call = Builder.CreateCall(llvmFn->getFunctionType(), llvmFn, value);
1741+
1742+
Builder.CreateBr(contBB);
1743+
1744+
Builder.emitBlock(contBB);
1745+
} else {
1746+
call = Builder.CreateCall(llvmFn->getFunctionType(), llvmFn, value);
1747+
}
1748+
17271749
call->setDoesNotThrow();
17281750
}

lib/IRGen/IRGenFunction.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,8 @@ class IRGenFunction {
575575
void emitBlockRelease(llvm::Value *value);
576576

577577
void emitForeignReferenceTypeLifetimeOperation(ValueDecl *fn,
578-
llvm::Value *value);
578+
llvm::Value *value,
579+
bool needsNullCheck = false);
579580

580581
// Routines for an unknown reference-counting style (meaning,
581582
// dynamically something compatible with either the ObjC or Swift styles).

test/Interop/Cxx/foreign-reference/Inputs/reference-counted.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ __attribute__((swift_attr("release:GCRelease"))) GlobalCount {
4646
inline void GCRetain(GlobalCount *x) { globalCount++; }
4747
inline void GCRelease(GlobalCount *x) { globalCount--; }
4848

49+
struct __attribute__((swift_attr("import_as_ref")))
50+
__attribute__((swift_attr("retain:GCRetainNullableInit")))
51+
__attribute__((swift_attr("release:GCReleaseNullableInit")))
52+
GlobalCountNullableInit {
53+
static GlobalCountNullableInit *_Nullable create(bool wantNullptr) {
54+
if (wantNullptr)
55+
return nullptr;
56+
return new (malloc(sizeof(GlobalCountNullableInit)))
57+
GlobalCountNullableInit();
58+
}
59+
};
60+
61+
inline void GCRetainNullableInit(GlobalCountNullableInit *x) { globalCount++; }
62+
inline void GCReleaseNullableInit(GlobalCountNullableInit *x) { globalCount--; }
63+
4964
SWIFT_END_NULLABILITY_ANNOTATIONS
5065

5166
#endif // TEST_INTEROP_CXX_FOREIGN_REFERENCE_INPUTS_REFERENCE_COUNTED_H
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// RUN: %target-swift-emit-irgen %s -I %S/Inputs -cxx-interoperability-mode=default -Xcc -fignore-exceptions -disable-availability-checking | %FileCheck %s
2+
// XFAIL: OS=linux-android, OS=linux-androideabi
3+
4+
import ReferenceCounted
5+
6+
7+
public func getLocalCount() -> NS.LocalCount {
8+
let result = NS.LocalCount.create()
9+
return result
10+
}
11+
12+
// CHECK: define {{.*}}swiftcc ptr @"$s4main13getLocalCountSo2NSO0cD0VyF"()
13+
// CHECK-NEXT: entry:
14+
// CHECK: %0 = call ptr @{{_ZN2NS10LocalCount6createEv|"\?create\@LocalCount\@NS\@\@SAPEAU12\@XZ"}}()
15+
// CHECK-NEXT: call void @{{_Z8LCRetainPN2NS10LocalCountE|"\?LCRetain\@\@YAXPEAULocalCount\@NS\@\@\@Z"}}(ptr %0)
16+
// CHECK: ret ptr %0
17+
// CHECK-NEXT: }
18+
19+
20+
public func get42() -> Int32 {
21+
let result = NS.LocalCount.create()
22+
return result.returns42()
23+
}
24+
25+
// CHECK: define {{.*}}swiftcc i32 @"$s4main5get42s5Int32VyF"()
26+
// CHECK-NEXT: entry:
27+
// CHECK: %0 = call ptr @{{_ZN2NS10LocalCount6createEv|"\?create\@LocalCount\@NS\@\@SAPEAU12\@XZ"}}()
28+
// CHECK-NEXT: call void @{{_Z8LCRetainPN2NS10LocalCountE|"\?LCRetain\@\@YAXPEAULocalCount\@NS\@\@\@Z"}}(ptr %0)
29+
// CHECK: %1 = call i32 @{{_ZN2NS10LocalCount9returns42Ev|"\?returns42\@LocalCount\@NS\@\@QEAAHXZ"}}
30+
// CHECK: ret i32 %1
31+
// CHECK-NEXT: }
32+
33+
34+
public func getNullable(wantNullptr: Bool) -> GlobalCountNullableInit? {
35+
let result = GlobalCountNullableInit.create(wantNullptr)
36+
return result
37+
}
38+
39+
// CHECK: define {{.*}}swiftcc i64 @"$s4main11getNullable11wantNullptrSo011GlobalCountC4InitVSgSb_tF"(i1 %0)
40+
// CHECK-NEXT: entry:
41+
// CHECK: %1 = call ptr @{{_ZN23GlobalCountNullableInit6createEb|"\?create\@GlobalCountNullableInit\@\@SAPEAU1\@_N\@Z"}}
42+
// CHECK-NEXT: %2 = ptrtoint ptr %1 to i64
43+
// CHECK-NEXT: %3 = inttoptr i64 %2 to ptr
44+
// CHECK-NEXT: %4 = icmp ne ptr %3, null
45+
// CHECK-NEXT: br i1 %4, label %lifetime.nonnull-value, label %lifetime.cont
46+
47+
// CHECK: lifetime.nonnull-value:
48+
// CHECK-NEXT: call void @{{_Z20GCRetainNullableInitP23GlobalCountNullableInit|"\?GCRetainNullableInit\@\@YAXPEAUGlobalCountNullableInit\@\@\@Z"}}(ptr %3)
49+
// CHECK-NEXT: br label %lifetime.cont
50+
51+
// CHECK: lifetime.cont:
52+
// CHECK: ret i64 %2
53+
// CHECK-NEXT: }

0 commit comments

Comments
 (0)