Last Comment Bug 732776 - Crash on heap [@ js::mjit::EnterMethodJIT] with mjitChunkLimit
: Crash on heap [@ js::mjit::EnterMethodJIT] with mjitChunkLimit
[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)
: Jason Orendorff [:jorendorff]
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: ---

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

Description User image 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) {}\
arr = new Float64Array(2);\
for(var $, { j}  = 0;;) test(0);\


==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 User image Brian Hackett (:bhackett) 2012-03-05 05:15:01 PST
Should be fixed by the patch in bug 730806.
Comment 2 User image Brian Hackett (:bhackett) 2012-03-05 05:41:46 PST
Oops, meant this for bug 732791.
Comment 3 User image Brian Hackett (:bhackett) 2012-03-05 06:38:09 PST
Created attachment 602880 [details] [diff] [review]

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 User image Brian Hackett (:bhackett) 2012-03-05 12:47:06 PST
Comment 5 User image Brian Hackett (:bhackett) 2012-03-05 13:43:06 PST
Comment on attachment 602880 [details] [diff] [review]

[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 User image Brian Hackett (:bhackett) 2012-03-06 11:10:12 PST
Comment 7 User image Alex Keybl [:akeybl] 2012-03-09 12:22:39 PST
Comment on attachment 602880 [details] [diff] [review]

[Triage Comment]
Low risk test/crash fix. Approved for Aurora 12.
Comment 8 User image Brian Hackett (:bhackett) 2012-03-10 06:36:01 PST
Comment 9 User image Christian Holler (:decoder) 2012-04-02 14:03:38 PDT
JSBugMon: This bug has been automatically verified fixed.
Comment 10 User image 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 User image 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.