From b2a52ff036dd30e37378306cc6a7fbb4fd1c5838 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Fri, 9 Jun 2023 10:33:24 -0700 Subject: [PATCH] [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 --- lib/IRGen/GenType.cpp | 6 +- .../moveonly_split_module_source_input.swift | 66 +++++++++++++++---- .../moveonly_deinit_separate_library.swift | 54 +++++++++++++++ .../moveonly_split_module_source_deinit.swift | 17 +++-- ...dule_source_deinit_library_evolution.swift | 17 ----- 5 files changed, 124 insertions(+), 36 deletions(-) create mode 100644 test/Interpreter/moveonly_deinit_separate_library.swift delete mode 100644 test/Interpreter/moveonly_split_module_source_deinit_library_evolution.swift diff --git a/lib/IRGen/GenType.cpp b/lib/IRGen/GenType.cpp index 0eb7def337d8f..82f31d8923ac2 100644 --- a/lib/IRGen/GenType.cpp +++ b/lib/IRGen/GenType.cpp @@ -2897,9 +2897,11 @@ static bool tryEmitDeinitCall(IRGenFunction &IGF, return false; } - auto deinitTable = IGF.getSILModule().lookUpMoveOnlyDeinit(nominal); + auto deinitTable = IGF.getSILModule().lookUpMoveOnlyDeinit( + nominal, false /*deserialize lazily*/); - // If we do not have a deinit table, call the value witness instead. + // If we do not have a deinit table already deserialized, call the value + // witness instead. if (!deinitTable) { irgen::emitDestroyCall(IGF, T, indirect()); indirectCleanup(); diff --git a/test/Interpreter/Inputs/moveonly_split_module_source_input.swift b/test/Interpreter/Inputs/moveonly_split_module_source_input.swift index ab9859bebc5e4..4e38c17c29838 100644 --- a/test/Interpreter/Inputs/moveonly_split_module_source_input.swift +++ b/test/Interpreter/Inputs/moveonly_split_module_source_input.swift @@ -1,19 +1,61 @@ -#if TEST_LIBRARY_EVOLUTION +#if TEST_LIBRARY_WITH_LIBRARY_EVOLUTION public struct MoveOnly : ~Copyable { - var name = "John" - public init() {} - deinit { - print("==> I am in the deinit resiliently!") - print("==> My name is: \(name)!") - } +#if MAKE_LARGE + var x0 = 0, x1 = 0, x2 = 0, x3 = 0, x4 = 0, x5 = 0, x6 = 0, x7 = 0, + x8 = 0, x9 = 0, x10 = 0, x11 = 0, x12 = 0, x13 = 0, x14 = 0, x15 = 0, + x16 = 0, x17 = 0, x18 = 0, x19 = 0, x20 = 0, x21 = 0, x22 = 0, + x23 = 0, x24 = 0, x25 = 0, x26 = 0, x27 = 0, x28 = 0, x29 = 0, + x30 = 0, x31 = 0, x32 = 0, x33 = 0, x34 = 0, x35 = 0, x36 = 0, + x37 = 0, x38 = 0 +#endif + var name = "John" + + public init() {} + deinit { + print("==> LIBRARY_EVOLUTION: I am in the deinit!") + print("==> My name is: \(name)!") + } +} +#else + +#if TEST_LIBRARY_WITHOUT_LIBRARY_EVOLUTION + +public struct MoveOnly : ~Copyable { +#if MAKE_LARGE + var x0 = 0, x1 = 0, x2 = 0, x3 = 0, x4 = 0, x5 = 0, x6 = 0, x7 = 0, + x8 = 0, x9 = 0, x10 = 0, x11 = 0, x12 = 0, x13 = 0, x14 = 0, x15 = 0, + x16 = 0, x17 = 0, x18 = 0, x19 = 0, x20 = 0, x21 = 0, x22 = 0, + x23 = 0, x24 = 0, x25 = 0, x26 = 0, x27 = 0, x28 = 0, x29 = 0, + x30 = 0, x31 = 0, x32 = 0, x33 = 0, x34 = 0, x35 = 0, x36 = 0, + x37 = 0, x38 = 0 +#endif + var name = "John" + public init() {} + deinit { + print("==> LIBRARY: I am in the deinit!") + print("==> My name is: \(name)!") + } } + #else + struct MoveOnly : ~Copyable { - var name = "John" - deinit { - print("==> I am in the deinit!") - print("==> My name is: \(name)!") - } +#if MAKE_LARGE + var x0 = 0, x1 = 0, x2 = 0, x3 = 0, x4 = 0, x5 = 0, x6 = 0, x7 = 0, + x8 = 0, x9 = 0, x10 = 0, x11 = 0, x12 = 0, x13 = 0, x14 = 0, x15 = 0, + x16 = 0, x17 = 0, x18 = 0, x19 = 0, x20 = 0, x21 = 0, x22 = 0, + x23 = 0, x24 = 0, x25 = 0, x26 = 0, x27 = 0, x28 = 0, x29 = 0, + x30 = 0, x31 = 0, x32 = 0, x33 = 0, x34 = 0, x35 = 0, x36 = 0, + x37 = 0, x38 = 0 +#endif + + var name = "John" + deinit { + print("==> I am in the deinit!") + print("==> My name is: \(name)!") + } } + +#endif #endif diff --git a/test/Interpreter/moveonly_deinit_separate_library.swift b/test/Interpreter/moveonly_deinit_separate_library.swift new file mode 100644 index 0000000000000..f2687bd904268 --- /dev/null +++ b/test/Interpreter/moveonly_deinit_separate_library.swift @@ -0,0 +1,54 @@ +// Normal test + +// RUN: %empty-directory(%t/normal) +// 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 +// RUN: %target-codesign %t/normal/%target-library-name(MoveOnlySplit) + +// RUN: %target-build-swift %s -lMoveOnlySplit -I %t/normal -L %t/normal -o %t/normal/main %target-rpath(%t/normal) +// RUN: %target-codesign %t/normal/main +// RUN: %target-run %t/normal/main %t/normal/%target-library-name(MoveOnlySplit) | %FileCheck %s + +// Normal large + +// RUN: %empty-directory(%t/normal_large) +// 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 +// RUN: %target-codesign %t/normal_large/%target-library-name(MoveOnlySplit) + +// RUN: %target-build-swift %s -lMoveOnlySplit -I %t/normal_large -L %t/normal_large -o %t/normal_large/main %target-rpath(%t/normal_large) +// RUN: %target-codesign %t/normal_large/main +// RUN: %target-run %t/normal_large/main %t/normal_large/%target-library-name(MoveOnlySplit) | %FileCheck %s + +// Library evolution test + +// RUN: %empty-directory(%t/library_evolution) +// 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 +// RUN: %target-codesign %t/library_evolution/%target-library-name(MoveOnlySplit) + +// RUN: %target-build-swift %s -lMoveOnlySplit -I %t/library_evolution -L %t/library_evolution -o %t/library_evolution/main %target-rpath(%t/library_evolution) +// RUN: %target-codesign %t/library_evolution/main +// RUN: %target-run %t/library_evolution/main %t/library_evolution/%target-library-name(MoveOnlySplit) | %FileCheck -check-prefix=CHECK-LIBRARY-EVOLUTION %s + +// Library evolution large + +// RUN: %empty-directory(%t/library_evolution_large) +// 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 +// RUN: %target-codesign %t/library_evolution_large/%target-library-name(MoveOnlySplit) + +// 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) +// RUN: %target-codesign %t/library_evolution_large/main +// RUN: %target-run %t/library_evolution_large/main %t/library_evolution_large/%target-library-name(MoveOnlySplit) | %FileCheck -check-prefix=CHECK-LIBRARY-EVOLUTION %s + + +// REQUIRES: executable_test + +import MoveOnlySplit + +func main() { + // CHECK: ==> LIBRARY: I am in the deinit! + // CHECK: ==> My name is: John! + // CHECK-LIBRARY-EVOLUTION: ==> LIBRARY_EVOLUTION: I am in the deinit! + // CHECK-LIBRARY-EVOLUTION: ==> My name is: John! + let server = MoveOnly() +} + +main() diff --git a/test/Interpreter/moveonly_split_module_source_deinit.swift b/test/Interpreter/moveonly_split_module_source_deinit.swift index a6a073680019d..31637772171b1 100644 --- a/test/Interpreter/moveonly_split_module_source_deinit.swift +++ b/test/Interpreter/moveonly_split_module_source_deinit.swift @@ -1,10 +1,17 @@ + // Normal test. +// RUN: %empty-directory(%t/normal) +// 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 +// 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 +// RUN: %target-codesign %t/normal/server +// RUN: %target-run %t/normal/server | %FileCheck %s -// RUN: %empty-directory(%t) -// RUN: %target-swiftc_driver -emit-module -module-name server -emit-module-path %t/server.swiftmodule %s %S/Inputs/moveonly_split_module_source_input.swift -// 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 -// RUN: %target-codesign %t/server -// RUN: %target-run %t/server | %FileCheck %s +// Large test. +// RUN: %empty-directory(%t/large) +// 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 +// 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 +// RUN: %target-codesign %t/large/server +// RUN: %target-run %t/large/server | %FileCheck %s // REQUIRES: executable_test diff --git a/test/Interpreter/moveonly_split_module_source_deinit_library_evolution.swift b/test/Interpreter/moveonly_split_module_source_deinit_library_evolution.swift deleted file mode 100644 index 45910f262f528..0000000000000 --- a/test/Interpreter/moveonly_split_module_source_deinit_library_evolution.swift +++ /dev/null @@ -1,17 +0,0 @@ -// RUN: %empty-directory(%t) -// RUN: %target-build-swift-dylib(%t/%target-library-name(MoveOnlySplit)) -enable-library-evolution %S/Inputs/moveonly_split_module_source_input.swift -emit-module -emit-module-path %t/MoveOnlySplit.swiftmodule -module-name MoveOnlySplit -DTEST_LIBRARY_EVOLUTION -// RUN: %target-codesign %t/%target-library-name(MoveOnlySplit) - -// RUN: %target-build-swift %s -lMoveOnlySplit -I %t -L %t -o %t/main %target-rpath(%t) -// RUN: %target-codesign %t/main -// RUN: %target-run %t/main %t/%target-library-name(MoveOnlySplit) | %FileCheck -check-prefix=CHECK-LIBRARY-EVOLUTION %s - -// REQUIRES: executable_test - -import MoveOnlySplit - -func main() { - let server = MoveOnly() // CHECK-LIBRARY-EVOLUTION: ==> I am in the deinit resiliently! -} - -main()