Closed Bug 730152 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: gkw, Assigned: dvander)

References

Details

(Keywords: assertion, regression, testcase)

Attachments

(2 files)

Attached file 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.
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)
Summary: IonMonkey: "Assertion failure: hasGlobal()," with verifybarriers and gc → IonMonkey: "Assertion failure: IsMarkedOrAllocated(static_cast<Cell *>(*thingp))," with verifybarriers and gc
Attached patch fixSplinter Review
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.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #601145 - Flags: review?(wmccloskey)
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.
Attachment #601145 - Flags: review?(wmccloskey) → review+
Landed with superior comment: http://hg.mozilla.org/projects/ionmonkey/rev/b81a0fdb2627
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug730152.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.