1126375 - Assertion failure: expectedFrameSize + JitStackAlignment > frameSize (The frame size is optimal), at js/src/jit/JitFrames.cpp:3050

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 38e4719e71af (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-debug, run with --fuzzing-safe --thread-count=2):

function entryFrame_1(d) {
    assertJitStackInvariants();
}
for (i = 0Xa; i < 40; i++) {
    entryFrame_1();
}


Backtrace:

Program received signal SIGSEGV, Segmentation fault.
js::jit::AssertJitStackInvariants (cx=cx@entry=0x92fc370) at js/src/jit/JitFrames.cpp:3049
3049	                MOZ_RELEASE_ASSERT(expectedFrameSize + JitStackAlignment > frameSize,
#0  js::jit::AssertJitStackInvariants (cx=cx@entry=0x92fc370) at js/src/jit/JitFrames.cpp:3049
#1  0x08092f3f in TestingFunc_assertJitStackInvariants (cx=0x92fc370, argc=0, vp=0xffffc578) at js/src/builtin/TestingFunctions.cpp:1360
#2  0xf762bd49 in ?? ()
#3  0x0931c870 in ?? ()
#4  0xf762573b in ?? ()
#5  0xf762be26 in ?? ()
#6  0x0931cb80 in ?? ()
#7  0xf76258ba in ?? ()
#8  0x081980ca in EnterBaseline (cx=0xf762bb56, cx@entry=0x92fc370, data=...) at js/src/jit/BaselineJIT.cpp:123
#9  0x081af9fb in js::jit::EnterBaselineAtBranch (cx=0x92fc370, fp=0x9369ba0, pc=0x9313cfb "ず") at js/src/jit/BaselineJIT.cpp:210
#10 0x084c19e8 in Interpret (cx=0x92fc370, state=...) at js/src/vm/Interpreter.cpp:1691
#11 0x084c1b96 in js::RunScript (cx=cx@entry=0x92fc370, state=...) at js/src/vm/Interpreter.cpp:448
#12 0x084c1d02 in js::ExecuteKernel (cx=cx@entry=0x92fc370, script=script@entry=..., scopeChainArg=..., thisv=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:657
#13 0x084c1e57 in js::Execute (cx=cx@entry=0x92fc370, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:694
#14 0x083a8149 in ExecuteScript (cx=0x92fc370, cx@entry=0xffffcdb8, obj=..., obj@entry=..., scriptArg=..., rval=0x0) at js/src/jsapi.cpp:4239
#15 0x083a82fd in JS_ExecuteScript (cx=0xffffcdb8, cx@entry=0x92fc370, obj=..., obj@entry=..., scriptArg=scriptArg@entry=...) at js/src/jsapi.cpp:4261
#16 0x08059ce5 in RunFile (compileOnly=false, file=0x9314120, filename=0xffffd32f "min.js", obj=..., cx=0x92fc370) at js/src/shell/js.cpp:453
#17 Process (cx=cx@entry=0x92fc370, obj_=<optimized out>, filename=0xffffd32f "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:586
#18 0x0805cdbe in ProcessArgs (op=0xffffcfcc, obj_=<optimized out>, cx=0x92fc370) at js/src/shell/js.cpp:5514
#19 Shell (op=0xffffcfcc, cx=0x92fc370, envp=<optimized out>) at js/src/shell/js.cpp:5753
#20 main (argc=4, argv=0xffffd164, envp=0xffffd178) at js/src/shell/js.cpp:6093
eax	0x0	0
ebx	0x92d2ff4	153956340
ecx	0xf7e558ac	-135964500
edx	0x0	0
esi	0xf7e54d9c	-135967332
edi	0xffffc4f0	-15120
ebp	0x2	2
esp	0xffffc4d0	4294952144
eip	0x825d111 <js::jit::AssertJitStackInvariants(JSContext*)+673>
=> 0x825d111 <js::jit::AssertJitStackInvariants(JSContext*)+673>:	movl   $0xbea,0x0
   0x825d11b <js::jit::AssertJitStackInvariants(JSContext*)+683>:	call   0x804aab0 <abort@plt>


Likely a shell-only bug.

Comment 1

[Nicolas B. Pierron [:nbp] {away}]

(In reply to Christian Holler (:decoder) from comment #0)
> Likely a shell-only bug.

This is a prelude to SIMD Ion implementation, so no code relies on this assertion yet.
I will investigate it later.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/85f601fa7b46
user:        Nicolas B. Pierron
date:        Mon Jan 26 12:07:58 2015 +0100
summary:     Bug 1112162 part 2 - Add assertion & test case to verify that we keep the same alignment across the rectifier frame. r=bbouvier

This iteration took 518.432 seconds to run.

Comment 3

[Nicolas B. Pierron [:nbp] {away}]

This is a problem with the code, but a problem of the assertion which does not account that x86 has an extra frame pointer pushed on the stack of the rectifier frame.

Comment 4

[Nicolas B. Pierron [:nbp] {away}]

(In reply to Nicolas B. Pierron [:nbp] from comment #3)
> This is a problem with the code, but a problem of the assertion which does

typo: This is *not* a problem …

Comment 5

[Nicolas B. Pierron [:nbp] {away}]

Attachment: Fix assertJitStackInvariant to account for details of x86 rectifier frames.

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8557969 [details] [diff] [review]
Fix assertJitStackInvariant to account for details of x86 rectifier frames.

Review of attachment 8557969 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing it.  So we have now at least three places that implicitly rely on the frame pointer to be pushed onto the stack, on x86, and no static nor dynamic assertion that would fail, would somebody removing the pushing of $ebp.  Any idea how we could fix this?

::: js/src/jit/JitFrames.cpp
@@ +2734,5 @@
>                    "The rectifier frame should keep the alignment");
>  
>                  size_t expectedFrameSize = 0
> +#if defined(JS_CODEGEN_X86)
> +                    + sizeof(void *) /* frame pointer */

Can you please add a comment that it's very specific to x86, as the bailout code relies upon EBP being pushed, in the baseline bailout code? (or something along the same comment as in the generateFrameRectifier function in Trampoline-x86.cpp)

Comment 7

[Nicolas B. Pierron [:nbp] {away}]

(In reply to Benjamin Bouvier [:bbouvier] from comment #6)
> So we have now at least three places that implicitly
> rely on the frame pointer to be pushed onto the stack, on x86, and no static
> nor dynamic assertion that would fail, would somebody removing the pushing
> of $ebp.  Any idea how we could fix this?

If somebody removes this push, then the instruction which is below will fail at runtime, and it will claim that the frame is too small to contain all the values.  Note that we have a low bound and a high bound on the frame size of the rectifier frame.

Comment 8

[Nicolas B. Pierron [:nbp] {away}]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4fcf479bd667

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4fcf479bd667