Closed Bug 878293 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla24
Tracking Status
firefox22 --- unaffected
firefox23 --- unaffected
firefox24 --- fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: gkw, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

Attached file 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
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.
Brian, is bug 678037 a likely regressor?
Blocks: LazyBytecode
Flags: needinfo?(bhackett1024)
Attached patch fix cruft patchSplinter Review
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)
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+
https://hg.mozilla.org/mozilla-central/rev/989c3713ab99
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Duplicate of this bug: 878166
Duplicate of this bug: 878041
Status: RESOLVED → VERIFIED
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.