WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
159446
RELEASE_ASSERT(!thisObject) in ObjCCallbackFunctionImpl::call when calling JSExport ObjC Constructor without operator new
https://bugs.webkit.org/show_bug.cgi?id=159446
Summary
RELEASE_ASSERT(!thisObject) in ObjCCallbackFunctionImpl::call when calling JS...
Joseph Pecoraro
Reported
2016-07-05 17:27:03 PDT
Summary: RELEASE_ASSERT(!thisObject) in ObjCCallbackFunctionImpl::call when calling JSExport ObjC Constructor without operator new Test: /* * shell> xcrun clang -framework Foundation -framework JavaScriptCore construct.m */ #import <Foundation/Foundation.h> #import <JavaScriptCore/JavaScriptCore.h> @protocol JSFooExports <JSExport> - (instancetype)initWithName:(NSString *)name; @end @interface JSFoo : NSObject <JSFooExports> - (instancetype)initWithName:(NSString *)name; @end @implementation JSFoo - (instancetype)initWithName:(NSString *)name { self = [super init]; if (!self) return nil; NSLog(@">>> initWithName: %@", name); return self; } @end int main() { JSContext *ctx = [[JSContext alloc] init]; ctx[@"JSFoo"] = [JSFoo class]; [ctx evaluateScript:@"JSFoo()"]; return 0; } Steps to Reproduce: 1. Compile + Run test => RELEASE_ASSERT crash immediately Crash: Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_BREAKPOINT (SIGTRAP) Exception Codes: 0x0000000000000002, 0x0000000000000000 Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010bdf0e6e JSC::ObjCCallbackFunctionImpl::call(JSContext*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 654 (ObjCCallbackFunction.mm:557) 1 com.apple.JavaScriptCore 0x000000010bdf0926 JSC::objCCallbackFunctionCallAsFunction(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 262 (ObjCCallbackFunction.mm:462) 2 com.apple.JavaScriptCore 0x000000010bdf1b7d long long JSC::APICallbackFunction::call<JSC::ObjCCallbackFunction>(JSC::ExecState*) + 573 (APICallbackFunction.h:61) 3 com.apple.JavaScriptCore 0x000000010bdb15b3 JSC::LLInt::setUpCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*) + 595 (LLIntSlowPaths.cpp:1203) 4 com.apple.JavaScriptCore 0x000000010bdb87ce llint_entry + 24803 5 com.apple.JavaScriptCore 0x000000010bdb2508 vmEntryToJavaScript + 299 6 com.apple.JavaScriptCore 0x000000010bc0c2ee JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158 (JITCode.cpp:81) 7 com.apple.JavaScriptCore 0x000000010bb65cb5 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 17125 (Interpreter.cpp:961) 8 com.apple.JavaScriptCore 0x000000010b7d0ea5 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 469 (Completion.cpp:107) 9 com.apple.JavaScriptCore 0x000000010bc61d98 JSEvaluateScript + 456 (NakedPtr.h:54) 10 com.apple.JavaScriptCore 0x000000010bc73639 -[JSContext evaluateScript:withSourceURL:] + 105 (JSContext.mm:102) 11 a.out 0x000000010b56ee9b main + 171 12 libdyld.dylib 0x00007fff918095ad start + 1 ...
Attachments
[PATCH] Proposed Fix
(9.92 KB, patch)
2016-07-05 19:39 PDT
,
Joseph Pecoraro
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1
2016-07-05 17:39:31 PDT
JavaScript Classes disallow calling the constructor without `new`.
> class Foo {}; Foo()
< Exception: TypeError: Cannot call a class constructor Maybe the ObjC bridge should do the same.
Geoffrey Garen
Comment 2
2016-07-05 17:40:41 PDT
JS exception seems like much better behavior than RELEASE_ASSERT.
Joseph Pecoraro
Comment 3
2016-07-05 19:39:39 PDT
Created
attachment 282842
[details]
[PATCH] Proposed Fix
WebKit Commit Bot
Comment 4
2016-07-05 19:40:25 PDT
Attachment 282842
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/tests/testapi.mm:1352: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Joseph Pecoraro
Comment 5
2016-07-05 19:43:19 PDT
The new testapi output looks like:
> testapi[41761:43777520] TEST: "ObjC Constructor without 'new' pre": PASSED > testapi[41761:43777520] TypeError: Cannot call a class constructor without |new| > testapi[41761:43777520] TEST: "ObjC Constructor without 'new' post": PASSED
I went ahead and updated the JavaScript error message for ES6 classes. Other browsers hinted that this was missing 'new', and I think we should do the same: Safari Before: TypeError: Cannot call a class constructor Safari After: TypeError: Cannot call a class constructor without |new| Chrome: TypeError: Class constructor Foo cannot be invoked without 'new'(…) Firefox: TypeError: Cannot call a class constructor without |new|
Mark Lam
Comment 6
2016-07-05 19:46:04 PDT
Comment on
attachment 282842
[details]
[PATCH] Proposed Fix View in context:
https://bugs.webkit.org/attachment.cgi?id=282842&action=review
r=me
> Source/JavaScriptCore/API/tests/testapi.mm:1337 > +
Please remove this empty line.
WebKit Commit Bot
Comment 7
2016-07-05 22:04:40 PDT
Comment on
attachment 282842
[details]
[PATCH] Proposed Fix Clearing flags on attachment: 282842 Committed
r202846
: <
http://trac.webkit.org/changeset/202846
>
WebKit Commit Bot
Comment 8
2016-07-05 22:04:44 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug