730152 - IonMonkey: "Assertion failure: IsMarkedOrAllocated(static_cast<Cell *>(*thingp))," with verifybarriers and gc

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

for (var i = 0; i < 30; i++) {}
for (i in Function("gc(verifybarriers()); yield")()) {}

asserts js debug shell (tested on 64-bit) on IonMonkey changeset 7008b902d362 with -m, --ion and -n at Assertion failure: IsMarkedOrAllocated(static_cast<Cell *>(*thingp)), when the testcase is passed in as a CLI argument.

Comment 1

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

autoBisect shows this is probably related to the following changeset: (or changesets)

Due to skipped revisions, the first bad revision could be any of:
changeset:   88151:8add57bafb0d
user:        David Anderson
date:        Tue Feb 21 12:47:02 2012 -0800
summary:     Implement IonMonkey write barriers (bug 724875, r=jandem,marty).

changeset:   88152:82c6ca0616d0
user:        David Anderson
date:        Tue Feb 21 12:48:48 2012 -0800
summary:     Work around argument-check bailouts not having a scope chain set (bug 724788, r=jandem).

changeset:   88153:6dd34eec6fbe
user:        Nicolas Pierron
date:        Tue Feb 21 13:59:08 2012 -0800
summary:     Fast version of charAt, charCodeAt and fromCharCode (Bug 718853, r=dvander)

changeset:   88348:61980734d3a2
parent:      88152:82c6ca0616d0
parent:      88347:7dcbce54a953
user:        David Anderson
date:        Tue Feb 21 15:08:22 2012 -0800
summary:     Merge from mozilla-central.

changeset:   88349:5a061abdf807
parent:      88348:61980734d3a2
parent:      88153:6dd34eec6fbe
user:        David Anderson
date:        Tue Feb 21 15:08:43 2012 -0800
summary:     Merge.

changeset:   88350:ca97bbcd6b90
user:        David Anderson
date:        Tue Feb 21 15:16:23 2012 -0800
summary:     Fix some merge fallout.

changeset:   88351:4307162c30b6
user:        Nicolas Pierron
date:        Tue Feb 21 15:55:40 2012 -0800
summary:     Fix OSX: Move explicit template instantiation to CPP file (Bug 718853, r=dvander)

changeset:   88352:70cc24cdd404
user:        Nicolas Pierron
date:        Tue Feb 21 18:43:53 2012 -0800
summary:     Fix OS X compilation, explicitly instantiate the function after its definition. (Bug 718853, r=dvander)

changeset:   88353:acb08144edf1
user:        Nicolas Pierron
date:        Tue Feb 21 22:06:47 2012 -0800
summary:     Implement JSOP_INITELEM. (Bug 691340, r=jandem)

changeset:   88354:14d9f14b129e
user:        Jan de Mooij
date:        Wed Feb 22 09:46:50 2012 +0100
summary:     Fix Clang (and probably also MSVC) errors (no bug, r=red)

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

The bug is that we could skip marking a pointer in the initial heap snapshot of IGC, and then try to mark it later. It turns out most of these things should not be marked anyway because they are weak pointers.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 601145 [details] [diff] [review]
fix

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

::: js/src/ion/Ion.cpp
@@ +175,5 @@
> +    // Otherwise, the compartment would be non-empty and kept permanently
> +    // alive. However there is a catch: if we begin incremental GC while the
> +    // compartment is inactive, and complete IGC while it is active, we could
> +    // accidentally collect these critical objects and crash. To prevent this,
> +    // we place both pointers in a read barrier.

Here's how I would write this. Please integrate them as you see fit.

This function marks Ion code objects that must be kept alive if there is any Ion code currently running. These pointers are marked at the start of incremental GC. Entering Ion code in the middle of an incremental GC triggers a read barrier on both these pointers, so they will still be marked in that case.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Landed with superior comment: http://hg.mozilla.org/projects/ionmonkey/rev/b81a0fdb2627

Comment 5

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug730152.js.