Skip to content

Commit df6651f

Browse files
Improve JSKit diagnostics for use-after-free of JSClosure (#195)
To mitigate the debugging difficulties, JSKit should provide more information in diagnostics.
1 parent ab3da74 commit df6651f

File tree

10 files changed

+68
-29
lines changed

10 files changed

+68
-29
lines changed

IntegrationTests/TestSuites/Sources/PrimaryTests/UnitTestUtils.swift

+6
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ func expectString(_ value: JSValue, file: StaticString = #file, line: UInt = #li
9494
}
9595
}
9696

97+
func expect(_ description: String, _ result: Bool, file: StaticString = #file, line: UInt = #line, column: UInt = #column) throws {
98+
if !result {
99+
throw MessageError(description, file: file, line: line, column: column)
100+
}
101+
}
102+
97103
func expectThrow<T>(_ body: @autoclosure () throws -> T, file: StaticString = #file, line: UInt = #line, column: UInt = #column) throws -> Error {
98104
do {
99105
_ = try body()

IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift

+11
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,17 @@ try test("Closure Lifetime") {
240240
// OneshotClosure won't call fatalError even if it's deallocated before `release`
241241
}
242242
#endif
243+
244+
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
245+
// Check diagnostics of use-after-free
246+
do {
247+
let c1 = JSClosure { $0[0] }
248+
c1.release()
249+
let error = try expectThrow(try evalClosure.throws(c1, JSValue.number(42.0))) as! JSValue
250+
try expect("Error message should contains definition location", error.description.hasSuffix("PrimaryTests/main.swift:247"))
251+
}
252+
#endif
253+
243254
}
244255

245256
try test("Host Function Registration") {

Runtime/src/index.ts

+9-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export class SwiftRuntime {
1414
private _instance: WebAssembly.Instance | null;
1515
private _memory: Memory | null;
1616
private _closureDeallocator: SwiftClosureDeallocator | null;
17-
private version: number = 707;
17+
private version: number = 708;
1818

1919
private textDecoder = new TextDecoder("utf-8");
2020
private textEncoder = new TextEncoder(); // Only support utf-8
@@ -68,7 +68,7 @@ export class SwiftRuntime {
6868
return this._closureDeallocator;
6969
}
7070

71-
private callHostFunction(host_func_id: number, args: any[]) {
71+
private callHostFunction(host_func_id: number, line: number, file: string, args: any[]) {
7272
const argc = args.length;
7373
const argv = this.exports.swjs_prepare_host_function_call(argc);
7474
for (let index = 0; index < args.length; index++) {
@@ -88,12 +88,15 @@ export class SwiftRuntime {
8888
const callback_func_ref = this.memory.retain((result: any) => {
8989
output = result;
9090
});
91-
this.exports.swjs_call_host_function(
91+
const alreadyReleased = this.exports.swjs_call_host_function(
9292
host_func_id,
9393
argv,
9494
argc,
9595
callback_func_ref
9696
);
97+
if (alreadyReleased) {
98+
throw new Error(`The JSClosure has been already released by Swift side. The closure is created at ${file}:${line}`);
99+
}
97100
this.exports.swjs_cleanup_host_function_call(argv);
98101
return output;
99102
}
@@ -371,9 +374,10 @@ export class SwiftRuntime {
371374
return obj instanceof constructor;
372375
},
373376

374-
swjs_create_function: (host_func_id: number) => {
377+
swjs_create_function: (host_func_id: number, line: number, file: ref) => {
378+
const fileString = this.memory.getObject(file) as string;
375379
const func = (...args: any[]) =>
376-
this.callHostFunction(host_func_id, args);
380+
this.callHostFunction(host_func_id, line, fileString, args);
377381
const func_ref = this.memory.retain(func);
378382
this.closureDeallocator?.track(func, func_ref);
379383
return func_ref;

Runtime/src/types.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export interface ExportedFunctions {
1414
argv: pointer,
1515
argc: number,
1616
callback_func_ref: ref
17-
): void;
17+
): bool;
1818

1919
swjs_free_host_function(host_func_id: number): void;
2020
}
@@ -95,7 +95,7 @@ export interface ImportedFunctions {
9595
exception_payload2_ptr: pointer
9696
): number;
9797
swjs_instanceof(obj_ref: ref, constructor_ref: ref): boolean;
98-
swjs_create_function(host_func_id: number): number;
98+
swjs_create_function(host_func_id: number, line: number, file: ref): number;
9999
swjs_create_typed_array(
100100
constructor_ref: ref,
101101
elementsPtr: pointer,

Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift

+12-6
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ public protocol JSClosureProtocol: JSValueCompatible {
1515
public class JSOneshotClosure: JSObject, JSClosureProtocol {
1616
private var hostFuncRef: JavaScriptHostFuncRef = 0
1717

18-
public init(_ body: @escaping ([JSValue]) -> JSValue) {
18+
public init(_ body: @escaping ([JSValue]) -> JSValue, file: String = #fileID, line: UInt32 = #line) {
1919
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`.
2020
super.init(id: 0)
2121

2222
// 2. Create a new JavaScript function which calls the given Swift function.
2323
hostFuncRef = JavaScriptHostFuncRef(bitPattern: Int32(ObjectIdentifier(self).hashValue))
24-
id = _create_function(hostFuncRef)
24+
id = withExtendedLifetime(JSString(file)) { file in
25+
_create_function(hostFuncRef, line, file.asInternalJSRef())
26+
}
2527

2628
// 3. Retain the given body in static storage by `funcRef`.
2729
JSClosure.sharedClosures[hostFuncRef] = (self, {
@@ -72,13 +74,15 @@ public class JSClosure: JSObject, JSClosureProtocol {
7274
})
7375
}
7476

75-
public init(_ body: @escaping ([JSValue]) -> JSValue) {
77+
public init(_ body: @escaping ([JSValue]) -> JSValue, file: String = #fileID, line: UInt32 = #line) {
7678
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`.
7779
super.init(id: 0)
7880

7981
// 2. Create a new JavaScript function which calls the given Swift function.
8082
hostFuncRef = JavaScriptHostFuncRef(bitPattern: Int32(ObjectIdentifier(self).hashValue))
81-
id = _create_function(hostFuncRef)
83+
id = withExtendedLifetime(JSString(file)) { file in
84+
_create_function(hostFuncRef, line, file.asInternalJSRef())
85+
}
8286

8387
// 3. Retain the given body in static storage by `funcRef`.
8488
Self.sharedClosures[hostFuncRef] = (self, body)
@@ -128,19 +132,21 @@ public class JSClosure: JSObject, JSClosureProtocol {
128132
// │ │ │
129133
// └─────────────────────┴──────────────────────────┘
130134

135+
/// Returns true if the host function has been already released, otherwise false.
131136
@_cdecl("_call_host_function_impl")
132137
func _call_host_function_impl(
133138
_ hostFuncRef: JavaScriptHostFuncRef,
134139
_ argv: UnsafePointer<RawJSValue>, _ argc: Int32,
135140
_ callbackFuncRef: JavaScriptObjectRef
136-
) {
141+
) -> Bool {
137142
guard let (_, hostFunc) = JSClosure.sharedClosures[hostFuncRef] else {
138-
fatalError("The function was already released")
143+
return true
139144
}
140145
let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map(\.jsValue)
141146
let result = hostFunc(arguments)
142147
let callbackFuncRef = JSFunction(id: callbackFuncRef)
143148
_ = callbackFuncRef(result)
149+
return false
144150
}
145151

146152

Sources/JavaScriptKit/Runtime/index.js

+9-5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Sources/JavaScriptKit/Runtime/index.mjs

+9-5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Sources/JavaScriptKit/XcodeSupport.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ import _CJavaScriptKit
9191
_: JavaScriptObjectRef,
9292
_: JavaScriptObjectRef
9393
) -> Bool { fatalError() }
94-
func _create_function(_: JavaScriptHostFuncRef) -> JavaScriptObjectRef { fatalError() }
94+
func _create_function(_: JavaScriptHostFuncRef, _: UInt32, _: JavaScriptObjectRef) -> JavaScriptObjectRef { fatalError() }
9595
func _create_typed_array<T: TypedArrayElement>(
9696
_: JavaScriptObjectRef,
9797
_: UnsafePointer<T>,

Sources/_CJavaScriptKit/_CJavaScriptKit.c

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
#include "_CJavaScriptKit.h"
22
#include <stdlib.h>
3+
#include <stdbool.h>
34

45
#if __wasm32__
56

6-
void _call_host_function_impl(const JavaScriptHostFuncRef host_func_ref,
7+
bool _call_host_function_impl(const JavaScriptHostFuncRef host_func_ref,
78
const RawJSValue *argv, const int argc,
89
const JavaScriptObjectRef callback_func);
910

1011
__attribute__((export_name("swjs_call_host_function")))
11-
void swjs_call_host_function(const JavaScriptHostFuncRef host_func_ref,
12+
bool swjs_call_host_function(const JavaScriptHostFuncRef host_func_ref,
1213
const RawJSValue *argv, const int argc,
1314
const JavaScriptObjectRef callback_func) {
14-
_call_host_function_impl(host_func_ref, argv, argc, callback_func);
15+
return _call_host_function_impl(host_func_ref, argv, argc, callback_func);
1516
}
1617

1718
void _free_host_function_impl(const JavaScriptHostFuncRef host_func_ref);
@@ -36,7 +37,7 @@ void swjs_cleanup_host_function_call(void *argv_buffer) {
3637
/// this and `SwiftRuntime.version` in `./Runtime/src/index.ts`.
3738
__attribute__((export_name("swjs_library_version")))
3839
int swjs_library_version(void) {
39-
return 707;
40+
return 708;
4041
}
4142

4243
int _library_features(void);

Sources/_CJavaScriptKit/include/_CJavaScriptKit.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,13 @@ extern bool _instanceof(const JavaScriptObjectRef obj,
268268
/// See also comments on JSFunction.swift
269269
///
270270
/// @param host_func_id The target Swift side function called by the created thunk function.
271+
/// @param line The line where the function is created. Will be used for diagnostics
272+
/// @param file The file name where the function is created. Will be used for diagnostics
271273
/// @returns A reference to the newly-created JavaScript thunk function
272274
__attribute__((__import_module__("javascript_kit"),
273275
__import_name__("swjs_create_function")))
274-
extern JavaScriptObjectRef _create_function(const JavaScriptHostFuncRef host_func_id);
276+
extern JavaScriptObjectRef _create_function(const JavaScriptHostFuncRef host_func_id,
277+
unsigned int line, JavaScriptObjectRef file);
275278

276279
/// Instantiate a new `TypedArray` object with given elements
277280
/// This is used to provide an efficient way to create `TypedArray`.

0 commit comments

Comments
 (0)