Open Bug 973683 Opened 6 years ago Updated 5 years ago

Enable CompartmentChecker in Beta and Release

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

People

(Reporter: mccr8, Unassigned)

References

Details

Billm enabled this in Nightly and Aurora, and it has found a bunch of bugs that are potentially dangerous (see bug 821733).  It would be nice to enable it in Beta and Release.

The two barriers I can think of are perf and stability.  I don't recall there being any benchmarks that took a hit when this was enabled, so maybe that is okay.

In terms of stability, these crashes are only ranked 65 on Aurora, so I think it should be okay to enable on Beta, at least on a trial basis.  Some, but not all, of these crashes would turn into other crashes without compartment checking.  We sometimes get spikes in this crash on Nightly, but those are things we want to fix before it gets to Aurora, anyways.  The biggest risk here is that Beta and Release users run different addons that tickle compartment errors.  But if that's the case, I think we probably want to know that and not just let it sit hidden.

The simplest way to enable this is to land a patch on Aurora and Nightly that makes it work everywhere, and then just have it ride the trains.  The drawback is that it could hide any perf regressions among other changes.
Stability-wise I think this is fine and a clear win. I'm concerned about perf though.
> I don't recall there being any benchmarks that took a hit when this was enabled

We could check by disabling it on trunk for a few days, I guess...

Most of the inline JS stuff doesn't do compartment checks, iirc (but should, perhaps?), so this is mostly an issue once you're doing full-on JSAPI method calls.  That might mitigate the perf hit, I guess.
You need to log in before you can comment on or make changes to this bug.