diff --git a/include/swift/AST/CASTBridging.h b/include/swift/AST/CASTBridging.h index 07643be909fcc..e361cb35c0a1c 100644 --- a/include/swift/AST/CASTBridging.h +++ b/include/swift/AST/CASTBridging.h @@ -307,6 +307,9 @@ void Plugin_lock(PluginHandle handle); /// Unlock the plugin. void Plugin_unlock(PluginHandle handle); +/// Launch the plugin if it's not running. +_Bool Plugin_spawnIfNeeded(PluginHandle handle); + /// Sends the message to the plugin, returns true if there was an error. /// Clients should receive the response by \c Plugin_waitForNextMessage . _Bool Plugin_sendMessage(PluginHandle handle, const BridgedData data); diff --git a/include/swift/AST/PluginRegistry.h b/include/swift/AST/PluginRegistry.h index 9fb91517fbd8a..b9075947ef27a 100644 --- a/include/swift/AST/PluginRegistry.h +++ b/include/swift/AST/PluginRegistry.h @@ -22,36 +22,79 @@ namespace swift { +/// Represent a "resolved" exectuable plugin. +/// +/// Plugin clients usually deal with this object to communicate with the actual +/// plugin implementation. +/// This object has a file path of the plugin executable, and is responsible to +/// launch it and manages the process. When the plugin process crashes, this +/// should automatically relaunch the process so the clients can keep using this +/// object as the interface. class LoadedExecutablePlugin { - const llvm::sys::procid_t pid; + + /// Represents the current process of the executable plugin. + struct PluginProcess { + const llvm::sys::procid_t pid; + const int inputFileDescriptor; + const int outputFileDescriptor; + bool isStale = false; + + PluginProcess(llvm::sys::procid_t pid, int inputFileDescriptor, + int outputFileDescriptor); + + ~PluginProcess(); + + ssize_t write(const void *buf, size_t nbyte) const; + ssize_t read(void *buf, size_t nbyte) const; + }; + + /// Launched current process. + std::unique_ptr Process; + + /// Path to the plugin executable. + const std::string ExecutablePath; + + /// Last modification time of the `ExecutablePath` when this is initialized. const llvm::sys::TimePoint<> LastModificationTime; - const int inputFileDescriptor; - const int outputFileDescriptor; /// Opaque value of the protocol capability of the pluugin. This is a /// value from ASTGen. const void *capability = nullptr; + /// Callbacks to be called when the connection is restored. + llvm::SmallVector *, 0> onReconnect; + /// Cleanup function to call ASTGen. std::function cleanup; std::mutex mtx; - ssize_t write(const void *buf, size_t nbyte) const; - ssize_t read(void *buf, size_t nbyte) const; - public: - LoadedExecutablePlugin(llvm::sys::procid_t pid, - llvm::sys::TimePoint<> LastModificationTime, - int inputFileDescriptor, int outputFileDescriptor); + LoadedExecutablePlugin(llvm::StringRef ExecutablePath, + llvm::sys::TimePoint<> LastModificationTime) + : ExecutablePath(ExecutablePath), + LastModificationTime(LastModificationTime){}; ~LoadedExecutablePlugin(); + + /// The last modification time of 'ExecutablePath' when this object is + /// created. llvm::sys::TimePoint<> getLastModificationTime() const { return LastModificationTime; } + /// Indicates that the current process is usable. + bool isAlive() const { return Process != nullptr && !Process->isStale; } + + /// Mark the current process "stale". + void setStale() const { Process->isStale = true; } + void lock() { mtx.lock(); } void unlock() { mtx.unlock(); } + // Launch the plugin if it's not already running, or it's stale. Return an + // error if it's fails to execute it. + llvm::Error spawnIfNeeded(); + /// Send a message to the plugin. llvm::Error sendMessage(llvm::StringRef message) const; @@ -63,7 +106,18 @@ class LoadedExecutablePlugin { this->cleanup = cleanup; } - llvm::sys::procid_t getPid() { return pid; } + /// Add "on reconnect" callback. + /// These callbacks are called when `spawnIfNeeded()` relaunched the plugin. + void addOnReconnect(std::function *fn) { + onReconnect.push_back(fn); + } + + /// Remove "on reconnect" callback. + void removeOnReconnect(std::function *fn) { + llvm::erase_value(onReconnect, fn); + } + + llvm::sys::procid_t getPid() { return Process->pid; } const void *getCapability() { return capability; }; void setCapability(const void *newValue) { capability = newValue; }; @@ -78,7 +132,12 @@ class PluginRegistry { LoadedPluginExecutables; public: + /// Load a dynamic link library specified by \p path. + /// If \p path plugin is already loaded, this returns the cached object. llvm::Expected loadLibraryPlugin(llvm::StringRef path); + + /// Load an executable plugin specified by \p path . + /// If \p path plugin is already loaded, this returns the cached object. llvm::Expected loadExecutablePlugin(llvm::StringRef path); diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index d0a9c8022d67a..c12da99da7757 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -528,6 +528,9 @@ struct ASTContext::Implementation { /// NOTE: Do not reference this directly. Use ASTContext::getPluginRegistry(). PluginRegistry *Plugins = nullptr; + /// `Plugins` storage if this ASTContext owns it. + std::unique_ptr OwnedPluginRegistry = nullptr; + /// Cache of loaded symbols. llvm::StringMap LoadedSymbols; @@ -6255,9 +6258,7 @@ PluginRegistry *ASTContext::getPluginRegistry() const { // Create a new one if it hasn't been set. if (!registry) { registry = new PluginRegistry(); - const_cast(this)->addCleanup([registry]{ - delete registry; - }); + getImpl().OwnedPluginRegistry.reset(registry); } assert(registry != nullptr); diff --git a/lib/AST/CASTBridging.cpp b/lib/AST/CASTBridging.cpp index d7fed9a6c99e1..f92eb505bbf7e 100644 --- a/lib/AST/CASTBridging.cpp +++ b/lib/AST/CASTBridging.cpp @@ -649,6 +649,14 @@ void Plugin_unlock(PluginHandle handle) { plugin->unlock(); } +bool Plugin_spawnIfNeeded(PluginHandle handle) { + auto *plugin = static_cast(handle); + auto error = plugin->spawnIfNeeded(); + bool hadError(error); + llvm::consumeError(std::move(error)); + return hadError; +} + bool Plugin_sendMessage(PluginHandle handle, const BridgedData data) { auto *plugin = static_cast(handle); StringRef message(data.baseAddress, data.size); diff --git a/lib/AST/PluginRegistry.cpp b/lib/AST/PluginRegistry.cpp index d8873a91023df..05d9f46d80be3 100644 --- a/lib/AST/PluginRegistry.cpp +++ b/lib/AST/PluginRegistry.cpp @@ -38,9 +38,6 @@ #include #endif -extern "C" const void *swift_ASTGen_getCompilerPluginCapability(void *handle); -extern "C" void swift_ASTGen_destroyCompilerPluginCapability(void *value); - using namespace swift; llvm::Expected PluginRegistry::loadLibraryPlugin(StringRef path) { @@ -74,14 +71,14 @@ PluginRegistry::loadExecutablePlugin(StringRef path) { } // See if the plugin is already loaded. - auto &plugin = LoadedPluginExecutables[path]; - if (plugin) { + auto &storage = LoadedPluginExecutables[path]; + if (storage) { // See if the loaded one is still usable. - if (plugin->getLastModificationTime() == stat.getLastModificationTime()) - return plugin.get(); + if (storage->getLastModificationTime() == stat.getLastModificationTime()) + return storage.get(); // The plugin is updated. Close the previously opened plugin. - plugin = nullptr; + storage = nullptr; } if (!llvm::sys::fs::exists(stat)) { @@ -94,8 +91,35 @@ PluginRegistry::loadExecutablePlugin(StringRef path) { "not executable"); } + auto plugin = std::unique_ptr( + new LoadedExecutablePlugin(path, stat.getLastModificationTime())); + + // Launch here to see if it's actually executable, and diagnose (by returning + // an error) if necessary. + if (auto error = plugin->spawnIfNeeded()) { + return std::move(error); + } + + storage = std::move(plugin); + return storage.get(); +} + +llvm::Error LoadedExecutablePlugin::spawnIfNeeded() { + if (Process) { + // See if the loaded one is still usable. + if (!Process->isStale) + return llvm::Error::success(); + + // NOTE: We don't check the mtime here because 'stat(2)' call is too heavy. + // PluginRegistry::loadExecutablePlugin() checks it and replace this object + // itself if the plugin is updated. + + // The plugin is stale. Discard the previously opened process. + Process.reset(); + } + // Create command line arguments. - SmallVector command{path}; + SmallVector command{ExecutablePath}; // Apply sandboxing. llvm::BumpPtrAllocator Allocator; @@ -107,29 +131,36 @@ PluginRegistry::loadExecutablePlugin(StringRef path) { return llvm::errorCodeToError(childInfo.getError()); } - plugin = std::unique_ptr(new LoadedExecutablePlugin( - childInfo->Pid, stat.getLastModificationTime(), - childInfo->ReadFileDescriptor, childInfo->WriteFileDescriptor)); + Process = std::unique_ptr( + new PluginProcess(childInfo->Pid, childInfo->ReadFileDescriptor, + childInfo->WriteFileDescriptor)); + + // Call "on reconnect" callbacks. + for (auto *callback : onReconnect) { + (*callback)(); + } - return plugin.get(); + return llvm::Error::success(); } -LoadedExecutablePlugin::LoadedExecutablePlugin( - llvm::sys::procid_t pid, llvm::sys::TimePoint<> LastModificationTime, - int inputFileDescriptor, int outputFileDescriptor) - : pid(pid), LastModificationTime(LastModificationTime), - inputFileDescriptor(inputFileDescriptor), +LoadedExecutablePlugin::PluginProcess::PluginProcess(llvm::sys::procid_t pid, + int inputFileDescriptor, + int outputFileDescriptor) + : pid(pid), inputFileDescriptor(inputFileDescriptor), outputFileDescriptor(outputFileDescriptor) {} -LoadedExecutablePlugin::~LoadedExecutablePlugin() { +LoadedExecutablePlugin::PluginProcess::~PluginProcess() { close(inputFileDescriptor); close(outputFileDescriptor); +} +LoadedExecutablePlugin::~LoadedExecutablePlugin() { // Let ASTGen to cleanup things. this->cleanup(); } -ssize_t LoadedExecutablePlugin::read(void *buf, size_t nbyte) const { +ssize_t LoadedExecutablePlugin::PluginProcess::read(void *buf, + size_t nbyte) const { ssize_t bytesToRead = nbyte; void *ptr = buf; @@ -154,7 +185,8 @@ ssize_t LoadedExecutablePlugin::read(void *buf, size_t nbyte) const { return nbyte - bytesToRead; } -ssize_t LoadedExecutablePlugin::write(const void *buf, size_t nbyte) const { +ssize_t LoadedExecutablePlugin::PluginProcess::write(const void *buf, + size_t nbyte) const { ssize_t bytesToWrite = nbyte; const void *ptr = buf; @@ -187,15 +219,17 @@ llvm::Error LoadedExecutablePlugin::sendMessage(llvm::StringRef message) const { // Write header (message size). uint64_t header = llvm::support::endian::byte_swap( uint64_t(size), llvm::support::endianness::little); - writtenSize = write(&header, sizeof(header)); + writtenSize = Process->write(&header, sizeof(header)); if (writtenSize != sizeof(header)) { + setStale(); return llvm::createStringError(llvm::inconvertibleErrorCode(), "failed to write plugin message header"); } // Write message. - writtenSize = write(data, size); + writtenSize = Process->write(data, size); if (writtenSize != ssize_t(size)) { + setStale(); return llvm::createStringError(llvm::inconvertibleErrorCode(), "failed to write plugin message data"); } @@ -208,9 +242,10 @@ llvm::Expected LoadedExecutablePlugin::waitForNextMessage() const { // Read header (message size). uint64_t header; - readSize = read(&header, sizeof(header)); + readSize = Process->read(&header, sizeof(header)); if (readSize != sizeof(header)) { + setStale(); return llvm::createStringError(llvm::inconvertibleErrorCode(), "failed to read plugin message header"); } @@ -224,8 +259,9 @@ llvm::Expected LoadedExecutablePlugin::waitForNextMessage() const { auto sizeToRead = size; while (sizeToRead > 0) { char buffer[4096]; - readSize = read(buffer, std::min(sizeof(buffer), sizeToRead)); + readSize = Process->read(buffer, std::min(sizeof(buffer), sizeToRead)); if (readSize == 0) { + setStale(); return llvm::createStringError(llvm::inconvertibleErrorCode(), "failed to read plugin message data"); } diff --git a/lib/ASTGen/Sources/ASTGen/PluginHost.swift b/lib/ASTGen/Sources/ASTGen/PluginHost.swift index 1cbd3a53943a0..b703b573a0e9b 100644 --- a/lib/ASTGen/Sources/ASTGen/PluginHost.swift +++ b/lib/ASTGen/Sources/ASTGen/PluginHost.swift @@ -16,6 +16,7 @@ import SwiftSyntax import swiftLLVMJSON enum PluginError: Error { + case stalePlugin case failedToSendMessage case failedToReceiveMessage case invalidReponseKind @@ -38,30 +39,37 @@ public func _deinitializePlugin( } /// Load the library plugin in the plugin server. +/// This should be called inside lock. @_cdecl("swift_ASTGen_pluginServerLoadLibraryPlugin") func swift_ASTGen_pluginServerLoadLibraryPlugin( opaqueHandle: UnsafeMutableRawPointer, libraryPath: UnsafePointer, moduleName: UnsafePointer, - cxxDiagnosticEngine: UnsafeMutablePointer + cxxDiagnosticEngine: UnsafeMutablePointer? ) -> Bool { let plugin = CompilerPlugin(opaqueHandle: opaqueHandle) assert(plugin.capability?.features.contains(.loadPluginLibrary) == true) let libraryPath = String(cString: libraryPath) let moduleName = String(cString: moduleName) - let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine) + + let diagEngine: PluginDiagnosticsEngine? + if let cxxDiagnosticEngine = cxxDiagnosticEngine { + diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine) + } else { + diagEngine = nil + } do { - let result = try plugin.sendMessageAndWait( + let result = try plugin.sendMessageAndWaitWithoutLock( .loadPluginLibrary(libraryPath: libraryPath, moduleName: moduleName) ) guard case .loadPluginLibraryResult(let loaded, let diagnostics) = result else { throw PluginError.invalidReponseKind } - diagEngine.emit(diagnostics); + diagEngine?.emit(diagnostics); return loaded } catch { - diagEngine.diagnose(error: error) + diagEngine?.diagnose(error: error) return false } } @@ -121,28 +129,31 @@ struct CompilerPlugin { return try LLVMJSON.decode(PluginToHostMessage.self, from: data) } + func sendMessageAndWaitWithoutLock(_ message: HostToPluginMessage) throws -> PluginToHostMessage { + try sendMessage(message) + return try waitForNextMessage() + } + func sendMessageAndWait(_ message: HostToPluginMessage) throws -> PluginToHostMessage { try self.withLock { - try sendMessage(message) - return try waitForNextMessage() + guard !Plugin_spawnIfNeeded(opaqueHandle) else { + throw PluginError.stalePlugin + } + return try sendMessageAndWaitWithoutLock(message); } } + /// Initialize the plugin. This should be called inside lock. func initialize() { - // Don't use `sendMessageAndWait` because we want to keep the lock until - // setting the returned value. do { - try self.withLock { - // Get capability. - try self.sendMessage(.getCapability) - let response = try self.waitForNextMessage() - guard case .getCapabilityResult(let capability) = response else { - throw PluginError.invalidReponseKind - } - let ptr = UnsafeMutablePointer.allocate(capacity: 1) - ptr.initialize(to: .init(capability)) - Plugin_setCapability(opaqueHandle, UnsafeRawPointer(ptr)) + // Get capability. + let response = try self.sendMessageAndWaitWithoutLock(.getCapability) + guard case .getCapabilityResult(let capability) = response else { + throw PluginError.invalidReponseKind } + let ptr = UnsafeMutablePointer.allocate(capacity: 1) + ptr.initialize(to: .init(capability)) + Plugin_setCapability(opaqueHandle, UnsafeRawPointer(ptr)) } catch { assertionFailure(String(describing: error)) return diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index e81144792b634..94d8620532495 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -319,6 +319,11 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) { if (!executablePlugin) return nullptr; + // Lock the plugin while initializing. + // Note that'executablePlugn' can be shared between multiple ASTContext. + executablePlugin->lock(); + SWIFT_DEFER { executablePlugin->unlock(); }; + // FIXME: Ideally this should be done right after invoking the plugin. // But plugin loading is in libAST and it can't link ASTGen symbols. if (!executablePlugin->isInitialized()) { @@ -338,11 +343,30 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) { if (fs->getRealPath(libraryPath, resolvedLibraryPath)) { return nullptr; } + std::string resolvedLibraryPathStr(resolvedLibraryPath); + std::string moduleNameStr(moduleName.str()); + bool loaded = swift_ASTGen_pluginServerLoadLibraryPlugin( - executablePlugin, resolvedLibraryPath.c_str(), moduleName.str().data(), + executablePlugin, resolvedLibraryPathStr.c_str(), moduleNameStr.c_str(), &ctx.Diags); if (!loaded) return nullptr; + + // Set a callback to load the library again on reconnections. + auto *callback = new std::function( + [executablePlugin, resolvedLibraryPathStr, moduleNameStr]() { + (void)swift_ASTGen_pluginServerLoadLibraryPlugin( + executablePlugin, resolvedLibraryPathStr.c_str(), + moduleNameStr.c_str(), + /*diags=*/nullptr); + }); + executablePlugin->addOnReconnect(callback); + + // Remove the callback and deallocate it when this ASTContext is destructed. + ctx.addCleanup([executablePlugin, callback]() { + executablePlugin->removeOnReconnect(callback); + delete callback; + }); #endif } diff --git a/test/Macros/Inputs/evil_macro_definitions.swift b/test/Macros/Inputs/evil_macro_definitions.swift new file mode 100644 index 0000000000000..7d84519af336a --- /dev/null +++ b/test/Macros/Inputs/evil_macro_definitions.swift @@ -0,0 +1,17 @@ +import SwiftDiagnostics +import SwiftOperators +import SwiftSyntax +import SwiftSyntaxBuilder +import SwiftSyntaxMacros + + +public struct CrashingMacro: ExpressionMacro { + public static func expansion( + of macro: some FreestandingMacroExpansionSyntax, + in context: some MacroExpansionContext + ) -> ExprSyntax { + let arg: UInt = UInt(macro.argumentList.first!.expression.description)! + let zero: UInt = 0 + return "\(raw: zero - arg).description" + } +} diff --git a/test/Macros/macro_plugin_error.swift b/test/Macros/macro_plugin_error.swift index fa884f54aeb81..4ccbebd2da2d2 100644 --- a/test/Macros/macro_plugin_error.swift +++ b/test/Macros/macro_plugin_error.swift @@ -29,8 +29,7 @@ func test() { let _: String = #fooMacro(2) // expected-error @-1 {{failedToReceiveMessage}} let _: String = #fooMacro(3) - // expected-error @-1 {{failedToSendMessage}} - //FIXME: ^ This should succeed. Error recovery is not implemented. + // ^ This should succeed. } //--- plugin.c @@ -51,6 +50,6 @@ MOCK_PLUGIN([ "expect": {"expandFreestandingMacro": { "macro": {"moduleName": "TestPlugin", "typeName": "FooMacro"}, "syntax": {"kind": "expression", "source": "#fooMacro(3)"}}}, - "response": {"expandFreestandingMacroResult": {"expandedSource": "3", "diagnostics": []}} + "response": {"expandFreestandingMacroResult": {"expandedSource": "3.description", "diagnostics": []}} } ]) diff --git a/test/Macros/macro_plugin_server.swift b/test/Macros/macro_plugin_server.swift index bed9378c4e300..aa49e1bdfcac9 100644 --- a/test/Macros/macro_plugin_server.swift +++ b/test/Macros/macro_plugin_server.swift @@ -15,6 +15,16 @@ // RUN: %S/Inputs/syntax_macro_definitions.swift \ // RUN: -g -no-toolchain-stdlib-rpath +// RUN: %target-build-swift \ +// RUN: -swift-version 5 \ +// RUN: -I %swift-host-lib-dir \ +// RUN: -L %swift-host-lib-dir \ +// RUN: -emit-library \ +// RUN: -o %t/plugins/%target-library-name(EvilMacros) \ +// RUN: -module-name=EvilMacros \ +// RUN: %S/Inputs/evil_macro_definitions.swift \ +// RUN: -g -no-toolchain-stdlib-rpath + // RUN: %swift-target-frontend \ // RUN: -typecheck -verify \ // RUN: -swift-version 5 \ @@ -27,13 +37,23 @@ @freestanding(expression) macro stringify(_ value: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro") +@freestanding(expression) macro evil(_ value: Int) -> String = #externalMacro(module: "EvilMacros", type: "CrashingMacro") func testStringify(a: Int, b: Int) { - let s: String = #stringify(a + b).1 - print(s) + let s1: String = #stringify(a + b).1 + print(s1) + + let s2: String = #evil(42) // expected-error {{failedToReceiveMessage (from macro 'evil')}} + print(s2) + + let s3: String = #stringify(b + a).1 + print(s3) } // CHECK: {{^}}------------------------------ // CHECK-NEXT: {{^}}(a + b, "a + b") // CHECK-NEXT: {{^}}------------------------------ +// CHECK: {{^}}------------------------------ +// CHECK-NEXT: {{^}}(b + a, "b + a") +// CHECK-NEXT: {{^}}------------------------------