Closed
Bug 466206
Opened 16 years ago
Closed 16 years ago
Crash [@ js_Interpret] : function variables may be unrooted
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
People
(Reporter: jorendorff, Assigned: igor)
References
Details
(4 keywords, Whiteboard: [sg:critical?])
Crash Data
Attachments
(2 files, 1 obsolete file)
1009 bytes,
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
2.45 KB,
text/plain
|
Details |
/* This crashes trying to access x.y. If I understand correctly this is because the Call object's slot for x isn't populated. Note that if the generator yields, js_PutCallObject is called and then there is no crash. */ var f; function g() { var x = {}; f = function () { x.y; }; if (0) yield; } try { g().next(); } catch (e) {} gc(); f();
Reporter | ||
Comment 1•16 years ago
|
||
Credit: nth16sd found this one using jsfunfuzz.
Assignee | ||
Updated•16 years ago
|
Assignee: general → igor
Reporter | ||
Comment 2•16 years ago
|
||
Correction. js_PutCallObject is not called if you change the "if (0)" to "if (1)". I think it's still at least potentially wrong in that case, and I think the generator frame should go away at gc(). I don't know why it doesn't crash. igor: brendan mentioned SendToGenerator is probably the right place for the call. Because yield previously couldn't fail, this might be nontrivial.
Comment 3•16 years ago
|
||
(In reply to comment #1) > Credit: nth16sd found this one using jsfunfuzz. " Testcase from Gary " would sound better. :) Thanks Jason for helping to reduce!
Severity: normal → critical
Summary: js_PutCallObject needed when walking off end of generator → Crash [@ js_Interpret] : js_PutCallObject needed when walking off end of generator
Comment 4•16 years ago
|
||
The testcase in comment #0 crashes near null at js_Interpret in trunk with or without "-j", and this bug also seems capable of morphing to crash near potentially exploitable areas.
Assignee | ||
Comment 5•16 years ago
|
||
(In reply to comment #2) > I think the generator frame should go away at gc(). For the generator the frame is a part of the generator object and fp->slots points into the memory allocated together with the generator. Thus without js_PutCallObject any live reference to a call object will root the corresponding generator and so the frame. See args_or_call_trace, http://hg.mozilla.org/mozilla-central/file/cf10a03eefe5/js/src/jsfun.cpp#l544 . This is why yielding generator is not problematic in the example - the call object will root its frame. The problem is that js_Interpret sets fp->regs to null on exit and js_TraceFrame, http://hg.mozilla.org/mozilla-central/file/cf10a03eefe5/js/src/jsgc.cpp#l2781 , does not trace the argument and var slots when !fp->regs. This is bad even without the generator since it means that during js_PutArgsObject the arguments and variables are unrooted. Since the later can trigger the GC when adding properties during shape generation, we have a hazard for normal JS code.
Assignee | ||
Comment 6•16 years ago
|
||
The fix always roots the fixed portion of fp->slots.
Attachment #349966 -
Flags: review?(brendan)
Updated•16 years ago
|
Attachment #349966 -
Flags: review?(brendan)
Attachment #349966 -
Flags: review+
Attachment #349966 -
Flags: approval1.9.1?
Attachment #349966 -
Flags: approval1.8.1.next?
Assignee | ||
Comment 7•16 years ago
|
||
The bug is a regression due to bug 441686 and exists only starting from 1.9.1.
Blocks: 441686
Assignee | ||
Updated•16 years ago
|
Attachment #349966 -
Flags: approval1.8.1.next?
Assignee | ||
Comment 8•16 years ago
|
||
I am changing the title to better reflect the reality.
Summary: Crash [@ js_Interpret] : js_PutCallObject needed when walking off end of generator → Crash [@ js_Interpret] : function variables may be unrooted
Comment 9•16 years ago
|
||
Comment on attachment 349966 [details] [diff] [review] v1 a191=beltzner
Attachment #349966 -
Flags: approval1.9.1? → approval1.9.1+
Assignee | ||
Comment 10•16 years ago
|
||
Attachment #349966 -
Attachment is obsolete: true
Attachment #350299 -
Flags: review+
Comment 11•16 years ago
|
||
Pushed changeset ab646b8253b0 to mozilla-central.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 13•16 years ago
|
||
Updated•16 years ago
|
Flags: in-testsuite+
Flags: in-litmus-
Updated•16 years ago
|
Flags: blocking1.9.1?
Updated•16 years ago
|
Whiteboard: [sg:critical?]
Assignee | ||
Comment 15•16 years ago
|
||
The patch is in 1.9.1 repository as it was pushed to mozilla-central before forking mozilla-1.9.1 repo: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ab646b8253b0
Keywords: fixed1.9.1
Updated•15 years ago
|
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.13?
Updated•15 years ago
|
Flags: wanted1.9.1.x+
Flags: blocking1.9.0.13?
Flags: blocking1.9.0.13+
Comment 17•15 years ago
|
||
I'm not sure why I marked this blocking given comment 7. That still holds true, doesn't it? We're not likely to take bug 441686 on the 1.9.0 branch. Maybe I got confused by the dupe which was filed using FF3.0.4, but if I would have read more closely I'd have seen that it affected "3.1" but not 3. Please confirm that this bug does not affect 1.9.0 and that it's OK to unhide the bug.
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.13?
Flags: blocking1.9.0.13+
Keywords: regression
Updated•15 years ago
|
Group: core-security
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x-
Flags: blocking1.9.0.14?
Comment 18•15 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/c064d7cd3ab3 /cvsroot/mozilla/js/tests/js1_7/geniter/regress-466206.js,v <-- regress-466206.js initial revision: 1.1
Updated•13 years ago
|
Crash Signature: [@ js_Interpret]
You need to log in
before you can comment on or make changes to this bug.
Description
•