1304528 - Crash [@ js::CompartmentChecker::fail]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The following testcase crashes on mozilla-central revision 560b2c805bf7 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

y = evalcx('lazy');
oomTest((function g() {
    return {
        m: (function f(z) {
            for (var x in y) {}
        })([])
    };
}))


Backtrace:

0   js-dbg-64-dm-clang-darwin-560b2c805bf7	0x0000000100b2d286 js::CompartmentChecker::fail(JSCompartment*, JSCompartment*) + 54 (jscntxtinlines.h:40)
1   js-dbg-64-dm-clang-darwin-560b2c805bf7	0x0000000100b2d1ea js::CompartmentChecker::check(JSObject*) + 106 (jscntxtinlines.h:59)
2   js-dbg-64-dm-clang-darwin-560b2c805bf7	0x0000000100988da2 js::GetIterator(JSContext*, JS::Handle<JSObject*>, unsigned int, JS::MutableHandle<JSObject*>) + 2722 (jsiter.cpp:940)
3   js-dbg-64-dm-clang-darwin-560b2c805bf7	0x000000010098a107 js::ValueToIterator(JSContext*, unsigned int, JS::Handle<JS::Value>) + 359 (jsiter.cpp:1210)
4   js-dbg-64-dm-clang-darwin-560b2c805bf7	0x0000000100b35b01 Interpret(JSContext*, js::RunState&) + 9953 (RootingAPI.h:717)
/snip

For detailed crash information, see attachment.

Setting s-s because this is a compartment mismatch issue.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/2356dac937b7
user:        Jan de Mooij
date:        Wed Sep 07 16:55:06 2016 +0200
summary:     Bug 1300548 - Fix the iterator cache to not reuse iterators in different compartments. r=jonco

Jan, is bug 1300548 a likely regressor? (also it seems mozilla-aurora might also be affected)

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Note that bug 1290844 involves Promises but this one does not, and they both seem to bisect to different possible regressors, so filing separately.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

The assertSameCompartment was a bit overzealous: when we return false on OOM, the |objp| outparam might be cross-compartment but it's not used so it doesn't matter. 

We could rewrite GetIterator to return JSObject* instead of using an outparam, but since we need to backport this I went with the simpler patch.

Comment 5

[Jan de Mooij [:jandem]]

Not s-s, just a bogus assert.

Comment 6

[Jon Coppeard (:jonco) (PTO until 2nd March)]

Comment on attachment 8793732 [details] [diff] [review]
Patch

Review of attachment 8793732 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2af19b7c731b
Remove an overzealous compartment assert. r=jonco

Comment 8

[Jan de Mooij [:jandem]]

I removed the test because it hit bug 1219128 and I didn't see an easy way to avoid that (I tried gczeal(0) etc) :/

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8793732 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1300548.
[User impact if declined]: Assertion failures with a debug build. Fuzzers may hit this.
[Describe test coverage new/current, TreeHerder]: Many tests exercise this code.
[Risks and why]: Very low. This should only affect debug builds.
[String/UUID change made/needed]: None.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2af19b7c731b

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8793732 [details] [diff] [review]
Patch

Fix a crash. Take it in 51 aurora.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8e2c49498037