1660465 - Crash [@ ??] with stack space exhaustion on 32-bit

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200820-8cb700c12bd3 (opt build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --baseline-eager --ion-warmup-threshold=0):

evalInWorker(`
  function f60() {
    var s78 = "for (var j = 0; j < 1200; j++) assertEq(g(";
    for (var i55 = 0; i55 < 50000; i55++)
        s78 += i55 + ",";
    s78 += "1), 5001);";
    g = undefined;
    eval(s78);
  }
  f60();
`);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x36fb5760 in ?? ()
#1  0xf6eda0d0 in ?? ()
#2  0x36faf67b in ?? ()
#3  0x56dce0d1 in EnterJit (cx=0x36fef13f, state=..., code=0x36fef010 "\351\025") at js/src/jit/Jit.cpp:106
#4  js::jit::MaybeEnterJit (cx=0xf6e06800, state=...) at js/src/jit/Jit.cpp:197
#5  0x56ea70bc in js::RunScript (cx=<optimized out>, state=...) at js/src/vm/Interpreter.cpp:453
#6  js::ExecuteKernel (cx=0xf6e06800, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=...) at js/src/vm/Interpreter.cpp:856
#7  0x56ebc580 in EvalKernel (cx=0xf6e06800, v=..., evalType=DIRECT_EVAL, caller=..., env=..., pc=0xf660962a "|\001", vp=...) at js/src/builtin/Eval.cpp:360
#8  0x56ebcf98 in js::DirectEval (cx=0xbe9, v=..., vp=...) at js/src/builtin/Eval.cpp:490
#9  0x56d3980c in js::jit::DoCallFallback (cx=<optimized out>, frame=0xf64fe5a8, stub=0xf6ed9208, argc=1, vp=0xf64fe568, res=...) at js/src/jit/BaselineIC.cpp:3004
#10 0x36fb156d in ?? ()
#11 0xf6ed9208 in ?? ()
#12 0x36faf67b in ?? ()
eax	0xf64f76b0	-162564432
ebx	0x61ae021	102424609
ecx	0xbe9	3049
edx	0xc351	50001
esi	0x36fef13f	922677567
edi	0xf6eda0d0	-152198960
ebp	0xf649bb50	4132027216
esp	0xf6440000	4131651584
eip	0x36fb5760	922441568
=> 0x36fb5760:	pushl  0x4(%eax)
   0x36fb5763:	pushl  (%eax)

I've been seeing an increasing amount of stack space exhaustion crashes on 32-bit (also on 64-bit, but much less), but I never was able to reproduce or reduce it reliably until now. This test only reproduces on an optimized non-debug 32-bit build. I couldn't get it to work on 64 bit even with adjusted limits because the maximum function argument limit kicks in at some point. Hope we can fix these, as they are a nuisance in fuzzing.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200821033746-c105f961424c.
Failed to bisect testcase (Start build crashes!):
> Start: ac63c8962183502a4b0ec32222efc67d3841d157 (20191107230801)
> End: 8cb700c12bd3acfdef56ce084c87d64fa4daae03 (20200820093209)
> BuildFlags: BuildFlags(asan=False, tsan=False, debug=False, fuzzing=False, coverage=False, valgrind=False)

Comment 3

[Jens Stutte [:jstutte]]

Hi Steven, can you please (let) triage this?

Comment 4

[Nicolas B. Pierron [:nbp]]

Sounds like we would generate stack overflow while pushing the arguments of g's function call in one of the JITs.
My guess would be that this is not Ion as Ion should reject compiling a function with 50001 arguments.

Matthew, can you look at this issue, to check if this is a CacheIR issue when making a call? Or forward it to the right person.

Comment 5

[Iain Ireland [:iain]]

I took a look at this.

We are overflowing the stack while pushing arguments in the JSOp::Call fallback stub: specifically, in the loop generated here. The caller has pushed 50K arguments onto the stack, and we have to copy them to reverse their order for DoCallFallback.

The most direct way to solve this problem would be to generate an over-recursed check in the fallback code, to check if we have room on the stack before pushing the arguments.

I would not expect this to be possible to recreate on 64-bit, or outside of a worker thread, because the minimum number of arguments to trigger it here is around 48K, and the maximum number of arguments the parser accepts is 64K. If we are exhausting the stack on 64-bit, it's probably happening somewhere else.

This is not a security issue. We are pushing values onto the stack in a loop, so there's no risk of skipping past a guard page.

Comment 6

[Jens Stutte [:jstutte]]

I assume this is not worth uplifting to any ESR version?

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:sdetar, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Steven DeTar [:sdetar]]

Iain, do you have next steps on this bug? Is this a priority to fix?

Comment 9

[Iain Ireland [:iain]]

Attachment: Bug 1660465: Check for over-recursion in call fallback r=jandem

Hopefully this should reduce the number of stack overflow crashes in fuzzing.

The performance impact of adding a check here should be minimal. This only triggers when we hit the call fallback, not for every call.

Comment 10

[Iain Ireland [:iain]]

Attachment: Bug 1660465: Increase shell worker stack size r=jandem

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66666ee5b0a9
Increase shell worker stack size r=jandem

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/66666ee5b0a9

Comment 13

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20201002154327-c25899d7b631.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.