Closed Bug 732776 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla13
Tracking Status
firefox11 --- unaffected
firefox12 + fixed
firefox13 --- fixed
firefox14 --- fixed
firefox-esr10 --- unaffected

People

(Reporter: decoder, Assigned: bhackett1024)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase, Whiteboard: [sg:critical] js-triage-needed)

Crash Data

Attachments

(1 file)

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.
Should be fixed by the patch in bug 730806.
Depends on: 730806
Oops, meant this for bug 732791.
No longer depends on: 730806
Attached patch patchSplinter 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).
Assignee: general → bhackett1024
Attachment #602880 - Flags: review?(dvander)
Attachment #602880 - Flags: review?(dvander) → review+
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.
Attachment #602880 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/54226ef0199e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 602880 [details] [diff] [review]
patch

[Triage Comment]
Low risk test/crash fix. Approved for Aurora 12.
Attachment #602880 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
Whiteboard: js-triage-needed → [sg:critical] js-triage-needed
Target Milestone: --- → mozilla13
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.
Group: core-security
Unable to reproduce this even on the original revision, marking in-testsuite-.
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.