Bug 136313 - ASSERTION FAILED: from.isCell() && from.asCell()->JSCell::inherits(std::remove_pointer<To>::type::info()) in JSC::jsCast(JSC::JSValue) [with To = JSC::JSScope*]
Summary: ASSERTION FAILED: from.isCell() && from.asCell()->JSCell::inherits(std::remov...
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2014-08-27 14:47 PDT by Akos Kiss
Modified: 2014-08-27 18:07 PDT (History)
5 users (show)

See Also:

Proposed patch. (1.40 KB, patch)
2014-08-27 14:53 PDT, Akos Kiss
msaboff: review+
Details | Formatted Diff | Diff
Proposed patch, v2 (1.37 KB, patch)
2014-08-27 15:52 PDT, Akos Kiss
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Akos Kiss 2014-08-27 14:47:28 PDT
When running tests on EFL/ARM64 (compiled with gcc), jsc segfaults on scripts containing eval() with "ASSERTION FAILED: from.isCell() && from.asCell()->JSCell::inherits(std::remove_pointer<To>::type::info())". The simplest test case to cause the assertion is:

var c = "";

The backtrace is always as follows:

Program received signal SIGSEGV, Segmentation fault.
0x0000000001098298 in WTFCrash () at /home/akiss/devel/WebKit/Source/WTF/wtf/Assertions.cpp:329
329	    *(int *)(uintptr_t)0xbbadbeef = 0;
(gdb) bt
#0  0x0000000001098298 in WTFCrash ()
    at /home/akiss/devel/WebKit/Source/WTF/wtf/Assertions.cpp:329
#1  0x0000000000aed314 in JSC::jsCast<JSC::JSScope*> (from=...)
    at /home/akiss/devel/WebKit/Source/JavaScriptCore/runtime/JSCell.h:241
#2  0x0000000000ae8270 in JSC::Register::scope (this=0x7fffffe598)
    at /home/akiss/devel/WebKit/Source/JavaScriptCore/runtime/JSScope.h:236
#3  0x0000000000ae3d38 in JSC::ExecState::scope (this=0x7fffffe580)
    at /home/akiss/devel/WebKit/Source/JavaScriptCore/interpreter/CallFrame.h:50
#4  0x0000000000bdd8dc in JSC::eval (callFrame=0x7fffffe550)
    at /home/akiss/devel/WebKit/Source/JavaScriptCore/interpreter/Interpreter.cpp:108
#5  0x0000000000c24fbc in JSC::operationCallEval (exec=0x7fffffe5a0, execCallee=0x7fffffe550)
    at /home/akiss/devel/WebKit/Source/JavaScriptCore/jit/JITOperations.cpp:619
#6  0x0000007fb4ca3cec in ?? ()
#7  0x0000007fb43ff970 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

It seems that for JSC:eval the callFrame parameter is not set up properly: its CallerFrameAndPC component is not filled in. A little bit of analysis follows.

The bytecode for the script is:

[   0] enter             
[   1] mov               loc0, Undefined(@k0)
[   4] resolve_scope     loc1, c(@id0), 1<ThrowIfNotFound|GlobalVar>, 0
[  10] put_to_scope      loc1, c(@id0), String (atomic) (identifier): , ID: 5(@k1), 65537<DoNotThrowIfNotFound|GlobalVar>, <structure>, 26182392
[  17] resolve_scope     loc3, eval(@id1), 0<ThrowIfNotFound|GlobalProperty>, 0
[  23] get_from_scope    loc1, loc3, eval(@id1), 0<ThrowIfNotFound|GlobalProperty>, <structure>, 120
[  31] resolve_scope     loc2, c(@id0), 1<ThrowIfNotFound|GlobalVar>, 0
[  37] get_from_scope    loc2, loc2, c(@id0), 1<ThrowIfNotFound|GlobalVar>, <structure>, 26182392
[  45] call_eval         loc0, loc1, 2, 10    Original; predicting None
[  54] end               loc0

The relevant JIT code compiled for the script:

[  45] call_eval         loc0, loc1, 2, 10    Original; predicting None
		0x7fb4ca3ca4:    sub    sp, fp, #64		// fp already points to the current ExecState, this will be exec for operationCallEval; start making space for execCallee on the stack (however, sp does *not* point to execCallee but to &(execCallee + JSStack::CallerFrameAndPCSize) )
		0x7fb4ca3ca8:    mov    w16, #0x2
		0x7fb4ca3cac:    stur   w16, [sp, #24]		// execCallee[JSStack::ArgumentCount] = 2
		0x7fb4ca3cb0:    movk   w16, #45
		0x7fb4ca3cb4:    stur   w16, [fp, #44]
		0x7fb4ca3cb8:    ldur   x0, [fp, #-16]
		0x7fb4ca3cbc:    stur   x0, [sp, #16]		// execCallee[JSStack::Callee] = loc1
		0x7fb4ca3cc0:    sub    x1, sp, #16		// now, x1 *does* point to the right place, i.e., to execCallee
		0x7fb4ca3cc4:    mov    x0, fp			// x0 points to exec
		0x7fb4ca3cc8:    movk   w16, #46
		0x7fb4ca3ccc:    stur   w16, [fp, #44]
		0x7fb4ca3cd0:    movz   x17, #55504
		0x7fb4ca3cd4:    movk   x17, #397, lsl #16
		0x7fb4ca3cd8:    str    fp, [x17, xzr]
		0x7fb4ca3cdc:    movz   x16, #20176		// 20176 = 0x4ed0
		0x7fb4ca3ce0:    movk   x16, #194, lsl #16	// 194 = 0xc2
		0x7fb4ca3ce4:    movk   x16, #0, lsl #32
		0x7fb4ca3ce8:    blr    x16 			// call JSC::operationCallEval (exec=x0, execCallee=x1)

Then, in operationCallEval:

EncodedJSValue JIT_OPERATION operationCallEval(ExecState* exec, ExecState* execCallee)
    // ...
    // ...
    JSValue result = eval(execCallee);

So, the rest of execCallee is set up "below sp", but CallerFrameAndPC is *not*, because the call to operationCallEval and its prologue is expected to put the return address on top of sp, and the caller frame on top of that, thus making execCallee complete. However, the prologue does not behave as expected (at least on ARM/EFL, and compiled with gcc). The disassembled code starts with:

Dump of assembler code for function JSC::operationCallEval(JSC::ExecState*, JSC::ExecState*):
   0x0000000000c24ed0 <+0>:	stp	x29, x30, [sp,#-64]!
   0x0000000000c24ed4 <+4>:	mov	x29, sp
   0x0000000000c24ed8 <+8>:	str	x0, [x29,#24]
   0x0000000000c24edc <+12>:	str	x1, [x29,#16]
   0x0000000000c24ee0 <+16>:	ldr	x0, [x29,#24]

That is, sp is "hoisted too much" in one step, making space for fp/x29 (the pointer to caller frame) and lr/x30 (the return address), which is good, and for whatever temporary space is needed (which is bad). "stp fp, lr, [sp, #-16]!" would be the expected first step here, it seems. However, this is in the hands of the compiler...

Fortunately, the first parameter of operationCallEval, exec, points to the frame we need, so adding "execCallee->setCallerFrame(static_cast<CallFrame*>(exec));" to operationCallEval might solve the problem. (If I'm right.) The return address is still not set by this approach, however.

This issue seems to be related to https://bugs.webkit.org/show_bug.cgi?id=127777 , which is fixed (with workaround) in https://bugs.webkit.org/show_bug.cgi?id=127898 and in https://bugs.webkit.org/show_bug.cgi?id=127909 , by passing -fno-omit-frame-pointer to gcc.
Comment 1 Akos Kiss 2014-08-27 14:53:12 PDT
Created attachment 237257 [details]
Proposed patch.

Since the proposed patch is not ARM64/EFL/GCC-specific, I tested it on x86_64 as well (but still on EFL and with GCC only). 
On ARM64, that assertion did not occur anymore when running run-javascriptcore-tests.
On x86_64, all jsc tests ran correctly before and after applying the patch, both in debug and release builds.
Comment 2 Michael Saboff 2014-08-27 15:14:33 PDT
If you build with -fno-omit-frame-pointer everywhere, does the crash still happen?
Comment 3 Csaba Osztrogonác 2014-08-27 15:19:16 PDT
(In reply to comment #2)
> If you build with -fno-omit-frame-pointer everywhere, does the crash still happen?

EFL builds with -fno-omit-frame-pointer long time ago ( since http://trac.webkit.org/changeset/163080 ), so the answer must be yes.
Comment 4 Michael Saboff 2014-08-27 15:25:57 PDT
Comment on attachment 237257 [details]
Proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=237257&action=review


> Source/JavaScriptCore/jit/JITOperations.cpp:614
> +    execCallee->setCallerFrame(static_cast<CallFrame*>(exec));

The static_cast is not needed.  CallFrame is a typedef of ExecState.
Comment 5 Akos Kiss 2014-08-27 15:52:39 PDT
Created attachment 237266 [details]
Proposed patch, v2

Static cast removed
Comment 6 Michael Saboff 2014-08-27 15:53:38 PDT
Comment on attachment 237266 [details]
Proposed patch, v2

Comment 7 WebKit Commit Bot 2014-08-27 16:06:17 PDT
Comment on attachment 237266 [details]
Proposed patch, v2

Clearing flags on attachment: 237266

Committed r173031: <http://trac.webkit.org/changeset/173031>
Comment 8 WebKit Commit Bot 2014-08-27 16:06:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Akos Kiss 2014-08-27 18:07:36 PDT
For the records:

On ARM64/iOS/clang, the prologue of JavaScriptCore`operationCallEval starts like this:

JavaScriptCore[0x256a0c]:  stp    x20, x19, [sp, #-32]!
JavaScriptCore[0x256a10]:  stp    fp, lr, [sp, #16]
JavaScriptCore[0x256a14]:  add    fp, sp, #16

The procedure call standard for ARM64 (http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf) says, in Section 5.4.3, that the location of the frame record within a stack frame is not specified, so both the iOS/clang and EFL/gcc prologues seem to be valid.