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

VERIFIED FIXED in Firefox 12

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: decoder, Assigned: bhackett)

Tracking

(Blocks: 1 bug, {crash, regression, testcase})

Trunk
mozilla13
x86_64
Linux
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox11 unaffected, firefox12+ fixed, firefox13 fixed, firefox14 fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [sg:critical] js-triage-needed, crash signature)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Should be fixed by the patch in bug 730806.
Depends on: 730806
(Assignee)

Comment 2

5 years ago
Oops, meant this for bug 732791.
No longer depends on: 730806
(Assignee)

Comment 3

5 years ago
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).
Assignee: general → bhackett1024
Attachment #602880 - Flags: review?(dvander)
Attachment #602880 - Flags: review?(dvander) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/54226ef0199e
(Assignee)

Comment 5

5 years ago
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?
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/54226ef0199e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 7

5 years ago
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+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/5541c0bb37f0
(Reporter)

Comment 9

5 years ago
JSBugMon: This bug has been automatically verified fixed.
(Reporter)

Updated

5 years ago
Status: RESOLVED → VERIFIED
status-firefox12: --- → affected
status-firefox13: --- → fixed
status-firefox14: --- → fixed
tracking-firefox12: --- → +
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.
Blocks: 706914
status-firefox-esr10: --- → unaffected
status-firefox12: affected → fixed
Keywords: regression
status-firefox11: --- → unaffected
Group: core-security
(Reporter)

Comment 11

4 years ago
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.