939476 - "Assertion failure: phases[phase].parent == parent" with gcparam('markStackLimit')

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi
2. Load the testcase

Assertion failure: phases[phase].parent == parent, at js/src/gc/Statistics.cpp:629

This assertion was added in:
changeset:   http://hg.mozilla.org/mozilla-central/rev/14893aa56835
user:        Bill McCloskey
date:        Tue Dec 04 18:29:45 2012 -0800
summary:     Bug 817341 - Differentiate marking and sweep-marking in GC stats (r=jonco)

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Jesse Ruderman]

Attachment: stack

Comment 3

[Jesse Ruderman]

Shell testcase coming up

Comment 4

[Jesse Ruderman]

Shell testcase:

Object.getOwnPropertyNames(this);
gcparam('markStackLimit', 2)

Comment 5

[Jon Coppeard (:jonco)]

markDelayedChildren() can take place in any of the following phases:

 - PHASE_MARK
 - PHASE_SWEEP_MARK
 - PHASE_SWEEP_MARK_INCOMING_BLACK
 - PHASE_SWEEP_MARK_INCOMING_GRAY
 - PHASE_SWEEP_MARK_GRAY

However only the first two are currently handled, and if it occurs in the others it will trigger this assertion.  Fortunately this is rare outside of deliberately setting markStackLimit.

I'm going to suggest that we only give this a separate phase for marking, and for the other phases it's called in we just roll its time into that phase.

Bill are you ok with that approach?

Comment 6

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

Yeah, sounds good. Perhaps you could use Maybe<AutoPhase> to implement that?

Comment 7

[Jon Coppeard (:jonco)]

Attachment: bug939476-stats-phase

Here's a patch.  I tried to use Maybe<AutoPhase> but the template construct() definitions only takes const& arguments, so that didn't work for passing Statistics.

Comment 8

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

Comment on attachment 8334618 [details] [diff] [review]
bug939476-stats-phase

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

I guess we could change AutoPhase to take a Statistics* to get around the const T& problem, but this seems fine too.

Comment 9

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/915744e45b57

Comment 10

[Ed Morley [:emorley]]

Unfortunately this and the other bugs in https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=db0f8a5eeb33 have been backed out for causing rootanalysis assertions, eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=30835010&tree=Mozilla-Inbound

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=05a0228c2caa

(For quick relanding, I recommend the third party qbackout extension and '--apply' mode)

Comment 11

[Jon Coppeard (:jonco)]

Relanding:

https://hg.mozilla.org/integration/mozilla-inbound/rev/7762a99ce512

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7762a99ce512

Comment 13

[Ada [:adalucinet]]

Reproduced with Nightly debug build from 2013-11-15 on Ubuntu 13.10 x64: 
Assertion failure: phases[phase].parent == parent, at /builds/slave/m-cen-l64-st-an-d-000000000000/build/js/src/gc/Statistics.cpp:629
No assertions with Fx 28 beta 2 (Build ID: 20140210135004) debug build on the same machine.

On Mac OS X 10.8 on both Nightly and Fx 28 beta 2 debug builds I get:
Assertion failure: isEmpty(), at /builds/slave/m-beta-osx64-d-000000000000000/build/js/src/jsgc.h:981 ; maybe bug 939499?
I suppose because of bug 939499 I cannot see/verify this failure.
Is there any other way to verify this fix on Mac OS X too and do not hit Assertion failure: isEmpty()?

Comment 14

[Jon Coppeard (:jonco)]

(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #13)
I don't know why you're getting that failure.  It seems like bug 939499 should only apply if verifyprebarriers() is called first, and there should be no difference between Mac OS and Linux either.

Just to confirm what's happening, can you post a backtrace of the assertion?

I'll have a look at bug 939499 as well.

Comment 15

[Ada [:adalucinet]]

Attachment: console log

This is what I get in the console.
Do you need something more specific? If yes, which tool should I use to get that?