1370922 - Assertion failure: fallibleScope_ ([OOM] Cannot allocate a new chunk in an infallible scope.), at js/src/ds/LifoAlloc.cpp:105

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 5801aa478de1 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

function f(y) 
  Math.min(
    0, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8,
    9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7,
    8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6,
    7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5,
    6, 7, 8, 9);
function g() f({});
x = 0 
for (var j = 0; j < 3000; j++) 
   g([j])



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000a8565f in js::LifoAlloc::getOrCreateChunk (this=this@entry=0x7ffff4344d00, n=n@entry=176) at js/src/ds/LifoAlloc.cpp:105
#0  0x0000000000a8565f in js::LifoAlloc::getOrCreateChunk (this=this@entry=0x7ffff4344d00, n=n@entry=176) at js/src/ds/LifoAlloc.cpp:105
#1  0x000000000060fc2b in js::LifoAlloc::allocImpl (this=0x7ffff4344d00, n=176) at js/src/ds/LifoAlloc.h:225
#2  0x00000000006811da in js::LifoAlloc::allocInfallible (this=<optimized out>, n=<optimized out>) at js/src/ds/LifoAlloc.h:291
#3  0x00000000007b1c3f in js::jit::TempAllocator::allocateInfallible (bytes=176, this=<optimized out>) at js/src/jit/JitAllocPolicy.h:44
#4  js::jit::TempObject::operator new (alloc=..., nbytes=176) at js/src/jit/JitAllocPolicy.h:162
#5  js::jit::MInstruction::operator new (alloc=..., nbytes=176) at js/src/jit/MIR.h:1121
#6  js::jit::MMinMax::New<js::jit::MMinMax*&, js::jit::MDefinition*&, js::jit::MIRType&, bool&> (alloc=...) at js/src/jit/MIR.h:6321
#7  js::jit::IonBuilder::inlineMathMinMax (this=this@entry=0x7fffffffadc0, callInfo=..., max=max@entry=false) at js/src/jit/MCallOptimize.cpp:1403
#8  0x00000000007b294b in js::jit::IonBuilder::inlineNativeCall (this=this@entry=0x7fffffffadc0, callInfo=..., target=0x7ffff46acd40) at js/src/jit/MCallOptimize.cpp:143
#9  0x00000000006f100e in js::jit::IonBuilder::inlineSingleCall (this=this@entry=0x7fffffffadc0, callInfo=..., targetArg=targetArg@entry=0x7ffff46acd40) at js/src/jit/IonBuilder.cpp:4220
#10 0x00000000006f286b in js::jit::IonBuilder::inlineCallsite (this=this@entry=0x7fffffffadc0, targets=..., callInfo=...) at js/src/jit/IonBuilder.cpp:4282
#11 0x00000000006f2d0e in js::jit::IonBuilder::jsop_call (this=this@entry=0x7fffffffadc0, argc=99, constructing=<optimized out>, ignoresReturnValue=<optimized out>) at js/src/jit/IonBuilder.cpp:5281
#12 0x00000000006f828b in js::jit::IonBuilder::inspectOpcode (this=this@entry=0x7fffffffadc0, op=op@entry=JSOP_CALL) at js/src/jit/IonBuilder.cpp:2032
#13 0x00000000006f9ad2 in js::jit::IonBuilder::visitBlock (this=this@entry=0x7fffffffadc0, cfgblock=cfgblock@entry=0x7ffff435e188, mblock=<optimized out>) at js/src/jit/IonBuilder.cpp:1535
#14 0x00000000006eeed6 in js::jit::IonBuilder::traverseBytecode (this=this@entry=0x7fffffffadc0) at js/src/jit/IonBuilder.cpp:1456
#15 0x00000000006f0409 in js::jit::IonBuilder::buildInline (this=this@entry=0x7fffffffadc0, callerBuilder=callerBuilder@entry=0x7fffffffb890, callerResumePoint=callerResumePoint@entry=0x7ffff69b08c8, callInfo=...) at js/src/jit/IonBuilder.cpp:1008
[...]
#31 0x00000000006efb7e in js::jit::IonBuilder::build (this=this@entry=0x7ffff69ab270) at js/src/jit/IonBuilder.cpp:846
#32 0x0000000000433339 in js::jit::IonCompile (cx=cx@entry=0x7ffff6924000, script=<optimized out>, baselineFrame=baselineFrame@entry=0x7fffffffc668, osrPc=osrPc@entry=0x7ffff420e5ae "ず", recompile=<optimized out>, optimizationLevel=<optimized out>) at js/src/jit/Ion.cpp:2176
#33 0x000000000070326b in js::jit::Compile (cx=cx@entry=0x7ffff6924000, script=script@entry=..., osrFrame=osrFrame@entry=0x7fffffffc668, osrPc=osrPc@entry=0x7ffff420e5ae "ず", forceRecompile=<optimized out>) at js/src/jit/Ion.cpp:2440
#34 0x0000000000703bab in BaselineCanEnterAtBranch (pc=0x7ffff420e5ae "ず", osrFrame=0x7fffffffc668, script=..., cx=0x7ffff6924000) at js/src/jit/Ion.cpp:2631
#35 js::jit::IonCompileScriptForBaseline (cx=cx@entry=0x7ffff6924000, frame=frame@entry=0x7fffffffc668, pc=pc@entry=0x7ffff420e5ae "ず") at js/src/jit/Ion.cpp:2689
#36 0x00000000005ef716 in js::jit::DoWarmUpCounterFallbackOSR (cx=0x7ffff6924000, frame=0x7fffffffc668, stub=0x7ffff4349370, infoPtr=0x7fffffffc640) at js/src/jit/BaselineIC.cpp:145
#37 0x00001f107f91c915 in ?? ()
[...]
#48 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x8000	32768
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffa5b0	140737488332208
rsp	0x7fffffffa4f0	140737488332016
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff69bb000	140737330786304
r13	0x7ffff4344d00	140737290456320
r14	0xb0	176
r15	0x0	0
rip	0xa8565f <js::LifoAlloc::getOrCreateChunk(unsigned long)+831>
=> 0xa8565f <js::LifoAlloc::getOrCreateChunk(unsigned long)+831>:	movl   $0x0,0x0
   0xa8566a <js::LifoAlloc::getOrCreateChunk(unsigned long)+842>:	ud2    



Not sure if this is already on file somewhere else but it keeps popping up here so filing now to make sure we have it covered.

Comment 1

[Fuzzing Team]

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/230e0c324669
user:        Jan de Mooij
date:        Sat Mar 11 10:21:03 2017 +0100
summary:     Bug 1346191 - Change Ion warmup threshold for small functions to be the same as the one for other functions. r=h4writer

This iteration took 268.780 seconds to run.

Comment 2

[Nicolas B. Pierron [:nbp]]

This assertion is often solved by adding an ensureBallast call (such as [1]) in a loop which occurred (likely) before the reported crash.

[1] http://searchfox.org/mozilla-central/source/js/src/jit/IonBuilder.cpp#1057-1058

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1370922 - [Ion] Handle allocation failure when generating Math.min.

Review commit: https://reviewboard.mozilla.org/r/153860/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153860/

Comment 4

[Ted Campbell [:tcampbell]]

When Math.min/max takes too many arguments, we overflow the LifoAlloc ballast. Add additional ensureBallast during construction.

Comment 5

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8882751 [details]
Bug 1370922 - [Ion] Handle allocation failure when generating Math.min.

https://reviewboard.mozilla.org/r/153860/#review159024

Thanks :)

::: js/src/jit-test/tests/ion/bug1370922.js:1
(Diff revision 1)
> +function f(y)
> +      Math.min(0, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,

note: unbraced function is a SpiderMonkey feature.

::: js/src/jit-test/tests/ion/bug1370922.js:23
(Diff revision 1)
> +for (var j = 0; j < 3000; j++)
> +   g([j])

nit: Use inIon() condition to end the loop once we did one loop cycle under Ion.  Make sure that it still reproduce. (maybe return inIon() as the result of function f.)

::: js/src/jit/MCallOptimize.cpp:1476
(Diff revision 1)
>      current->add(last);
>  
>      for (unsigned i = 2; i < cases.length(); i++) {
> +        if (!alloc().ensureBallast())
> +            return abort(AbortReason::Alloc);
>          MMinMax* ins = MMinMax::New(alloc(), last, cases[i], returnType, max);

nit: Ok, this patch is fine, but when there is only a single allocation, we should prefer to make use of a fallible allocator.

Thus:
  MMinMax* ins = MMinMax::New(alloc().fallible(), …);
  if (!ins)
      …;

This fallible allocator will basically allow us to bump at the same time as the allocation.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/891d359d064d
[Ion] Handle allocation failure when generating Math.min. r=nbp

Comment 7

[Ted Campbell [:tcampbell]]

Ah, helpful. Fixed the nits.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/891d359d064d

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 10

[Ted Campbell [:tcampbell]]

Attachment: bug1370922.patch

Final patch with nits fixed. Forwarding review.

Comment 11

[Ted Campbell [:tcampbell]]

Comment on attachment 8885370 [details] [diff] [review]
bug1370922.patch

Approval Request Comment
[Feature/Bug causing the regression]:
Original problem in Bug 936234, but not exposed until Bug 1346191 changed compile parameters.
[User impact if declined]:
Wasted fuzzer cycles. Rare crash in javascript code using many arguments to Math.min/max.
[Is this code covered by automated tests?]:
Yes.
[Has the fix been verified in Nightly?]:
Yes.
[Needs manual test from QE? If yes, steps to reproduce]: 
No.
[List of other uplifts needed for the feature/fix]:
Standalone. Applies cleanly to beta and passes tests.
[Is the change risky?]:
Low risk.
[Why is the change risky/not risky?]:
We are adding additional allocation-failure checks. Risk is general typos and silly mistakes but it has been on nightly for a week.
[String changes made/needed]:
No.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8885370 [details] [diff] [review]
bug1370922.patch

Crash fix, let's uplift this for beta 9.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/363c30132703

Comment 14

[Andrei Vaida [:avaida]]

(In reply to Ted Campbell [:tcampbell] from comment #11)
> [Is this code covered by automated tests?]:
> Yes.
> [Has the fix been verified in Nightly?]:
> Yes.
> [Needs manual test from QE? If yes, steps to reproduce]: 
> No.

Setting qe-verify- based on Ted's assessment on manual testing needs and the fact that this fix has automated coverage.