Closed
Bug 728342
Opened 12 years ago
Closed 12 years ago
Crash near null with testcase involving mjitChunkLimit
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla13
Tracking | Status | |
---|---|---|
firefox11 | --- | unaffected |
firefox12 | --- | fixed |
firefox13 | --- | fixed |
firefox-esr10 | --- | unaffected |
People
(Reporter: gkw, Assigned: bhackett1024)
References
Details
(Keywords: crash, regression, testcase, Whiteboard: [sg:dos] js-triage-needed)
Attachments
(2 files)
905 bytes,
patch
|
dvander
:
review+
jst
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
10.16 KB,
patch
|
dvander
:
review+
jst
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
mjitChunkLimit(6) try { (function() { function a([], x) { x.x::c } a() })() } catch (e) {} crashes js debug and opt 32-bit shell on m-c changeset 78fde7e54d92 with -m, -a and -n near null. Locking s-s just-in-case even though this seems like a null deref. autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 84929:d0c192e5bd41 user: Brian Hackett date: Wed Jan 18 16:40:18 2012 -0800 summary: Compile large scripts in chunks, bug 706914. r=dvander Opt shell stack trace: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x0000000c 0x00f597e1 in ?? () (gdb) bt #0 0x00f597e1 in ?? () #1 0x01b0af00 in ?? () Previous frame inner to this frame (gdb could not unwind past this frame) (gdb) x/i $pc 0xf597e1: mov 0xc(%esi),%ebx (gdb) x/b $esi 0x0: Cannot access memory at address 0x0 (gdb) x/b $ebx 0xb390d8: 0x02
Assignee | ||
Comment 1•12 years ago
|
||
The closure analysis done for NAME ops could return the current script, which is a problem if the script has not been marked as an outer function as its active call object will not be updated in prologues/epilogues. This was not caused by chunk compilation, but was induced by it since this situation could not come up with whole method compilation (which would abort on the QNAMECONST and other opcodes which make the frontend throw up its hands in resolving local name accesses). I'll put together another patch to make sure we do not do any compilation in scripts which contain any uncompileable opcodes (generalizing bug 726799), which should cut off any more strange new cases which could not have occurred before chunked compilation.
Assignee: general → bhackett1024
Attachment #598886 -
Flags: review?(dvander)
Assignee | ||
Comment 2•12 years ago
|
||
Patch to ensure no parts of scripts are compiled when they contain uncompileable opcodes. This is pretty ugly but I think it's a good fail safe to put in and avoid hard-to-fuzz problems with weird rare opcodes.
Attachment #599141 -
Flags: review?(dvander)
Updated•12 years ago
|
Attachment #598886 -
Flags: review?(dvander) → review+
Updated•12 years ago
|
Attachment #599141 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 3•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0457004daa8c
Assignee | ||
Comment 4•12 years ago
|
||
Comment on attachment 599141 [details] [diff] [review] patch #2 (3b8ad7252ccb) [Approval Request Comment] User impact if declined: Defensive patch against potential crashes caused by compiling part of a script which would not have been compiled at all before bug 706914. Risk to taking this patch (and alternatives if risky): Should be low risk, patch changes a fair bit of code but should not change semantics at all. If this is too much for aurora, patch #1 is more restricted (just fixes a crash).
Attachment #599141 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 5•12 years ago
|
||
Followup fix to compile scripts containing decomposed opcodes. https://hg.mozilla.org/integration/mozilla-inbound/rev/bf3651d2e149
Comment 6•12 years ago
|
||
(In reply to Brian Hackett (:bhackett) from comment #5) > Followup fix to compile scripts containing decomposed opcodes. Will this fix these regressions? https://groups.google.com/forum/#!topic/mozilla.dev.tree-management/CVxkOgplIuE
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Ed Morley [:edmorley] from comment #6) > (In reply to Brian Hackett (:bhackett) from comment #5) > > Followup fix to compile scripts containing decomposed opcodes. > > Will this fix these regressions? > https://groups.google.com/forum/#!topic/mozilla.dev.tree-management/ > CVxkOgplIuE Yeah, it should. The first push was causing us to not compile large portions of many benchmarks.
Comment 8•12 years ago
|
||
(In reply to Brian Hackett (:bhackett) from comment #7) > Yeah, it should. The first push was causing us to not compile large > portions of many benchmarks. Great :-) Graphs-new has since confirmed this, eg: http://graphs-new.mozilla.org/graph.html#tests=[[74,131,15]]&sel=1329852891000,1330025691000&displayrange=7&datatype=running and http://graphs-new.mozilla.org/graph.html#tests=[[77,131,15]]&sel=1329852891000,1330025691000&displayrange=7&datatype=running
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0457004daa8c https://hg.mozilla.org/mozilla-central/rev/bf3651d2e149
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Comment 10•12 years ago
|
||
Comment on attachment 599141 [details] [diff] [review] patch #2 (3b8ad7252ccb) Drivers decided to take patch 1 for aurora here. Minusing this one, and I'll approve the first patch.
Attachment #599141 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Comment 11•12 years ago
|
||
Comment on attachment 598886 [details] [diff] [review] patch #1 [Triage Comment]
Attachment #598886 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 12•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/74efe408346a
Assignee | ||
Updated•12 years ago
|
Target Milestone: mozilla13 → mozilla12
Updated•12 years ago
|
status-firefox12:
--- → fixed
Target Milestone: mozilla12 → mozilla13
Updated•12 years ago
|
status-firefox-esr10:
--- → unaffected
status-firefox11:
--- → unaffected
status-firefox13:
--- → fixed
Updated•12 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•