878293 - Assertion failure: mark <= bump, at ds/LifoAlloc.h

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

"".replace(RegExp(""), (function() {
    'a'.replace(/a/, eval)
}))

asserts js debug shell on m-c changeset c531be620eba without any CLI arguments at Assertion failure: mark <= bump, at ds/LifoAlloc.h

Comment 1

[Christian Holler (:decoder)]

Likely a dup of bug 878041. And those asserts in LifoAlloc are usually all s-s.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   133453:d71234d65e90
user:        Brian Hackett
date:        Thu May 30 06:29:56 2013 -0600
summary:     Bug 678037 - Add (disabled) ability to parse script bytecode lazily, r=luke.

This iteration took 328.784 seconds to run.

Comment 3

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

Brian, is bug 678037 a likely regressor?

Comment 4

[Brian Hackett [Laid off!]]

Attachment: fix cruft patch

The problem here is that Parser::init() was not being called on some of the SyntaxParsers being constructed on the stack, causing the destructor to reset the temp lifoalloc incorrectly.

More generally though, Parser::init() does nothing except set the mark spot on the lifoalloc (which is infallible) and make sure the cx has a parse maps pool.  The latter thing is associated with a whole lot of cruft that is now unnecessary with single threaded runtimes.  The attached patch fixes this stuff so that the runtime holds the parse maps pool and it always exists (it's only a couple vectors).

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 757496 [details] [diff] [review]
fix cruft patch

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

Nice.  Another step toward unifying JSContext and JSRuntime!

::: js/src/frontend/Parser.cpp
@@ +430,2 @@
>  
>      tempPoolMark = context->tempLifoAlloc().mark();

Use either cx or context throughout this method, not a mix of the two.  (I don't know that we have any existing style rule to follow.)

::: js/src/jsapi.cpp
@@ +5439,5 @@
>          Parser<frontend::FullParseHandler> parser(cx, options, chars, length,
>                                                    /* foldConstants = */ true, NULL, NULL);
> +        older = JS_SetErrorReporter(cx, NULL);
> +        if (!parser.parse(obj) &&
> +            parser.tokenStream.isUnexpectedEOF()) {

This looks like it might fit on one line now, maybe.

::: js/src/jscntxt.cpp
@@ +1147,5 @@
>      functionCallback(NULL),
>  #endif
>      innermostGenerator_(NULL),
>  #ifdef DEBUG
> +    stackIterAssertionEnabled(true)

This is probably a compile error in opt builds, right?  (Trailing comma in member-init list.)

Comment 6

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/989c3713ab99

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/989c3713ab99

Comment 10

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.