Closed
Bug 611432
Opened 14 years ago
Closed 13 years ago
Add assertions to check that compartments are stay the same for JM activations
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: dmandelin, Assigned: dmandelin)
References
Details
(Whiteboard: compartments, softblocker, fixed-in-tracemonkey)
Attachments
(1 file, 2 obsolete files)
1.89 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
Discovered via bug 609244. That bug is caused by the fact that something can change cx->compartment somewhere in a JM activation (i.e., when EnterMethodJIT is on the stack) and then fail to change it back before we come out of the JM activation. We don't have a test case--we just know it's happening in the field based on analysis of bug 609244.
Comment 1•14 years ago
|
||
David, there are some compartment mismatch assertions I've hit only when JM is enabled. See steps to reproduce in bug 610973 and some analysis in bug 610941 (where it basically comes down to the web page picking up String.prototype from the wrong compartment). Could that be related to what you're seeing?
blocking2.0: --- → ?
Updated•14 years ago
|
blocking2.0: ? → betaN+
Assignee | ||
Comment 3•14 years ago
|
||
Morphing this bug to add the assertions, so the violations can be caught and fixed in the normal course of things.
Assignee: general → dmandelin
Summary: Compartments are not getting reset after cross-compartment calls in JM → Add assertions to check that comparments are stay the same for JM activations
Assignee | ||
Comment 4•14 years ago
|
||
This passes jit-tests and jstests, so no current bustage. Note for reviewer: I'm not sure forcing the compartment back to the original value is the right thing to do; maybe we should just assert and not change it.
Attachment #499096 -
Flags: review?(jorendorff)
Assignee | ||
Comment 5•14 years ago
|
||
Whoops, forgot to qref before that last one. My comments about passing tests apply to this patch, not the bad one (although that passes too, because of its badness).
Attachment #499096 -
Attachment is obsolete: true
Attachment #499118 -
Flags: review?(jorendorff)
Attachment #499096 -
Flags: review?(jorendorff)
Assignee | ||
Updated•13 years ago
|
Summary: Add assertions to check that comparments are stay the same for JM activations → Add assertions to check that compartments are stay the same for JM activations
Whiteboard: compartments → compartments,softblocker
Assignee | ||
Updated•13 years ago
|
Attachment #499118 -
Flags: review?(jorendorff) → review?(gal)
Comment 6•13 years ago
|
||
Comment on attachment 499118 [details] [diff] [review] Fixed patch Thats a really terrible name. How about AssertCompartmentUnchanged or something like that. And I also think this should be debug only. Unbalanced compartment setting is a bad bug. We wont be shipping with code that does that.
Attachment #499118 -
Flags: review?(gal) → review+
Assignee | ||
Comment 7•13 years ago
|
||
Can you review this again, since I changed it substantially? I used your name and took out forcing the compartment to be correct, since that is apparently just wrong.
Attachment #499118 -
Attachment is obsolete: true
Attachment #501517 -
Flags: review?(gal)
Updated•13 years ago
|
Attachment #501517 -
Flags: review?(gal) → review+
Assignee | ||
Comment 8•13 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/acab52f46625
Status: NEW → ASSIGNED
Whiteboard: compartments,softblocker → compartments, softblocker, fixed-in-tracemonkey
Comment 9•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/acab52f46625
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•