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

VERIFIED FIXED in Firefox 24



6 years ago
5 years ago


(Reporter: gkw, Assigned: bhackett)


(Blocks: 1 bug, 4 keywords)

assertion, regression, sec-high, testcase
Dependency tree / graph

Firefox Tracking Flags

(firefox22 unaffected, firefox23 unaffected, firefox24 fixed, firefox-esr17 unaffected, b2g18 unaffected)


(Whiteboard: [jsbugmon:update])


(2 attachments)



6 years ago
Created attachment 756779 [details]

"".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
Likely a dup of bug 878041. And those asserts in LifoAlloc are usually all s-s.
Group: core-security
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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

6 years ago
Brian, is bug 678037 a likely regressor?
Blocks: 678037
Flags: needinfo?(bhackett1024)

Comment 4

6 years ago
Created attachment 757496 [details] [diff] [review]
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).
Assignee: general → bhackett1024
Attachment #757496 - Flags: review?(luke)
Flags: needinfo?(bhackett1024)
status-b2g18: --- → unaffected
status-firefox22: --- → unaffected
status-firefox23: --- → unaffected
status-firefox24: --- → affected
status-firefox-esr17: --- → unaffected
tracking-firefox24: --- → ?
Keywords: sec-high


6 years ago
Attachment #757496 - Flags: review?(luke) → review?(jwalden+bmo)
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.)
Attachment #757496 - Flags: review?(jwalden+bmo) → review+
Last Resolved: 6 years ago
status-firefox24: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla24


6 years ago
Duplicate of this bug: 878166
Duplicate of this bug: 878041
tracking-firefox24: ? → ---
JSBugMon: This bug has been automatically verified fixed.
Duplicate of this bug: 879747
Group: core-security
You need to log in before you can comment on or make changes to this bug.