Skip to content

Commit

Permalink
fix(bridge): Catch ObjC Exceptions and throw them to JS
Browse files Browse the repository at this point in the history
* Wrap `ffi_call`s with @try-block
* Throw ObjC exception to JavaScript
(attaching the original native one as `nativeException` property)
* Update libffi with the latest master in an effort to get the
fix from libffi/libffi#418

refs #1029
  • Loading branch information
mbektchiev committed Dec 21, 2018
1 parent de9107f commit 963c50a
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/NativeScript/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ set(SOURCE_FILES
Calling/FFICallPrototype.cpp
Calling/CFunctionWrapper.mm
Calling/FFIFunctionCallback.cpp
Calling/FunctionWrapper.cpp
Calling/FunctionWrapper.mm
GlobalObject.mm
GlobalObject.moduleLoader.mm
inspector/CachedResource.mm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,37 @@

#include "FFICache.h"
#include "FFICall.h"
#include "ObjCTypes.h"
#include <JavaScriptCore/JSObjectRef.h>
#include <JavaScriptCore/JSPromiseDeferred.h>
#include <JavaScriptCore/StrongInlines.h>
#include <JavaScriptCore/interpreter/FrameTracers.h>
#include <JavaScriptCore/interpreter/Interpreter.h>
#include <JavaScriptCore/runtime/Error.h>
#include <dispatch/dispatch.h>
#include <malloc/malloc.h>

#include "FunctionWrapper.h"
#include "Metadata.h"

#import "TNSRuntime.h"

namespace NativeScript {
using namespace JSC;

JSObject* createErrorFromNSException(TNSRuntime* runtime, ExecState* execState, NSException* exception) {
JSObject* error = createError(execState, [[exception reason] UTF8String]);

JSGlobalContextRef context = runtime.globalContext;
JSValueRef wrappedException = [runtime convertObject:exception];
JSStringRef nativeExceptionPropertyName = JSStringCreateWithUTF8CString("nativeException");
JSObjectSetProperty(context, (JSObjectRef)error, nativeExceptionPropertyName,
wrappedException, kJSPropertyAttributeNone, NULL);
JSStringRelease(nativeExceptionPropertyName);

return error;
}

const ClassInfo FunctionWrapper::s_info = { "FunctionWrapper", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(FunctionWrapper) };

void FunctionWrapper::initializeFunctionWrapper(VM& vm, size_t maxParametersCount) {
Expand Down Expand Up @@ -71,16 +89,20 @@ EncodedJSValue JSC_HOST_CALL FunctionWrapper::call(ExecState* execState) {
return JSValue::encode(scope.exception());
}

{
JSLock::DropAllLocks locksDropper(execState);
ffi_call(callee->cif().get(), FFI_FN(invocation.function), invocation._buffer + callee->returnOffset(), reinterpret_cast<void**>(invocation._buffer + callee->argsArrayOffset()));
}
@try {
{
JSLock::DropAllLocks locksDropper(execState);
ffi_call(callee->_cif.get(), FFI_FN(invocation.function), invocation._buffer + callee->_returnOffset, reinterpret_cast<void**>(invocation._buffer + callee->_argsArrayOffset));
}

JSValue result = callee->returnType().read(execState, invocation._buffer + callee->returnOffset(), callee->returnTypeCell().get());
JSValue result = callee->returnType().read(execState, invocation._buffer + callee->returnOffset(), callee->returnTypeCell().get());

callee->postCall(execState, invocation);
callee->postCall(execState, invocation);

return JSValue::encode(result);
return JSValue::encode(result);
} @catch (NSException* exception) {
return throwVMError(execState, scope, createErrorFromNSException([TNSRuntime current], execState, exception));
}
}

JSObject* FunctionWrapper::async(ExecState* execState, JSValue thisValue, const ArgList& arguments) {
Expand Down Expand Up @@ -120,52 +142,61 @@ JSObject* FunctionWrapper::async(ExecState* execState, JSValue thisValue, const
JSPromiseDeferred* deferred = JSPromiseDeferred::create(execState, execState->lexicalGlobalObject());
auto* releasePool = new ReleasePoolBase::Item(releasePoolHolder.relinquish());
__block Strong<FunctionWrapper> callee(execState->vm(), this);
__block TNSRuntime* runtime = [TNSRuntime current];

dispatch_async(dispatch_get_global_queue(0, 0), ^{
JSC::VM& vm = fakeExecState->vm();
auto scope = DECLARE_CATCH_SCOPE(vm);
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
JSC::VM& vm = fakeExecState->vm();
auto scope = DECLARE_CATCH_SCOPE(vm);

ffi_call(call->cif().get(), FFI_FN(invocation->function), invocation->resultBuffer(), reinterpret_cast<void**>(invocation->_buffer + call->argsArrayOffset()));
NSException* nsexception = nullptr;

// Native call is made outside of the VM lock by design.
// For more information see https://github.com/NativeScript/ios-runtime/issues/215 and it's corresponding PR.
// This creates a racing condition which might corrupt the internal state of the VM but
// a fix for it is outside of this PR's scope, so I'm leaving it like it has always been.

JSLockHolder lockHolder(vm);
// we no longer have a valid csaller on the stack, what with being async and all
fakeExecState->setCallerFrame(CallFrame::noCaller());

JSValue result;
{
TopCallFrameSetter frameSetter(vm, fakeExecState);
result = call->returnType().read(fakeExecState, invocation->_buffer + call->returnOffset(), call->returnTypeCell().get());

call->postCall(fakeExecState, *invocation);
}

if (Exception* exception = scope.exception()) {
scope.clearException();
CallData rejectCallData;
CallType rejectCallType = JSC::getCallData(vm, deferred->reject(), rejectCallData);
@try {
ffi_call(callee->_cif.get(), FFI_FN(invocation->function), invocation->resultBuffer(), reinterpret_cast<void**>(invocation->_buffer + callee->_argsArrayOffset));
} @catch (NSException* ex) {
nsexception = ex;
}

MarkedArgumentBuffer rejectArguments;
rejectArguments.append(exception->value());
JSC::call(fakeExecState->lexicalGlobalObject()->globalExec(), deferred->reject(), rejectCallType, rejectCallData, jsUndefined(), rejectArguments);
// Native call is made outside of the VM lock by design.
// For more information see https://github.com/NativeScript/ios-runtime/issues/215 and it's corresponding PR.
// This creates a racing condition which might corrupt the internal state of the VM but
// a fix for it is outside of this PR's scope, so I'm leaving it like it has always been.
JSLockHolder lockHolder(vm);

// we no longer have a valid csaller on the stack, what with being async and all
fakeExecState->setCallerFrame(fakeExecState->lexicalGlobalObject()->globalExec());

JSValue result;
JSValue jsexception = jsNull();
if (Exception* ex = scope.exception()) {
scope.clearException();
jsexception = ex->value();
} else if (nsexception != nullptr) {
jsexception = JSValue(createErrorFromNSException(runtime, fakeExecState, nsexception));
}
}

if (jsexception != jsNull()) {
scope.clearException();
CallData rejectCallData;
CallType rejectCallType = JSC::getCallData(vm, deferred->reject(), rejectCallData);

MarkedArgumentBuffer rejectArguments;
rejectArguments.append(jsexception);
JSC::call(fakeExecState->lexicalGlobalObject()->globalExec(), deferred->reject(), rejectCallType, rejectCallData, jsUndefined(), rejectArguments);
} else {
CallData resolveCallData;
CallType resolveCallType = JSC::getCallData(vm, deferred->resolve(), resolveCallData);
CallData resolveCallData;
CallType resolveCallType = JSC::getCallData(vm, deferred->resolve(), resolveCallData);

MarkedArgumentBuffer resolveArguments;
resolveArguments.append(result);
JSC::call(fakeExecState->lexicalGlobalObject()->globalExec(), deferred->resolve(), resolveCallType, resolveCallData, jsUndefined(), resolveArguments);
MarkedArgumentBuffer resolveArguments;
resolveArguments.append(result);
JSC::call(fakeExecState->lexicalGlobalObject()->globalExec(), deferred->resolve(), resolveCallType, resolveCallData, jsUndefined(), resolveArguments);
}

delete[] fakeCallFrame;
delete releasePool;
});
});

return deferred->promise();
return deferred->promise();
}

} // namespace NativeScript
2 changes: 1 addition & 1 deletion src/libffi
Submodule libffi updated 68 files
+6 −4 .appveyor.yml
+10 −0 .github/issue_template.md
+2 −0 .gitignore
+20 −16 .travis.yml
+270 −0 .travis/ar-lib
+34 −0 .travis/build.sh
+351 −0 .travis/compile
+11 −3 .travis/install.sh
+60 −0 .travis/moxie-sim.exp
+18 −0 .travis/site.exp
+352 −0 LICENSE-BUILDTOOLS
+4 −45 Makefile.am
+237 −230 README.md
+15 −9 configure.ac
+5 −0 configure.host
+4 −4 doc/version.texi
+48 −26 include/ffi.h.in
+3 −1 include/ffi_common.h
+18 −16 m4/ax_append_flag.m4
+21 −8 m4/ax_cc_maxopt.m4
+4 −4 m4/ax_cflags_warn_all.m4
+9 −7 m4/ax_check_compile_flag.m4
+8 −5 m4/ax_compiler_vendor.m4
+5 −5 m4/ax_configure_args.m4
+7 −6 m4/ax_enable_builddir.m4
+99 −61 m4/ax_gcc_archflag.m4
+18 −8 m4/ax_gcc_x86_cpuid.m4
+37 −0 m4/ax_require_defined.m4
+82 −14 msvcc.sh
+8 −31 src/aarch64/ffi.c
+31 −1 src/closures.c
+1 −1 src/frv/ffi.c
+24 −6 src/ia64/ffi.c
+2 −1 src/ia64/ffitarget.h
+6 −1 src/ia64/unix.S
+1 −1 src/metag/ffi.c
+29 −12 src/mips/ffi.c
+7 −12 src/mips/ffitarget.h
+1 −1 src/moxie/eabi.S
+19 −6 src/moxie/ffi.c
+45 −16 src/powerpc/ffi_linux64.c
+481 −0 src/riscv/ffi.c
+69 −0 src/riscv/ffitarget.h
+293 −0 src/riscv/sysv.S
+3 −1 src/types.c
+19 −0 src/x86/ffi.c
+15 −9 src/x86/ffi64.c
+8 −2 src/x86/ffitarget.h
+30 −9 src/x86/ffiw64.c
+88 −2 src/x86/sysv.S
+41 −0 src/x86/unix64.S
+6 −1 src/xtensa/sysv.S
+108 −77 testsuite/Makefile.am
+195 −5 testsuite/lib/libffi.exp
+28 −0 testsuite/libffi.bhaible/Makefile
+78 −0 testsuite/libffi.bhaible/README
+50 −0 testsuite/libffi.bhaible/alignof.h
+58 −0 testsuite/libffi.bhaible/bhaible.exp
+1,745 −0 testsuite/libffi.bhaible/test-call.c
+2,885 −0 testsuite/libffi.bhaible/test-callback.c
+743 −0 testsuite/libffi.bhaible/testcases.c
+46 −0 testsuite/libffi.call/align_stdcall.c
+14 −1 testsuite/libffi.call/call.exp
+3 −1 testsuite/libffi.call/ffitest.h
+1 −0 testsuite/libffi.call/nested_struct10.c
+57 −0 testsuite/libffi.call/struct10.c
+1 −1 testsuite/libffi.call/unwindtest.cc
+1 −1 testsuite/libffi.call/unwindtest_ffi_call.cc
23 changes: 23 additions & 0 deletions tests/TestRunner/app/ApiTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,29 @@ describe(module.id, function () {
done();
});
});

if (isSimulator) {
// Skip on simulator because libffi breaks exception unwinding on iOS Simulator
// see https://github.com/libffi/libffi/issues/418
console.warn("warning: Skipping async ObjC exceptions tests on Simulator device!");
} else {
it("should throw Objective-C exceptions to JavaScript", function (done) {
const value = 333;
const arr = NSArray.arrayWithObject(value);
var promise = arr.objectAtIndex.async(arr, [0])
.then(res => {
expect(res).toBe(value);
expect(NSThread.currentThread.isMainThread).toBe(true);
})
.then(() => arr.objectAtIndex.async(arr, [2]))
.catch(error => {
expect(NSThread.currentThread.isMainThread).toBe(true);
expect(error.toString()).toMatch("index 2 beyond bounds");
expect(error.stack).toEqual("objectAtIndex@[native code]\n[native code]");
done();
});
});
}
});

it("should distinguish between undefined and unavailable variables", function () {
Expand Down
9 changes: 9 additions & 0 deletions tests/TestRunner/app/Infrastructure/simulator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function isSimulator() {
if (NSProcessInfo.processInfo.isOperatingSystemAtLeastVersion({majorVersion: 9, minorVersion: 0, patchVersion: 0})) {
return NSProcessInfo.processInfo.environment.objectForKey("SIMULATOR_DEVICE_NAME") !== null;
} else {
return UIDevice.currentDevice.name.toLowerCase().indexOf("simulator") > -1;
}
}

global.isSimulator = isSimulator();
33 changes: 33 additions & 0 deletions tests/TestRunner/app/MethodCallsTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,39 @@ describe(module.id, function () {
var actual = TNSGetOutput();
expect(actual).toBe('static setDerivedCategoryProperty: calledstatic derivedCategoryProperty called');
});

if (isSimulator) {
// Skip on simulator because libffi breaks exception unwinding on iOS Simulator
// see https://github.com/libffi/libffi/issues/418
console.warn("warning: Skipping ObjC exceptions tests on Simulator device!");
} else {
it('Throw_ObjC_exceptions_to_JavaScript', function () {
try {
NSArray.alloc().init().objectAtIndex(3);
expect(false).toBeTruthy("Should never be reached because objectAtIndex(3) should throw an Objective-C exception.");
} catch(e) {
expect(e.stack).toContain("app/MethodCallsTests.js")
expect(e.nativeException).toBeDefined();
expect(e.nativeException instanceof NSException).toBe(true);
expect(e.nativeException.toString()).toBe(e.nativeException.reason);
expect(e.nativeException.reason).toContain("index 3 beyond bounds");
expect(e.nativeException.name).toBe("NSRangeException");
expect(e.nativeException.callStackSymbols.count).toBeGreaterThan(5);

var nativeCallstack = "";
for (var i=0; i < e.nativeException.callStackSymbols.count; i++) {
nativeCallstack += e.nativeException.callStackSymbols[i] + '\n';
}

expect(nativeCallstack).toContain("CoreFoundation");
expect(nativeCallstack).toContain("NativeScript");
expect(nativeCallstack).toContain("objc_exception_throw");
expect(nativeCallstack).toContain("ffi_call");
expect(nativeCallstack).toContain("vmEntryToJavaScript");
expect(nativeCallstack).toContain("[TNSRuntime executeModule:]");
}
});
}

it('Override: More than one methods with same jsname', function () {

Expand Down
1 change: 1 addition & 0 deletions tests/TestRunner/app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
console.log('Application Start!');

import "./Infrastructure/timers";
import "./Infrastructure/simulator";

global.UNUSED = function (param) {
};
Expand Down

0 comments on commit 963c50a

Please sign in to comment.