Bug 159446 – RELEASE_ASSERT(!thisObject) in ObjCCallbackFunctionImpl::call when calling JSExport ObjC Constructor without operator new

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Joseph Pecoraro]

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
...

[Joseph Pecoraro]

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]

JS exception seems like much better behavior than RELEASE_ASSERT.

[Joseph Pecoraro]

Created attachment 282842 [details]
[PATCH] Proposed Fix

[WebKit Commit Bot]

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]

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 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 on attachment 282842 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 282842

Committed r202846: <http://trac.webkit.org/changeset/202846>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.