Skip to content

Improve JSKit diagnostics for use-after-free of JSClosure #195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ func expectString(_ value: JSValue, file: StaticString = #file, line: UInt = #li
}
}

func expect(_ description: String, _ result: Bool, file: StaticString = #file, line: UInt = #line, column: UInt = #column) throws {
if !result {
throw MessageError(description, file: file, line: line, column: column)
}
}

func expectThrow<T>(_ body: @autoclosure () throws -> T, file: StaticString = #file, line: UInt = #line, column: UInt = #column) throws -> Error {
do {
_ = try body()
Expand Down
11 changes: 11 additions & 0 deletions IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,17 @@ try test("Closure Lifetime") {
// OneshotClosure won't call fatalError even if it's deallocated before `release`
}
#endif

#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
// Check diagnostics of use-after-free
do {
let c1 = JSClosure { $0[0] }
c1.release()
let error = try expectThrow(try evalClosure.throws(c1, JSValue.number(42.0))) as! JSValue
try expect("Error message should contains definition location", error.description.hasSuffix("PrimaryTests/main.swift:247"))
}
#endif

}

try test("Host Function Registration") {
Expand Down
14 changes: 9 additions & 5 deletions Runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export class SwiftRuntime {
private _instance: WebAssembly.Instance | null;
private _memory: Memory | null;
private _closureDeallocator: SwiftClosureDeallocator | null;
private version: number = 707;
private version: number = 708;

private textDecoder = new TextDecoder("utf-8");
private textEncoder = new TextEncoder(); // Only support utf-8
Expand Down Expand Up @@ -68,7 +68,7 @@ export class SwiftRuntime {
return this._closureDeallocator;
}

private callHostFunction(host_func_id: number, args: any[]) {
private callHostFunction(host_func_id: number, line: number, file: string, args: any[]) {
const argc = args.length;
const argv = this.exports.swjs_prepare_host_function_call(argc);
for (let index = 0; index < args.length; index++) {
Expand All @@ -88,12 +88,15 @@ export class SwiftRuntime {
const callback_func_ref = this.memory.retain((result: any) => {
output = result;
});
this.exports.swjs_call_host_function(
const alreadyReleased = this.exports.swjs_call_host_function(
host_func_id,
argv,
argc,
callback_func_ref
);
if (alreadyReleased) {
throw new Error(`The JSClosure has been already released by Swift side. The closure is created at ${file}:${line}`);
}
this.exports.swjs_cleanup_host_function_call(argv);
return output;
}
Expand Down Expand Up @@ -371,9 +374,10 @@ export class SwiftRuntime {
return obj instanceof constructor;
},

swjs_create_function: (host_func_id: number) => {
swjs_create_function: (host_func_id: number, line: number, file: ref) => {
const fileString = this.memory.getObject(file) as string;
const func = (...args: any[]) =>
this.callHostFunction(host_func_id, args);
this.callHostFunction(host_func_id, line, fileString, args);
const func_ref = this.memory.retain(func);
this.closureDeallocator?.track(func, func_ref);
return func_ref;
Expand Down
4 changes: 2 additions & 2 deletions Runtime/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export interface ExportedFunctions {
argv: pointer,
argc: number,
callback_func_ref: ref
): void;
): bool;

swjs_free_host_function(host_func_id: number): void;
}
Expand Down Expand Up @@ -95,7 +95,7 @@ export interface ImportedFunctions {
exception_payload2_ptr: pointer
): number;
swjs_instanceof(obj_ref: ref, constructor_ref: ref): boolean;
swjs_create_function(host_func_id: number): number;
swjs_create_function(host_func_id: number, line: number, file: ref): number;
swjs_create_typed_array(
constructor_ref: ref,
elementsPtr: pointer,
Expand Down
18 changes: 12 additions & 6 deletions Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ public protocol JSClosureProtocol: JSValueCompatible {
public class JSOneshotClosure: JSObject, JSClosureProtocol {
private var hostFuncRef: JavaScriptHostFuncRef = 0

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

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

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

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

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

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

/// Returns true if the host function has been already released, otherwise false.
@_cdecl("_call_host_function_impl")
func _call_host_function_impl(
_ hostFuncRef: JavaScriptHostFuncRef,
_ argv: UnsafePointer<RawJSValue>, _ argc: Int32,
_ callbackFuncRef: JavaScriptObjectRef
) {
) -> Bool {
guard let (_, hostFunc) = JSClosure.sharedClosures[hostFuncRef] else {
fatalError("The function was already released")
return true
}
let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map(\.jsValue)
let result = hostFunc(arguments)
let callbackFuncRef = JSFunction(id: callbackFuncRef)
_ = callbackFuncRef(result)
return false
}


Expand Down
14 changes: 9 additions & 5 deletions Sources/JavaScriptKit/Runtime/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 9 additions & 5 deletions Sources/JavaScriptKit/Runtime/index.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Sources/JavaScriptKit/XcodeSupport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ import _CJavaScriptKit
_: JavaScriptObjectRef,
_: JavaScriptObjectRef
) -> Bool { fatalError() }
func _create_function(_: JavaScriptHostFuncRef) -> JavaScriptObjectRef { fatalError() }
func _create_function(_: JavaScriptHostFuncRef, _: UInt32, _: JavaScriptObjectRef) -> JavaScriptObjectRef { fatalError() }
func _create_typed_array<T: TypedArrayElement>(
_: JavaScriptObjectRef,
_: UnsafePointer<T>,
Expand Down
9 changes: 5 additions & 4 deletions Sources/_CJavaScriptKit/_CJavaScriptKit.c
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
#include "_CJavaScriptKit.h"
#include <stdlib.h>
#include <stdbool.h>

#if __wasm32__

void _call_host_function_impl(const JavaScriptHostFuncRef host_func_ref,
bool _call_host_function_impl(const JavaScriptHostFuncRef host_func_ref,
const RawJSValue *argv, const int argc,
const JavaScriptObjectRef callback_func);

__attribute__((export_name("swjs_call_host_function")))
void swjs_call_host_function(const JavaScriptHostFuncRef host_func_ref,
bool swjs_call_host_function(const JavaScriptHostFuncRef host_func_ref,
const RawJSValue *argv, const int argc,
const JavaScriptObjectRef callback_func) {
_call_host_function_impl(host_func_ref, argv, argc, callback_func);
return _call_host_function_impl(host_func_ref, argv, argc, callback_func);
}

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

int _library_features(void);
Expand Down
5 changes: 4 additions & 1 deletion Sources/_CJavaScriptKit/include/_CJavaScriptKit.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,13 @@ extern bool _instanceof(const JavaScriptObjectRef obj,
/// See also comments on JSFunction.swift
///
/// @param host_func_id The target Swift side function called by the created thunk function.
/// @param line The line where the function is created. Will be used for diagnostics
/// @param file The file name where the function is created. Will be used for diagnostics
/// @returns A reference to the newly-created JavaScript thunk function
__attribute__((__import_module__("javascript_kit"),
__import_name__("swjs_create_function")))
extern JavaScriptObjectRef _create_function(const JavaScriptHostFuncRef host_func_id);
extern JavaScriptObjectRef _create_function(const JavaScriptHostFuncRef host_func_id,
unsigned int line, JavaScriptObjectRef file);

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