Skip to content

Commit 50128e1

Browse files
gottesmmgottesmm
committed
[move-only] Do not attempt to lazily deserialize the moveonly deinit table in IRGen.
SIL Functions are serialized in canonical SIL before they have their final ABI adjusted for large function arguments. Large function argument ABI is adjusted to be indirect as part of the transition from canonical SIL to lowered SIL. This means that if we deserialize a function from another module in canonical SIL and attempt to call it in IRGen we will call it with the wrong ABI implying if we reference any fields of the type in the deinit we will most likely crash (among other potential issues). This patch fixes the issue by changing IRGen to not lazily deserialize the moveonly deinit table and its associated functions. Instead if we do not have our table already deserialized, we just call the function's deinit via the destroy value deinit table. rdar://110496872 (cherry picked from commit b2a52ff)
1 parent 66a38a1 commit 50128e1

5 files changed

+124
-36
lines changed

‎lib/IRGen/GenType.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,9 +2895,11 @@ static bool tryEmitDeinitCall(IRGenFunction &IGF,
28952895
return false;
28962896
}
28972897

2898-
auto deinitTable = IGF.getSILModule().lookUpMoveOnlyDeinit(nominal);
2898+
auto deinitTable = IGF.getSILModule().lookUpMoveOnlyDeinit(
2899+
nominal, false /*deserialize lazily*/);
28992900

2900-
// If we do not have a deinit table, call the value witness instead.
2901+
// If we do not have a deinit table already deserialized, call the value
2902+
// witness instead.
29012903
if (!deinitTable) {
29022904
irgen::emitDestroyCall(IGF, T, indirect());
29032905
indirectCleanup();

‎test/Interpreter/Inputs/moveonly_split_module_source_input.swift

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,61 @@
11

2-
#if TEST_LIBRARY_EVOLUTION
2+
#if TEST_LIBRARY_WITH_LIBRARY_EVOLUTION
33
public struct MoveOnly : ~Copyable {
4-
var name = "John"
5-
public init() {}
6-
deinit {
7-
print("==> I am in the deinit resiliently!")
8-
print("==> My name is: \(name)!")
9-
}
4+
#if MAKE_LARGE
5+
var x0 = 0, x1 = 0, x2 = 0, x3 = 0, x4 = 0, x5 = 0, x6 = 0, x7 = 0,
6+
x8 = 0, x9 = 0, x10 = 0, x11 = 0, x12 = 0, x13 = 0, x14 = 0, x15 = 0,
7+
x16 = 0, x17 = 0, x18 = 0, x19 = 0, x20 = 0, x21 = 0, x22 = 0,
8+
x23 = 0, x24 = 0, x25 = 0, x26 = 0, x27 = 0, x28 = 0, x29 = 0,
9+
x30 = 0, x31 = 0, x32 = 0, x33 = 0, x34 = 0, x35 = 0, x36 = 0,
10+
x37 = 0, x38 = 0
11+
#endif
12+
var name = "John"
13+
14+
public init() {}
15+
deinit {
16+
print("==> LIBRARY_EVOLUTION: I am in the deinit!")
17+
print("==> My name is: \(name)!")
18+
}
19+
}
20+
#else
21+
22+
#if TEST_LIBRARY_WITHOUT_LIBRARY_EVOLUTION
23+
24+
public struct MoveOnly : ~Copyable {
25+
#if MAKE_LARGE
26+
var x0 = 0, x1 = 0, x2 = 0, x3 = 0, x4 = 0, x5 = 0, x6 = 0, x7 = 0,
27+
x8 = 0, x9 = 0, x10 = 0, x11 = 0, x12 = 0, x13 = 0, x14 = 0, x15 = 0,
28+
x16 = 0, x17 = 0, x18 = 0, x19 = 0, x20 = 0, x21 = 0, x22 = 0,
29+
x23 = 0, x24 = 0, x25 = 0, x26 = 0, x27 = 0, x28 = 0, x29 = 0,
30+
x30 = 0, x31 = 0, x32 = 0, x33 = 0, x34 = 0, x35 = 0, x36 = 0,
31+
x37 = 0, x38 = 0
32+
#endif
33+
var name = "John"
34+
public init() {}
35+
deinit {
36+
print("==> LIBRARY: I am in the deinit!")
37+
print("==> My name is: \(name)!")
38+
}
1039
}
40+
1141
#else
42+
1243
struct MoveOnly : ~Copyable {
13-
var name = "John"
14-
deinit {
15-
print("==> I am in the deinit!")
16-
print("==> My name is: \(name)!")
17-
}
44+
#if MAKE_LARGE
45+
var x0 = 0, x1 = 0, x2 = 0, x3 = 0, x4 = 0, x5 = 0, x6 = 0, x7 = 0,
46+
x8 = 0, x9 = 0, x10 = 0, x11 = 0, x12 = 0, x13 = 0, x14 = 0, x15 = 0,
47+
x16 = 0, x17 = 0, x18 = 0, x19 = 0, x20 = 0, x21 = 0, x22 = 0,
48+
x23 = 0, x24 = 0, x25 = 0, x26 = 0, x27 = 0, x28 = 0, x29 = 0,
49+
x30 = 0, x31 = 0, x32 = 0, x33 = 0, x34 = 0, x35 = 0, x36 = 0,
50+
x37 = 0, x38 = 0
51+
#endif
52+
53+
var name = "John"
54+
deinit {
55+
print("==> I am in the deinit!")
56+
print("==> My name is: \(name)!")
57+
}
1858
}
59+
60+
#endif
1961
#endif

‎test/Interpreter/moveonly_deinit_separate_library.swift

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Normal test
2+
3+
// RUN: %empty-directory(%t/normal)
4+
// RUN: %target-build-swift-dylib(%t/normal/%target-library-name(MoveOnlySplit)) %S/Inputs/moveonly_split_module_source_input.swift -emit-module -emit-module-path %t/normal/MoveOnlySplit.swiftmodule -module-name MoveOnlySplit -DTEST_LIBRARY_WITHOUT_LIBRARY_EVOLUTION
5+
// RUN: %target-codesign %t/normal/%target-library-name(MoveOnlySplit)
6+
7+
// RUN: %target-build-swift %s -lMoveOnlySplit -I %t/normal -L %t/normal -o %t/normal/main %target-rpath(%t/normal)
8+
// RUN: %target-codesign %t/normal/main
9+
// RUN: %target-run %t/normal/main %t/normal/%target-library-name(MoveOnlySplit) | %FileCheck %s
10+
11+
// Normal large
12+
13+
// RUN: %empty-directory(%t/normal_large)
14+
// RUN: %target-build-swift-dylib(%t/normal_large/%target-library-name(MoveOnlySplit)) %S/Inputs/moveonly_split_module_source_input.swift -emit-module -emit-module-path %t/normal_large/MoveOnlySplit.swiftmodule -module-name MoveOnlySplit -DTEST_LIBRARY_WITHOUT_LIBRARY_EVOLUTION -DMAKE_LARGE
15+
// RUN: %target-codesign %t/normal_large/%target-library-name(MoveOnlySplit)
16+
17+
// RUN: %target-build-swift %s -lMoveOnlySplit -I %t/normal_large -L %t/normal_large -o %t/normal_large/main %target-rpath(%t/normal_large)
18+
// RUN: %target-codesign %t/normal_large/main
19+
// RUN: %target-run %t/normal_large/main %t/normal_large/%target-library-name(MoveOnlySplit) | %FileCheck %s
20+
21+
// Library evolution test
22+
23+
// RUN: %empty-directory(%t/library_evolution)
24+
// RUN: %target-build-swift-dylib(%t/library_evolution/%target-library-name(MoveOnlySplit)) %S/Inputs/moveonly_split_module_source_input.swift -emit-module -emit-module-path %t/library_evolution/MoveOnlySplit.swiftmodule -module-name MoveOnlySplit -DTEST_LIBRARY_WITH_LIBRARY_EVOLUTION
25+
// RUN: %target-codesign %t/library_evolution/%target-library-name(MoveOnlySplit)
26+
27+
// RUN: %target-build-swift %s -lMoveOnlySplit -I %t/library_evolution -L %t/library_evolution -o %t/library_evolution/main %target-rpath(%t/library_evolution)
28+
// RUN: %target-codesign %t/library_evolution/main
29+
// RUN: %target-run %t/library_evolution/main %t/library_evolution/%target-library-name(MoveOnlySplit) | %FileCheck -check-prefix=CHECK-LIBRARY-EVOLUTION %s
30+
31+
// Library evolution large
32+
33+
// RUN: %empty-directory(%t/library_evolution_large)
34+
// RUN: %target-build-swift-dylib(%t/library_evolution_large/%target-library-name(MoveOnlySplit)) %S/Inputs/moveonly_split_module_source_input.swift -emit-module -emit-module-path %t/library_evolution_large/MoveOnlySplit.swiftmodule -module-name MoveOnlySplit -DTEST_LIBRARY_WITH_LIBRARY_EVOLUTION -DMAKE_LARGE
35+
// RUN: %target-codesign %t/library_evolution_large/%target-library-name(MoveOnlySplit)
36+
37+
// RUN: %target-build-swift %s -lMoveOnlySplit -I %t/library_evolution_large -L %t/library_evolution_large -o %t/library_evolution_large/main %target-rpath(%t/library_evolution_large)
38+
// RUN: %target-codesign %t/library_evolution_large/main
39+
// RUN: %target-run %t/library_evolution_large/main %t/library_evolution_large/%target-library-name(MoveOnlySplit) | %FileCheck -check-prefix=CHECK-LIBRARY-EVOLUTION %s
40+
41+
42+
// REQUIRES: executable_test
43+
44+
import MoveOnlySplit
45+
46+
func main() {
47+
// CHECK: ==> LIBRARY: I am in the deinit!
48+
// CHECK: ==> My name is: John!
49+
// CHECK-LIBRARY-EVOLUTION: ==> LIBRARY_EVOLUTION: I am in the deinit!
50+
// CHECK-LIBRARY-EVOLUTION: ==> My name is: John!
51+
let server = MoveOnly()
52+
}
53+
54+
main()

‎test/Interpreter/moveonly_split_module_source_deinit.swift

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
1+
12
// Normal test.
3+
// RUN: %empty-directory(%t/normal)
4+
// RUN: %target-swiftc_driver -emit-module -module-name server -emit-module-path %t/normal/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift
5+
// RUN: %target-swiftc_driver -emit-executable -module-name server -emit-module-path %t/normal/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift -o %t/normal/server
6+
// RUN: %target-codesign %t/normal/server
7+
// RUN: %target-run %t/normal/server | %FileCheck %s
28

3-
// RUN: %empty-directory(%t)
4-
// RUN: %target-swiftc_driver -emit-module -module-name server -emit-module-path %t/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift
5-
// RUN: %target-swiftc_driver -emit-executable -module-name server -emit-module-path %t/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift -o %t/server
6-
// RUN: %target-codesign %t/server
7-
// RUN: %target-run %t/server | %FileCheck %s
9+
// Large test.
10+
// RUN: %empty-directory(%t/large)
11+
// RUN: %target-swiftc_driver -emit-module -module-name server -emit-module-path %t/large/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift -DMAKE_LARGE
12+
// RUN: %target-swiftc_driver -emit-executable -module-name server -emit-module-path %t/large/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift -o %t/large/server
13+
// RUN: %target-codesign %t/large/server
14+
// RUN: %target-run %t/large/server | %FileCheck %s
815

916
// REQUIRES: executable_test
1017

‎test/Interpreter/moveonly_split_module_source_deinit_library_evolution.swift

Lines changed: 0 additions & 17 deletions
This file was deleted.

0 commit comments

Comments
 (0)