Last Comment Bug 631377 - Add compartment asserts to jsdbgapi.cpp
: Add compartment asserts to jsdbgapi.cpp
Status: RESOLVED FIXED
[fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Steve Fink [:sfink] [:s:]
: jsd
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 636907
  Show dependency treegraph
 
Reported: 2011-02-03 14:56 PST by Steve Fink [:sfink] [:s:]
Modified: 2011-07-08 00:24 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Conservative set of compartment asserts for jsdbgapi.cpp (4.82 KB, patch)
2011-02-03 14:59 PST, Steve Fink [:sfink] [:s:]
gal: review+
Details | Diff | Splinter Review

Description Steve Fink [:sfink] [:s:] 2011-02-03 14:56:20 PST
Forked from 630471.

jsdbgapi.cpp is used by JSD as well as XPConnect and several other places. It probably ought to have the same compartment check asserts that jsapi.cpp does.

I'm still not really sure what things need to check, though. I'm trying to use a metric of "can run JS code or set a pending exception" as the rule, but it's not always easy to determine.
Comment 1 Steve Fink [:sfink] [:s:] 2011-02-03 14:59:05 PST
Created attachment 509600 [details] [diff] [review]
Conservative set of compartment asserts for jsdbgapi.cpp

Here's a fairly conservative set, where in auditing the code I could see code getting run or exceptions being thrown. Even here, though, I didn't *really* find problems with 100% of these. For example, some of the watch stuff hinges on whether this:

        shape = wp->object->changeProperty(cx, wprop, 0, wprop->attributes(),
                                           wprop->getter(), wp->setter);

is allowed to cross compartments. I said no.
Comment 2 Steve Fink [:sfink] [:s:] 2011-02-08 14:12:33 PST
Comment on attachment 509600 [details] [diff] [review]
Conservative set of compartment asserts for jsdbgapi.cpp

This set of asserts hasn't had any false alarms for me for a while, so asking for review. I don't feel any great need to get this in, so I'll let someone else request blocking if they want to. Otherwise, feel free to ignore until post FF4.
Comment 3 Steve Fink [:sfink] [:s:] 2011-03-03 10:54:17 PST
http://hg.mozilla.org/tracemonkey/rev/aef1f7b0af3e
Comment 4 Steve Fink [:sfink] [:s:] 2011-04-28 13:59:26 PDT
http://hg.mozilla.org/mozilla-central/rev/aef1f7b0af3e

Note You need to log in before you can comment on or make changes to this bug.