Skip to content

Commit 5184e21

Browse files
tbkkatbkka
authored
Merge pull request swiftlang#69926 from jckarter/resilient-throwing-miscompiles-5.10
[5.10] Fix miscompiles in client code using resilient noncopyable types.
2 parents 1dc51e7 + f02eb83 commit 5184e21

File tree

6 files changed

+229
-4
lines changed

6 files changed

+229
-4
lines changed

‎lib/SILGen/SILGenFunction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ void SILGenFunction::emitCaptures(SILLocation loc,
767767
val = B.createMarkUnresolvedNonCopyableValueInst(
768768
loc, val,
769769
MarkUnresolvedNonCopyableValueInst::CheckKind::
770-
AssignableButNotConsumable);
770+
NoConsumeOrAssign);
771771
}
772772
val = emitLoad(loc, val, tl, SGFContext(), IsNotTake).forward(*this);
773773
}

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2985,8 +2985,14 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
29852985
InitableButNotConsumable)
29862986
continue;
29872987

2988-
auto *insertPt = inst->getNextInstruction();
2989-
assert(insertPt && "def instruction was a terminator");
2988+
SILInstruction *insertPt;
2989+
if (auto tryApply = dyn_cast<TryApplyInst>(inst)) {
2990+
// The dead def is only valid on the success return path.
2991+
insertPt = &tryApply->getNormalBB()->front();
2992+
} else {
2993+
insertPt = inst->getNextInstruction();
2994+
assert(insertPt && "instruction is a terminator that wasn't handled?");
2995+
}
29902996
insertDestroyBeforeInstruction(addressUseState, insertPt,
29912997
liveness.getRootValue(), defPair.second,
29922998
consumes);

‎test/Interpreter/Inputs/moveonly_resilient_type.swift

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
struct MyError: Error {}
2+
3+
public struct Resilient: ~Copyable {
4+
static var nextValue: Int = 0
5+
6+
private(set) public var value: Int
7+
public init(nonthrowing: ()) {
8+
value = Self.nextValue
9+
Self.nextValue += 1
10+
}
11+
deinit { print("resilient deinit \(value)") }
12+
13+
public init(throwing: Bool) throws {
14+
if throwing {
15+
throw MyError()
16+
}
17+
self = .init(nonthrowing: ())
18+
}
19+
public init(throwingAfterInit: Bool) throws {
20+
self = .init(nonthrowing: ())
21+
if throwingAfterInit {
22+
throw MyError()
23+
}
24+
}
25+
26+
public static func instanceCount() -> Int {
27+
return nextValue
28+
}
29+
}
30+
31+
func testCapture(_: () -> Bool) {}
32+
33+
public struct ResilientCapturesInDeinit: ~Copyable {
34+
static var nextValue: Int = 0
35+
36+
private(set) public var value: Int
37+
public init(nonthrowing: ()) {
38+
value = Self.nextValue
39+
Self.nextValue += 1
40+
}
41+
deinit {
42+
testCapture { value >= 0 }
43+
print("resilient capture in deinit \(value)")
44+
}
45+
46+
public init(throwing: Bool) throws {
47+
if throwing {
48+
throw MyError()
49+
}
50+
self = .init(nonthrowing: ())
51+
}
52+
public init(throwingAfterInit: Bool) throws {
53+
self = .init(nonthrowing: ())
54+
if throwingAfterInit {
55+
throw MyError()
56+
}
57+
}
58+
59+
public static func instanceCount() -> Int {
60+
return nextValue
61+
}
62+
}

‎test/Interpreter/moveonly_resilient_capture_during_deinit.swift

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift -emit-module-path %t -enable-library-evolution -module-name moveonly_resilient_type -parse-as-library %S/Inputs/moveonly_resilient_type.swift -c -o %t/moveonly_resilient_type.o
3+
// RUN: %target-build-swift -enable-library-evolution -module-name moveonly_resilient_type -parse-as-library %S/Inputs/moveonly_resilient_type.swift -c -o %t/moveonly_resilient_type.o
4+
// RUN: %target-build-swift -o %t/a.out -I %t %s %t/moveonly_resilient_type.o
5+
// RUN: %target-run %t/a.out | %FileCheck %s
6+
7+
// REQUIRES: executable_test
8+
9+
import moveonly_resilient_type
10+
11+
// CHECK: start
12+
13+
func test1a() throws {
14+
// CHECK-NEXT: resilient capture in deinit 0
15+
_ = ResilientCapturesInDeinit(nonthrowing: ())
16+
}
17+
func test1b() throws {
18+
// CHECK-NEXT: resilient capture in deinit 1
19+
let x = ResilientCapturesInDeinit(nonthrowing: ())
20+
}
21+
func test2a() throws {
22+
// CHECK-NEXT: resilient capture in deinit 2
23+
_ = try ResilientCapturesInDeinit(throwing: false)
24+
}
25+
func test2b() throws {
26+
// CHECK-NEXT: resilient capture in deinit 3
27+
let x = try ResilientCapturesInDeinit(throwing: false)
28+
}
29+
func test3a() throws {
30+
_ = try ResilientCapturesInDeinit(throwing: true)
31+
}
32+
func test3b() throws {
33+
let x = try ResilientCapturesInDeinit(throwing: true)
34+
}
35+
func test4a() throws {
36+
// CHECK-NEXT: resilient capture in deinit 4
37+
_ = try ResilientCapturesInDeinit(throwingAfterInit: false)
38+
}
39+
func test4b() throws {
40+
// CHECK-NEXT: resilient capture in deinit 5
41+
let x = try ResilientCapturesInDeinit(throwingAfterInit: false)
42+
}
43+
func test5a() throws {
44+
// CHECK-NEXT: resilient capture in deinit 6
45+
_ = try ResilientCapturesInDeinit(throwingAfterInit: true)
46+
}
47+
func test5b() throws {
48+
// CHECK-NEXT: resilient capture in deinit 7
49+
let x = try ResilientCapturesInDeinit(throwingAfterInit: true)
50+
}
51+
52+
func main() {
53+
print("start")
54+
55+
_ = try? test1a()
56+
_ = try? test1b()
57+
_ = try? test2a()
58+
_ = try? test2b()
59+
_ = try? test3a()
60+
_ = try? test3b()
61+
_ = try? test4a()
62+
_ = try? test4b()
63+
_ = try? test5a()
64+
_ = try? test5b()
65+
66+
// CHECK-NEXT: total 8
67+
print("total \(ResilientCapturesInDeinit.instanceCount())")
68+
}
69+
main()

‎test/Interpreter/moveonly_resilient_deinit_on_throw.swift

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift -emit-module-path %t -enable-library-evolution -module-name moveonly_resilient_type -parse-as-library %S/Inputs/moveonly_resilient_type.swift -c -o %t/moveonly_resilient_type.o
3+
// RUN: %target-build-swift -enable-library-evolution -module-name moveonly_resilient_type -parse-as-library %S/Inputs/moveonly_resilient_type.swift -c -o %t/moveonly_resilient_type.o
4+
// RUN: %target-build-swift -o %t/a.out -I %t %s %t/moveonly_resilient_type.o
5+
// RUN: %target-run %t/a.out | %FileCheck %s
6+
7+
// REQUIRES: executable_test
8+
9+
// CHECK: starting
10+
11+
import moveonly_resilient_type
12+
13+
func makeItem1() throws -> Resilient {
14+
return Resilient(nonthrowing: ())
15+
}
16+
17+
func test1a() throws {
18+
// CHECK-NEXT: resilient deinit 0
19+
_ = try makeItem1()
20+
}
21+
func test1b() throws {
22+
// CHECK-NEXT: resilient deinit 1
23+
let x = try makeItem1()
24+
}
25+
26+
func makeItem2(throwing: Bool) throws -> Resilient {
27+
return try Resilient(throwing: throwing)
28+
}
29+
30+
func test2aa() throws {
31+
// CHECK-NEXT: resilient deinit 2
32+
_ = try makeItem2(throwing: false)
33+
}
34+
35+
func test2ab() throws {
36+
_ = try makeItem2(throwing: true)
37+
}
38+
39+
func test2ba() throws {
40+
// CHECK-NEXT: resilient deinit 3
41+
let x = try makeItem2(throwing: false)
42+
}
43+
44+
func test2bb() throws {
45+
let x = try makeItem2(throwing: true)
46+
}
47+
48+
func makeItem3(throwing: Bool) throws -> Resilient {
49+
return try Resilient(throwingAfterInit: throwing)
50+
}
51+
52+
func test3aa() throws {
53+
// CHECK-NEXT: resilient deinit 4
54+
_ = try makeItem3(throwing: false)
55+
}
56+
57+
func test3ab() throws {
58+
// CHECK-NEXT: resilient deinit 5
59+
_ = try makeItem3(throwing: true)
60+
}
61+
62+
func test3ba() throws {
63+
// CHECK-NEXT: resilient deinit 6
64+
let x = try makeItem3(throwing: false)
65+
}
66+
67+
func test3bb() throws {
68+
// CHECK-NEXT: resilient deinit 7
69+
let x = try makeItem3(throwing: true)
70+
}
71+
72+
func main() {
73+
print("starting")
74+
_ = try? test1a()
75+
_ = try? test1b()
76+
_ = try? test2aa()
77+
_ = try? test2ab()
78+
_ = try? test2ba()
79+
_ = try? test2bb()
80+
_ = try? test3aa()
81+
_ = try? test3ab()
82+
_ = try? test3ba()
83+
_ = try? test3bb()
84+
85+
// CHECK-NEXT: 8 instances in total
86+
print("\(Resilient.instanceCount()) instances in total")
87+
}
88+
main()

‎test/SILGen/moveonly.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ func testGlobalAssign() {
913913
// CHECK: store [[ARG]] to [init] [[PROJECT]]
914914
//
915915
// CHECK: [[FN:%.*]] = function_ref @$s8moveonly49checkMarkUnresolvedNonCopyableValueInstOnCaptured1xyAA2FDVn_tFyyXEfU_ : $@convention(thin) @substituted <τ_0_0> (@guaranteed FD) -> @out τ_0_0 for <()>
916-
// CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[PROJECT]]
916+
// CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[PROJECT]]
917917
// CHECK: [[VALUE:%.*]] = load [copy] [[MARK]]
918918
// CHECK: [[CLOSURE:%.*]] = partial_apply [callee_guaranteed] [[FN]]([[VALUE]])
919919
// CHECK: } // end sil function '$s8moveonly49checkMarkUnresolvedNonCopyableValueInstOnCaptured1xyAA2FDVn_tF'

0 commit comments

Comments
 (0)