777834 - "Assertion failure: ptr,"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: 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,

Comment 1

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

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

Comment 2

[:Benjamin Peterson]

Attachment: ignore userbuf when it's posioned

Comment 3

[Jason Orendorff [:jorendorff]]

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)

Comment 4

[Jason Orendorff [:jorendorff]]

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.

Comment 5

[:Benjamin Peterson]

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

Comment 6

[:Benjamin Peterson]

Attachment: add a tokenizer error flag

Comment 7

[:Benjamin Peterson]

Attachment: don't call endOffset if the tokenizer is in error

Comment 8

[Jason Orendorff [:jorendorff]]

Comment on attachment 646726 [details] [diff] [review]
add a tokenizer error flag

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

Yeah, ok.

Comment 9

[Jason Orendorff [:jorendorff]]

Comment on attachment 646727 [details] [diff] [review]
don't call endOffset if the tokenizer is in error

All right.

Comment 10

[:Benjamin Peterson]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0b03576b81b5
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2817ed24402

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0b03576b81b5
https://hg.mozilla.org/mozilla-central/rev/c2817ed24402

Comment 12

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/function-tosource-exprbody-bug777834.js.