Skip to content

Commit e93e4ba

Browse files
committed
[Macro] Handle error during initializing executable plugins
Previously, the compiler fails to read the result from 'getCapability' IPC message, it used to just emit a fatalError and crashed. Instead, propagate the error status and diagnose it. rdar://108022847
1 parent 0bdb39f commit e93e4ba

File tree

4 files changed

+80
-26
lines changed

4 files changed

+80
-26
lines changed

lib/AST/PluginRegistry.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,8 @@ LoadedExecutablePlugin::PluginProcess::~PluginProcess() {
178178

179179
LoadedExecutablePlugin::~LoadedExecutablePlugin() {
180180
// Let ASTGen to cleanup things.
181-
this->cleanup();
181+
if (this->cleanup)
182+
this->cleanup();
182183
}
183184

184185
ssize_t LoadedExecutablePlugin::PluginProcess::read(void *buf,

lib/ASTGen/Sources/ASTGen/PluginHost.swift

+28-22
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,18 @@ enum PluginError: Error {
2424

2525
@_cdecl("swift_ASTGen_initializePlugin")
2626
public func _initializePlugin(
27-
opaqueHandle: UnsafeMutableRawPointer
28-
) {
27+
opaqueHandle: UnsafeMutableRawPointer,
28+
cxxDiagnosticEngine: UnsafeMutablePointer<UInt8>?
29+
) -> Bool {
2930
let plugin = CompilerPlugin(opaqueHandle: opaqueHandle)
30-
plugin.initialize()
31+
do {
32+
try plugin.initialize()
33+
return true
34+
} catch {
35+
// Diagnostics are emitted in the caller.
36+
// FIXME: Return what happened or emit diagnostics here.
37+
return false
38+
}
3139
}
3240

3341
@_cdecl("swift_ASTGen_deinitializePlugin")
@@ -52,12 +60,7 @@ func swift_ASTGen_pluginServerLoadLibraryPlugin(
5260
let libraryPath = String(cString: libraryPath)
5361
let moduleName = String(cString: moduleName)
5462

55-
let diagEngine: PluginDiagnosticsEngine?
56-
if let cxxDiagnosticEngine = cxxDiagnosticEngine {
57-
diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine)
58-
} else {
59-
diagEngine = nil
60-
}
63+
let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine)
6164

6265
do {
6366
let result = try plugin.sendMessageAndWaitWithoutLock(
@@ -136,20 +139,15 @@ struct CompilerPlugin {
136139
}
137140

138141
/// Initialize the plugin. This should be called inside lock.
139-
func initialize() {
140-
do {
141-
// Get capability.
142-
let response = try self.sendMessageAndWaitWithoutLock(.getCapability)
143-
guard case .getCapabilityResult(let capability) = response else {
144-
throw PluginError.invalidReponseKind
145-
}
146-
let ptr = UnsafeMutablePointer<Capability>.allocate(capacity: 1)
147-
ptr.initialize(to: .init(capability))
148-
Plugin_setCapability(opaqueHandle, UnsafeRawPointer(ptr))
149-
} catch {
150-
assertionFailure(String(describing: error))
151-
return
142+
func initialize() throws {
143+
// Get capability.
144+
let response = try self.sendMessageAndWaitWithoutLock(.getCapability)
145+
guard case .getCapabilityResult(let capability) = response else {
146+
throw PluginError.invalidReponseKind
152147
}
148+
let ptr = UnsafeMutablePointer<Capability>.allocate(capacity: 1)
149+
ptr.initialize(to: .init(capability))
150+
Plugin_setCapability(opaqueHandle, UnsafeRawPointer(ptr))
153151
}
154152

155153
func deinitialize() {
@@ -179,6 +177,14 @@ class PluginDiagnosticsEngine {
179177
self.cxxDiagnosticEngine = cxxDiagnosticEngine
180178
}
181179

180+
/// Failable convenience initializer for optional cxx engine pointer.
181+
convenience init?(cxxDiagnosticEngine: UnsafeMutablePointer<UInt8>?) {
182+
guard let cxxDiagnosticEngine = cxxDiagnosticEngine else {
183+
return nil
184+
}
185+
self.init(cxxDiagnosticEngine: cxxDiagnosticEngine)
186+
}
187+
182188
/// Register an 'ExportedSourceFile' to the engine. So the engine can get
183189
/// C++ SourceLoc from a pair of filename and offset.
184190
func add(exportedSourceFile: UnsafePointer<ExportedSourceFile>) {

lib/Sema/TypeCheckMacros.cpp

+10-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "swift/AST/ASTMangler.h"
2222
#include "swift/AST/ASTNode.h"
2323
#include "swift/AST/CASTBridging.h"
24+
#include "swift/AST/DiagnosticsFrontend.h"
2425
#include "swift/AST/Expr.h"
2526
#include "../AST/InlinableText.h"
2627
#include "swift/AST/MacroDefinition.h"
@@ -75,7 +76,7 @@ extern "C" ptrdiff_t swift_ASTGen_expandAttachedMacro(
7576
void *parentDeclSourceFile, const void *parentDeclSourceLocation,
7677
const char **evaluatedSource, ptrdiff_t *evaluatedSourceLength);
7778

78-
extern "C" void swift_ASTGen_initializePlugin(void *handle);
79+
extern "C" bool swift_ASTGen_initializePlugin(void *handle, void *diagEngine);
7980
extern "C" void swift_ASTGen_deinitializePlugin(void *handle);
8081
extern "C" bool swift_ASTGen_pluginServerLoadLibraryPlugin(
8182
void *handle, const char *libraryPath, const char *moduleName,
@@ -309,7 +310,11 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
309310
// But plugin loading is in libAST and it can't link ASTGen symbols.
310311
if (!executablePlugin->isInitialized()) {
311312
#if SWIFT_SWIFT_PARSER
312-
swift_ASTGen_initializePlugin(executablePlugin);
313+
if (!swift_ASTGen_initializePlugin(executablePlugin, &ctx.Diags)) {
314+
ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded,
315+
executablePluginPath, "failed to initialize plugin");
316+
return nullptr;
317+
}
313318
executablePlugin->setCleanup([executablePlugin] {
314319
swift_ASTGen_deinitializePlugin(executablePlugin);
315320
});
@@ -321,7 +326,9 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
321326
#if SWIFT_SWIFT_PARSER
322327
llvm::SmallString<128> resolvedLibraryPath;
323328
auto fs = ctx.SourceMgr.getFileSystem();
324-
if (fs->getRealPath(libraryPath, resolvedLibraryPath)) {
329+
if (auto err = fs->getRealPath(libraryPath, resolvedLibraryPath)) {
330+
ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded,
331+
executablePluginPath, err.message());
325332
return nullptr;
326333
}
327334
std::string resolvedLibraryPathStr(resolvedLibraryPath);

test/Macros/macro_plugin_broken.swift

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// REQUIRES: swift_swift_parser
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: split-file %s %t
5+
6+
// RUN: %host-build-swift \
7+
// RUN: -swift-version 5 -o %t/broken-plugin \
8+
// RUN: -module-name=TestPlugin \
9+
// RUN: %t/broken_plugin.swift \
10+
// RUN: -g -no-toolchain-stdlib-rpath -swift-version 5
11+
12+
// RUN: not %swift-target-frontend \
13+
// RUN: -typecheck \
14+
// RUN: -swift-version 5 -enable-experimental-feature Macros \
15+
// RUN: -load-plugin-executable %t/broken-plugin#TestPlugin \
16+
// RUN: -module-name MyApp \
17+
// RUN: -serialize-diagnostics-path %t/macro_expand.dia \
18+
// RUN: %t/test.swift
19+
20+
// RUN: c-index-test -read-diagnostics %t/macro_expand.dia 2>&1 | %FileCheck -check-prefix CHECK %s
21+
22+
// CHECK: (null):0:0: warning: compiler plugin not loaded: {{.+}}broken-plugin; loader error: failed to initialize plugin
23+
// CHECK: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro';
24+
// CHECK: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro';
25+
// CHECK: +-{{.+}}test.swift:1:33: note: 'fooMacro' declared here
26+
27+
//--- test.swift
28+
@freestanding(expression) macro fooMacro(_: Any) -> String = #externalMacro(module: "TestPlugin", type: "FooMacro")
29+
30+
func test() {
31+
_ = #fooMacro(1)
32+
}
33+
34+
//--- broken_plugin.swift
35+
func minusTen(value: UInt) -> UInt {
36+
// Causes crash.
37+
return value - 10
38+
}
39+
40+
_ = minusTen(value: 5)

0 commit comments

Comments
 (0)