Devirtualise marking

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Oliver Hunt]

Marking currently uses virtual methods, but in the general case these virtual calls are unnecessary and can be elided.

[Oliver Hunt]

Created attachment 34809 [details]
Patch v1

[Maciej Stachowiak]

Comment on attachment 34809 [details]
Patch v1

r=me but consider reformatting the long inline method definitions as suggested.

[Oliver Hunt]

Committed r47267

[Gabor Loki]

It looks like this patch breaks the ARM JIT port at r47269 with ENABLE_YARR=1 ENABLE_YARR_JIT=1 ENABLE_JIT=1 WTF_USE_JSVALUE32=1.

There is a regression at JavaScriptCore/tests/mozilla/ecma/Array/15.4.4.4-1.js .

The GDB backtrace says:
#0  JSC::JSValue::isGetterSetter (this=0x4007ea4c) at JavaScriptCore/runtime/JSCell.h:186
#1  callDefaultValueFunction (exec=0x426a004c, object=0x428b2660, propertyName=@0x2e4320) at JavaScriptCore/runtime/JSObject.cpp:218
#2  JSC::JSObject::defaultValue (this=0x428b2660, exec=0x426a004c, hint=JSC::NoPreference) at JavaScriptCore/runtime/JSObject.cpp:245
#3  JSC::JSObject::toPrimitive (this=0x428b2660, exec=0x426a004c, preferredType=JSC::NoPreference)
    at JavaScriptCore/runtime/JSObject.h:538
#4  JSC::JSValue::toPrimitive (this=0x4007eae8, exec=0x426a004c, preferredType=JSC::NoPreference)
    at JavaScriptCore/runtime/JSCell.h:261
#5  cti_op_to_primitive (args=0x4007eb04) at JavaScriptCore/jit/JITStubs.cpp:2786
#6  ctiTrampoline ()

Do you have any hint or thought how to fix it?

[Gabor Loki]

The bug can be reproduced on ARM with interpreter and jit as well. The situation is the same with debug and with release mode also. Does iPhone run correctly?

[Oliver Hunt]

(In reply to comment #5)
> The bug can be reproduced on ARM with interpreter and jit as well. The
> situation is the same with debug and with release mode also. Does iPhone run
> correctly?

I just did a trivial amount of debugging myself, and forced JSVALUE_32 on x86 and this crash occurred.  Given that's the only substantive difference between ARM and x86 build settings that seems obvious and demonstrates the bug.

[Oliver Hunt]

JSVALUE32 fix landed in r47522