Last Comment Bug 732776 - Crash on heap [@ js::mjit::EnterMethodJIT] with mjitChunkLimit
: Crash on heap [@ js::mjit::EnterMethodJIT] with mjitChunkLimit
Status: VERIFIED FIXED
[sg:critical] js-triage-needed
: crash, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: mozilla13
Assigned To: Brian Hackett (:bhackett)
:
Mentors:
Depends on:
Blocks: langfuzz 706914
  Show dependency treegraph
 
Reported: 2012-03-04 03:55 PST by Christian Holler (:decoder)
Modified: 2013-03-11 07:50 PDT (History)
5 users (show)
choller: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
fixed
fixed
fixed
unaffected


Attachments
patch (1.88 KB, patch)
2012-03-05 06:38 PST, Brian Hackett (:bhackett)
dvander: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description Christian Holler (:decoder) 2012-03-04 03:55:51 PST
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 (:bhackett) 2012-03-05 05:15:01 PST
Should be fixed by the patch in bug 730806.
Comment 2 Brian Hackett (:bhackett) 2012-03-05 05:41:46 PST
Oops, meant this for bug 732791.
Comment 3 Brian Hackett (:bhackett) 2012-03-05 06:38:09 PST
Created attachment 602880 [details] [diff] [review]
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 (:bhackett) 2012-03-05 12:47:06 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/54226ef0199e
Comment 5 Brian Hackett (:bhackett) 2012-03-05 13:43:06 PST
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 (:bhackett) 2012-03-06 11:10:12 PST
https://hg.mozilla.org/mozilla-central/rev/54226ef0199e
Comment 7 Alex Keybl [:akeybl] 2012-03-09 12:22:39 PST
Comment on attachment 602880 [details] [diff] [review]
patch

[Triage Comment]
Low risk test/crash fix. Approved for Aurora 12.
Comment 8 Brian Hackett (:bhackett) 2012-03-10 06:36:01 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/5541c0bb37f0
Comment 9 Christian Holler (:decoder) 2012-04-02 14:03:38 PDT
JSBugMon: This bug has been automatically verified fixed.
Comment 10 Daniel Veditz [:dveditz] 2012-04-02 15:48:55 PDT
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) 2013-03-11 07:50:49 PDT
Unable to reproduce this even on the original revision, marking in-testsuite-.

Note You need to log in before you can comment on or make changes to this bug.