"Assertion failure: ptr,"

RESOLVED FIXED in mozilla17

Status

()

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

People

(Reporter: gkw, Assigned: Benjamin)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:p1:fx17])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 646236 [details]
stack

function f(code) {
	new Function(code.substr(code.length / 2, code.length));
}
f("ion() { v0 = g1.eval(\"function f()((l()))++2s")

asserts js debug shell on m-c changeset 7065b767f30d without any CLI arguments at Assertion failure: ptr,
(Reporter)

Comment 1

5 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   99950:e080642175e6
user:        Benjamin Peterson
date:        Fri Jul 20 20:17:38 2012 +0200
summary:     Bug 761723 - Save script sources to implement Function.prototype.toString. r=jorendorff,njn,jimb,jst,Ms2ger
Blocks: 761723
(Assignee)

Comment 2

5 years ago
Created attachment 646478 [details] [diff] [review]
ignore userbuf when it's posioned
Assignee: general → bpeterson
Attachment #646478 - Flags: review?(jorendorff)
Whiteboard: [js:p1:fx17]
Comment on attachment 646478 [details] [diff] [review]
ignore userbuf when it's posioned

I'm pretty sure we don't want this, because
>     * Poisoning userbuf on error establishes an invariant: once an erroneous
>     * token has been seen, userbuf will not be consulted again. [...]
(comment in TokenStream.cpp)
Attachment #646478 - Flags: review?(jorendorff) → review-
I think the problem here is that the code under BEGIN_EXPR_PARSER(mulExpr1) calls tokenStream.getToken() without checking whether it returned TOK_ERROR.

I imagine there are lots of bugs where we don't correctly check for tokenizer errors. Fixing them all (perhaps by changing the signature of .getToken()) is one path; giving up on poisoning is another; I'm open to a middle way, but the patch of comment 2 doesn't give me any reason to think we won't just assert somewhere else next time.
(Assignee)

Comment 5

5 years ago
Any suggestions? endOffset() doesn't actually need to consult userbuf. It just does for more debug mode assertions.
(Assignee)

Comment 6

5 years ago
Created attachment 646726 [details] [diff] [review]
add a tokenizer error flag
Attachment #646726 - Flags: review?(jorendorff)
(Assignee)

Comment 7

5 years ago
Created attachment 646727 [details] [diff] [review]
don't call endOffset if the tokenizer is in error
Attachment #646478 - Attachment is obsolete: true
Attachment #646727 - Flags: review?(jorendorff)
Comment on attachment 646726 [details] [diff] [review]
add a tokenizer error flag

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

Yeah, ok.
Attachment #646726 - Flags: review?(jorendorff) → review+
Comment on attachment 646727 [details] [diff] [review]
don't call endOffset if the tokenizer is in error

All right.
Attachment #646727 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b03576b81b5
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2817ed24402
https://hg.mozilla.org/mozilla-central/rev/0b03576b81b5
https://hg.mozilla.org/mozilla-central/rev/c2817ed24402
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/function-tosource-exprbody-bug777834.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.