Closed Bug 831846 Opened 7 years ago Closed 7 years ago
Compartment mismatch with evalcx and watch
evalcx('').watch("", /()/) crashes js debug shell on m-c changeset ce9cdd801a73 without any CLI arguments with a compartment mismatch message: *** Compartment mismatch 0x101845600 vs. 0x10184d400 Segmentation fault: 11 autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 118999:39885ae3a597 user: Brendan Eich date: Tue Jan 15 18:17:50 2013 -0800 summary: Bug 810525 - unregress DecompileValueGenerator change to handle object literal reference bases (r=jandem).
s-s because this involves a compartment mismatch. Brendan, is bug 810525 a likely regressor?
Bug 830389 is another compartment mismatch (without a testcase) involving js_ValueToSource (though likely unrelated).
(In reply to Andrew McCreight [:mccr8] from comment #2) > Bug 830389 is another compartment mismatch (without a testcase) involving > js_ValueToSource (though likely unrelated). That seems unlikely to be related. Scoobidiver in bug 821733 comment 11 says that crashes "have almost completely stopped since 21.0a1/20130111". autoBisect points to changeset 39885ae3a597 which landed yesterday around noon. Although I won't exactly discount a latent bug appearing occasionally.
Right, crashes with that signature were mostly due to a different print preview issue which has been fixed, but the bug I linked to also involved a compartment mismatch with js_ValueToSource. I've only seen that once, though, so it isn't a common issue.
Assuming sec-critical pending further analysis, feel free to change if this is not the case.
We properly enter sandbox's compartment through the CCW over Object.wrap. We then try to generate an error about the regular expression not being callable in the expression decompiler. The expression decompiler starts playing around with objects in the outer global script creating the compartment violation. This patch makes the expression decompiler be in the compartment of the script's global.
Attachment #703566 - Flags: feedback?(wmccloskey)
No more info to add, comment 6 diagnoses. Hoping this can get fast r+ from a compartment expert (cc'ed gal since he is around). /be
Comment on attachment 703566 [details] [diff] [review] enter some compartments Can you please check that there is no other frame walking path that might have the same problem?
Attachment #703566 - Flags: feedback?(wmccloskey) → feedback+
I think I got all the paths. Anyway, here's a patch with an assertion that will go off next time.
Assigning to benjamin since this has a patch from him awaiting review from gal.
Assignee: general → benjamin
Status: NEW → ASSIGNED
JSBugMon: Cannot process bug: Unknown exception (check manually)
Attachment #704358 - Flags: review?(gal) → review?(wmccloskey)
I invited gal to do an r+. One peer should be enough. Is there a reason for two? Bill seems busy enough for real, also in that he didn't get to the original r? yet. /be
gal only f+ last time. I asked him by email if he wanted to finish, but he should I should switch defer to someone else.
Comment on attachment 704358 [details] [diff] [review] v2 Sorry, this got taken off my list when Andreas added the f+. It looks fine. It does make sense to audit all the FrameIter uses for compartment mismatches, but we should land this patch now.
Attachment #704358 - Flags: review?(wmccloskey) → review+
(In reply to Bill McCloskey (:billm) from comment #14) > It does make sense to audit all the FrameIter uses for compartment > mismatches, but we should land this patch now. File bug 835170 for that.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
JSBugMon: This bug has been automatically verified fixed.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.