1137341 - Assertion failure: ptr.found() && &*ptr == &e.front(), at vm/ObjectGroup.cpp

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

// Randomly chosen test: js/src/jit-test/tests/debug/bug1108159.js
schedulegc(this);
startgc(0, "shrinking");
var g = newGlobal();
g.offThreadCompileScript('debugger;', {});
g.runOffThreadScript();

asserts js debug shell on m-c changeset 0a8b3b67715a with --fuzzing-safe --ion-offthread-compile=off --ion-eager at Assertion failure: ptr.found() && &*ptr == &e.front(), at vm/ObjectGroup.cpp.

Configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh ~/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build -R ~/trees/mozilla-central" -r 0a8b3b67715a

autoBisect is running.

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x289ce, 0x0000000100248bb4 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::ObjectGroupCompartment::checkNewTableAfterMovingGC(this=<unavailable>, table=<unavailable>) + 964 at ObjectGroup.cpp:1544, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100248bb4 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::ObjectGroupCompartment::checkNewTableAfterMovingGC(this=<unavailable>, table=<unavailable>) + 964 at ObjectGroup.cpp:1544
    frame #1: 0x00000001007e6c21 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::CheckHashTablesAfterMovingGC(JSRuntime*) [inlined] js::CompartmentsIterT<js::ZonesIter>::operator->(this=0x0000000101d2be00, this=0x00000001028a48b8) const + 15 at ObjectGroup.h:678
    frame #2: 0x00000001007e6c12 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::CheckHashTablesAfterMovingGC(rt=0x000000010201f800) + 114 at jsgc.cpp:7021
    frame #3: 0x00000001007e656d js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::GCRuntime::compactPhase(this=0x000000010201fb30, reason=<unavailable>) + 749 at jsgc.cpp:5525
    frame #4: 0x00000001007e83a2 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::GCRuntime::incrementalCollectSlice(this=0x000000010201fb30, budget=<unavailable>, reason=API) + 1138 at jsgc.cpp:5941
(lldb)

Comment 2

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

Something related to Debugger and/or compacting GC might be at play here, setting s-s to be safe since GC is all over the stack.

Comment 3

[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/26d8f946a53b
user:        Jon Coppeard
date:        Fri Jan 16 14:34:32 2015 +0000
summary:     Bug 650161 - Enable compacting GC on GC_SHRINK collections r=terrence r=glandium

Jon, is compacting GC a likely regressor?

Comment 4

[Jon Coppeard (:jonco)]

Attachment: bug1137341-merge-crash

What's happening here is that GlobalHelperThreadState::finishParseTask() temporarily creates cross-compartment pointers for ObjectGroup prototypes which become same-compartment pointers when MergeCompartments() happens.  However if a GC tries to move the prototype objects before this point then it won't know about the cross-compartment pointers and it will not update them.

The fix is to finish any ongoing GC activity before creating these pointers and assert that we don't trigger any more before calling MergeCompartments().

Comment 5

[Terrence Cole [:terrence]]

Comment on attachment 8570427 [details] [diff] [review]
bug1137341-merge-crash

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

Yikes!

Comment 6

[Jon Coppeard (:jonco)]

Test testcase is hanging on opt builds due to bug 1138390.

Comment 7

[Jon Coppeard (:jonco)]

Comment on attachment 8570427 [details] [diff] [review]
bug1137341-merge-crash

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Hard, since it relies on the timing of GC slices.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No.

Which older supported branches are affected by this flaw?

Aurora is affected.

If not all supported branches, which bug introduced the flaw?

Bug 650161.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

This patch should apply cleanly.

How likely is this patch to cause regressions; how much testing does it need?

Unlikely.

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8570427 [details] [diff] [review]
bug1137341-merge-crash

sec-approval+ for trunk.

We should get a patch nominated and approved for Aurora as well.

Comment 9

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f42cf3d85fab

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f42cf3d85fab

Comment 11

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 12

[Jon Coppeard (:jonco)]

Needinfo'ing myself to request aurora uplift in a week.

Comment 13

[Jon Coppeard (:jonco)]

Comment on attachment 8570427 [details] [diff] [review]
bug1137341-merge-crash

Approval Request Comment
[Feature/regressing bug #]: Bug 650161
[User impact if declined]: Possible security issue.
[Describe test coverage new/current, TreeHerder]: On central for a week.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e2c811c726a1

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for jit bustage in https://hg.mozilla.org/releases/mozilla-aurora/rev/ae81a10f22b6

https://treeherder.mozilla.org/logviewer.html#?job_id=682214&repo=mozilla-aurora


Do we need to get bug 1138390 uplifted to aurora first?

Comment 16

[Jon Coppeard (:jonco)]

(In reply to Wes Kocher (:KWierso) from comment #15)

> Do we need to get bug 1138390 uplifted to aurora first?

Yes, we do.  I requested uplift for both and didn't expect one to get uplifted without the other :)

Comment 17

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cad8615a378c

Comment 18

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed on Fx38