If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gkw, Assigned: dvander)

Tracking

(Blocks: 2 bugs, {assertion, regression, testcase})

Trunk
x86
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 600222 [details]
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.
(Reporter)

Comment 1

6 years ago
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
(Assignee)

Comment 2

6 years ago
Created attachment 601145 [details] [diff] [review]
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.
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+
(Assignee)

Comment 4

6 years ago
Landed with superior comment: http://hg.mozilla.org/projects/ionmonkey/rev/b81a0fdb2627
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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.