732776 - Crash on heap [@ js::mjit::EnterMethodJIT] with mjitChunkLimit

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test crashes on mozilla-central revision 343ec916dfd5 (options -m -n):


new Function("\
function test(m) {}\
mjitChunkLimit(10);\
arr = new Float64Array(2);\
for(var $, { j}  = 0;;) test(0);\
")();


Backtrace:

==1057== Invalid read of size 8
==1057==    at 0x403950B: ???
==1057==    by 0x693A07: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1052)
==1057==    by 0x693C71: CheckStackAndEnterMethodJIT(JSContext*, js::StackFrame*, void*, bool) (MethodJIT.cpp:1111)
==1057==    by 0x693D74: js::mjit::JaegerShotAtSafePoint(JSContext*, void*, bool) (MethodJIT.cpp:1129)
==1057==    by 0x4FED3A: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:1713)
==1057==    by 0x4FA601: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:454)
==1057==    by 0x4FB14A: js::ExecuteKernel(JSContext*, JSScript*, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) (jsinterp.cpp:657)
==1057==    by 0x4FB387: js::Execute(JSContext*, JSScript*, JSObject&, JS::Value*) (jsinterp.cpp:698)
==1057==    by 0x43DE62: JS_ExecuteScript (jsapi.cpp:5283)
==1057==    by 0x404F65: Process(JSContext*, JSObject*, char const*, bool) (js.cpp:477)
==1057==    by 0x410814: ProcessArgs(JSContext*, JSObject*, js::cli::OptionParser*) (js.cpp:5208)
==1057==    by 0x410A8C: Shell(JSContext*, js::cli::OptionParser*, char**) (js.cpp:5291)
==1057==  Address 0xfffb800006810138 is not stack'd, malloc'd or (recently) free'd


S-s due to read from corrupted address.

Comment 1

[Brian Hackett [Laid off!]]

Should be fixed by the patch in bug 730806.

Comment 2

[Brian Hackett [Laid off!]]

Oops, meant this for bug 732791.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: patch

Chunked compilation requires that loops be preceded by a JSOP_GOTO or JSOP_NOP, so that chunk boundaries can be set so that chunks don't start right at a JSOP_LOOPHEAD.  Otherwise a fallthrough to the loop head will be a cross chunk edge and will not run the loop prologue to generate loop invariants and load loop-carried registers.  The above property holds for all normal loops, but 'for' loops with destructuring assignments in the initializer could have a JSOP_POP preceding the loop head (chunks can't start at JSOP_POP, due to opcode fusions in the compiler).

Comment 4

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/54226ef0199e

Comment 5

[Brian Hackett [Laid off!]]

Comment on attachment 602880 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: Potential jitcode crashes controlled by use of obscure language feature (destructuring).
Risk to taking this patch (and alternatives if risky): Very low, small tweak to generated bytecode when destructuring assignments are used around 'for' loops.

Comment 6

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/mozilla-central/rev/54226ef0199e

Comment 7

[Alex Keybl [:akeybl]]

Comment on attachment 602880 [details] [diff] [review]
patch

[Triage Comment]
Low risk test/crash fix. Approved for Aurora 12.

Comment 8

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5541c0bb37f0

Comment 9

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 10

[Daniel Veditz [:dveditz]]

Is this a regression from the mjitChunkLimit() feature or is there a way to trigger this without that in the ESR? Assuming we're Ok until I hear otherwise.

Comment 11

[Christian Holler (:decoder)]

Unable to reproduce this even on the original revision, marking in-testsuite-.