1171909 - Crash [@ js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler>::finishPool] with OOM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 98820360ab66 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --enable-arm-simulator --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-eager --arm-sim-icache-checks --arm-hwcap=vfp):

function oomTest(f) {
    var i = 1;
    do {
        try {
            oomAtAllocation(i);
            f();
        } catch (e) {}
        more = resetOOMFailure();
        i++;
    } while(more);
}
oomTest((function(x) { assertEq(x + y + ex, 25); }));



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0868cb92 in js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler>::finishPool (this=0xffff9c88) at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:712
#0  0x0868cb92 in js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler>::finishPool (this=0xffff9c88) at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:712
#1  0x0868cfe8 in js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler>::insertEntryForwards (this=this@entry=0xffff9c88, numInst=numInst@entry=1, numPoolEntries=numPoolEntries@entry=0, inst=inst@entry=0xffff9690 "\004 \235\344 4f\tȖ\377\377\363Tb\b\002", data=data@entry=0x0) at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:545
#2  0x0863d0c7 in allocEntry (numInst=1, numPoolEntries=0, data=0x0, pe=0x0, markAsBranch=false, inst=0xffff9690 "\004 \235\344 4f\tȖ\377\377\363Tb\b\002", this=0xffff9c88) at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:588
#3  putInt (markAsBranch=false, value=3835502596, this=0xffff9c88) at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:610
#4  js::jit::Assembler::writeInst (this=0xffff9a8c, x=3835502596) at js/src/jit/arm/Assembler-arm.cpp:1395
#5  0x0864d274 in as_dtr (c=<optimized out>, addr=..., rt=..., mode=<optimized out>, size=<optimized out>, ls=<optimized out>, this=<optimized out>) at js/src/jit/arm/Assembler-arm.cpp:1710
#6  js::jit::MacroAssemblerARM::ma_dataTransferN (this=this@entry=0xffff9a8c, ls=js::jit::IsLoad, rn=..., rn@entry=..., offset=offset@entry=..., rt=rt@entry=..., mode=mode@entry=js::jit::PostIndex, cc=cc@entry=js::jit::Assembler::Always, IsSigned=true, size=32) at js/src/jit/arm/MacroAssembler-arm.cpp:1268
#7  0x0865096d in ma_dtr (cc=js::jit::Assembler::Always, mode=js::jit::PostIndex, rt=..., offset=..., rn=..., ls=js::jit::IsLoad, this=0xffff9a8c) at js/src/jit/arm/MacroAssembler-arm.cpp:1056
#8  js::jit::MacroAssemblerARM::ma_pop (this=0xffff9a8c, r=...) at js/src/jit/arm/MacroAssembler-arm.cpp:1335
#9  0x0865099d in js::jit::MacroAssemblerARMCompat::popValue (this=this@entry=0xffff9a8c, val=...) at js/src/jit/arm/MacroAssembler-arm.cpp:3545
#10 0x0846db2e in js::jit::BaselineCompiler::emitOutOfLinePostBarrierSlot (this=this@entry=0xffff9a80) at js/src/jit/BaselineCompiler.cpp:497
#11 0x084a2186 in js::jit::BaselineCompiler::compile (this=this@entry=0xffff9a80) at js/src/jit/BaselineCompiler.cpp:107
#12 0x084a3741 in js::jit::BaselineCompile (cx=cx@entry=0xf7a82040, script=0xf614b160, forceDebugInstrumentation=false) at js/src/jit/BaselineJIT.cpp:247
#13 0x084a6031 in CanEnterBaselineJIT (cx=cx@entry=0xf7a82040, script=..., script@entry=..., osrFrame=osrFrame@entry=0x0) at js/src/jit/BaselineJIT.cpp:286
#14 0x084a6407 in js::jit::CanEnterBaselineMethod (cx=cx@entry=0xf7a82040, state=...) at js/src/jit/BaselineJIT.cpp:354
#15 0x08556ce9 in js::jit::CanEnter (cx=cx@entry=0xf7a82040, state=...) at js/src/jit/Ion.cpp:2422
#16 0x082b2b81 in js::RunScript (cx=cx@entry=0xf7a82040, state=...) at js/src/vm/Interpreter.cpp:628
#17 0x082b33d5 in js::Invoke (cx=cx@entry=0xf7a82040, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:722
#18 0x082b4570 in js::Invoke (cx=cx@entry=0xf7a82040, thisv=..., fval=..., argc=argc@entry=0, argv=argv@entry=0xf63ffd88, rval=rval@entry=...) at js/src/vm/Interpreter.cpp:759
#19 0x084b0149 in js::jit::DoCallFallback (cx=0xf7a82040, frame=0xf63ffdb8, stub_=0xf7a21510, argc=0, vp=0xf63ffd78, res=...) at js/src/jit/BaselineIC.cpp:10416
#20 0x0866bb73 in js::jit::Simulator::softwareInterrupt (this=0xf7a81000, instr=0xf7a02e14) at js/src/jit/arm/Simulator-arm.cpp:2171
#21 0x0866bdb6 in js::jit::Simulator::decodeType7 (this=0xf7a81000, instr=0xf7a02e14) at js/src/jit/arm/Simulator-arm.cpp:3270
#22 0x0866a0b5 in js::jit::Simulator::instructionDecode (this=this@entry=0xf7a81000, instr=instr@entry=0xf7a02e14) at js/src/jit/arm/Simulator-arm.cpp:4189
#23 0x0866d92c in execute<false> (this=0xf7a81000) at js/src/jit/arm/Simulator-arm.cpp:4244
#24 js::jit::Simulator::callInternal (this=this@entry=0xf7a81000, entry=entry@entry=0xf7fc8a18 "\360O-\351\004\320M\342\020\212-\355\r\200\240\341h\220\235\345\r\260\240\341t\240\235", <incomplete sequence \345>) at js/src/jit/arm/Simulator-arm.cpp:4332
#25 0x0866ddce in js::jit::Simulator::call (this=0xf7a81000, entry=entry@entry=0xf7fc8a18 "\360O-\351\004\320M\342\020\212-\355\r\200\240\341h\220\235\345\r\260\240\341t\240\235", <incomplete sequence \345>, argument_count=argument_count@entry=8) at js/src/jit/arm/Simulator-arm.cpp:4415
#26 0x08426bb9 in EnterBaseline (cx=cx@entry=0xf7a82040, data=...) at js/src/jit/BaselineJIT.cpp:124
#27 0x0845751d in js::jit::EnterBaselineMethod (cx=cx@entry=0xf7a82040, state=...) at js/src/jit/BaselineJIT.cpp:156
#28 0x082b2d00 in js::RunScript (cx=cx@entry=0xf7a82040, state=...) at js/src/vm/Interpreter.cpp:642
#29 0x082b33d5 in js::Invoke (cx=cx@entry=0xf7a82040, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:722
#30 0x082b4570 in js::Invoke (cx=cx@entry=0xf7a82040, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0xf63ffed0, rval=rval@entry=...) at js/src/vm/Interpreter.cpp:759
#31 0x084b0149 in js::jit::DoCallFallback (cx=0xf7a82040, frame=0xf63fff00, stub_=0xf7a21368, argc=1, vp=0xf63ffec0, res=...) at js/src/jit/BaselineIC.cpp:10416
#32 0x0866bb73 in js::jit::Simulator::softwareInterrupt (this=0xf7a81000, instr=0xf7a02e14) at js/src/jit/arm/Simulator-arm.cpp:2171
#33 0x0866bdb6 in js::jit::Simulator::decodeType7 (this=0xf7a81000, instr=0xf7a02e14) at js/src/jit/arm/Simulator-arm.cpp:3270
#34 0x0866a0b5 in js::jit::Simulator::instructionDecode (this=this@entry=0xf7a81000, instr=instr@entry=0xf7a02e14) at js/src/jit/arm/Simulator-arm.cpp:4189
#35 0x0866d92c in execute<false> (this=0xf7a81000) at js/src/jit/arm/Simulator-arm.cpp:4244
#36 js::jit::Simulator::callInternal (this=this@entry=0xf7a81000, entry=entry@entry=0xf7fc8a18 "\360O-\351\004\320M\342\020\212-\355\r\200\240\341h\220\235\345\r\260\240\341t\240\235", <incomplete sequence \345>) at js/src/jit/arm/Simulator-arm.cpp:4332
#37 0x0866ddce in js::jit::Simulator::call (this=0xf7a81000, entry=entry@entry=0xf7fc8a18 "\360O-\351\004\320M\342\020\212-\355\r\200\240\341h\220\235\345\r\260\240\341t\240\235", <incomplete sequence \345>, argument_count=argument_count@entry=8) at js/src/jit/arm/Simulator-arm.cpp:4415
#38 0x08426bb9 in EnterBaseline (cx=cx@entry=0xf7a82040, data=...) at js/src/jit/BaselineJIT.cpp:124
#39 0x0845751d in js::jit::EnterBaselineMethod (cx=cx@entry=0xf7a82040, state=...) at js/src/jit/BaselineJIT.cpp:156
#40 0x082b2d00 in js::RunScript (cx=cx@entry=0xf7a82040, state=...) at js/src/vm/Interpreter.cpp:642
#41 0x082ba823 in js::ExecuteKernel (cx=cx@entry=0xf7a82040, script=..., script@entry=..., scopeChainArg=..., thisv=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:878
#42 0x082bac4a in js::Execute (cx=cx@entry=0xf7a82040, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:918
#43 0x086f634d in ExecuteScript (cx=cx@entry=0xf7a82040, obj=..., scriptArg=scriptArg@entry=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4188
#44 0x086f6506 in JS_ExecuteScript (cx=cx@entry=0xf7a82040, scriptArg=scriptArg@entry=...) at js/src/jsapi.cpp:4210
#45 0x0806a04c in RunFile (compileOnly=false, file=0xf7ae49e0, filename=0xffffcf81 "min.js", cx=0xf7a82040) at js/src/shell/js.cpp:443
#46 Process (cx=cx@entry=0xf7a82040, filename=0xffffcf81 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:561
#47 0x080c5257 in ProcessArgs (op=0xffffcbe0, cx=<optimized out>) at js/src/shell/js.cpp:5762
#48 Shell (envp=<optimized out>, op=0xffffcbe0, cx=<optimized out>) at js/src/shell/js.cpp:6031
#49 main (argc=7, argv=0xffffcd34, envp=0xffffcd54) at js/src/shell/js.cpp:6367
eax	0x0	0
ebx	0x9663420	157692960
ecx	0x80	128
edx	0x410	1040
esi	0xffff9c88	-25464
edi	0x1	1
ebp	0xffff95f8	4294940152
esp	0xffff95a0	4294940064
eip	0x868cb92 <js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler>::finishPool()+802>
=> 0x868cb92 <js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler>::finishPool()+802>:	mov    %ecx,0x4(%eax)
   0x868cb95 <js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler>::finishPool()+805>:	mov    -0x1c(%ebp),%ecx

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, failed due to error (try manually).

Comment 2

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 29d982aac2a2).

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/6ffa14c65354
user:        Jon Coppeard
date:        Fri May 22 18:52:38 2015 +0100
summary:     Bug 1155618 - Add better support for testing OOM behaviour r=terrence

Not sure if bug 1155618 is the real cause though. Jon, what do you think? The testcase might also be intermittent.

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #3)
That changeset adds oomAtAllocation() which is used in the testcase, so that's just the first revision the testcase will run at.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: bug1171909-pool-oom

I fixed the main problem which was AssemblerBufferWithConstantPools::initWithAllocator() not checking for allocation failure, but I had to make failure to initialize BaselineStackBuilder into an unhandleable OOM crash in BailoutIonToBaseline().  I'm not sure how handle it in this case - ExceptionHandlerBailout() will clear pending exceptions if this fails.  Jan any ideas?

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8620886 [details] [diff] [review]
bug1171909-pool-oom

Sorry for the delay!

(In reply to Jon Coppeard (:jonco) from comment #5)
> but I had to make failure to initialize
> BaselineStackBuilder into an unhandleable OOM crash in
> BailoutIonToBaseline().  I'm not sure how handle it in this case -
> ExceptionHandlerBailout() will clear pending exceptions if this fails.  Jan
> any ideas?

Hm, OOM used to be an uncatchable exception but that changed a while ago. How about we only clear the exception in the overrecursion case, and in the OOM case we leave the OOM exception? I think that'd work correctly, let me know if it doesn't somehow.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: bug1171909-pool-oom v2

Great, that seems to fix things.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8630423 [details] [diff] [review]
bug1171909-pool-oom v2

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

Thanks, r=me with comment below addressed.

::: js/src/jit/Bailouts.cpp
@@ +234,1 @@
>              MOZ_ASSERT(retval == BAILOUT_RETURN_FATAL_ERROR);

One of the two callers of ExceptionHandlerBailout will assert !cx->isExceptionPending() in this case, I think that can fail now.

Thinking about it more, the exception handling code is really complicated, especially when the debugger is enabled, and ExceptionHandlerBailout shouldn't be that common, so how do you feel about putting a MOZ_CRASH after this assert, with a brief comment explaining we crash for now to not complicate the exception handling code more?

That might be the safest fix and if it turns out to become a topcrash (which I doubt), we can try to do something smarter.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/afb1f49458db

Comment 10

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/02953bbdba40

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0386e2d2d05c

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/0386e2d2d05c