Last Comment Bug 777834 - "Assertion failure: ptr,"
: "Assertion failure: ptr,"
Status: RESOLVED FIXED
[js:p1:fx17]
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: mozilla17
Assigned To: :Benjamin Peterson
: general
Mentors:
Depends on:
Blocks: jsfunfuzz savesource
  Show dependency treegraph
 
Reported: 2012-07-26 11:51 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-01-14 08:42 PST (History)
6 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stack (5.33 KB, text/plain)
2012-07-26 11:51 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
ignore userbuf when it's posioned (1.75 KB, patch)
2012-07-26 23:30 PDT, :Benjamin Peterson
jorendorff: review-
Details | Diff | Review
add a tokenizer error flag (5.16 KB, patch)
2012-07-27 14:40 PDT, :Benjamin Peterson
jorendorff: review+
Details | Diff | Review
don't call endOffset if the tokenizer is in error (1.42 KB, patch)
2012-07-27 14:40 PDT, :Benjamin Peterson
jorendorff: review+
Details | Diff | Review

Description Gary Kwong [:gkw] [:nth10sd] 2012-07-26 11:51:59 PDT
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,
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2012-07-26 12:03:12 PDT
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 2012-07-26 23:30:38 PDT
Created attachment 646478 [details] [diff] [review]
ignore userbuf when it's posioned
Comment 3 Jason Orendorff [:jorendorff] 2012-07-27 10:56:17 PDT
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] 2012-07-27 11:09:58 PDT
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 2012-07-27 11:16:01 PDT
Any suggestions? endOffset() doesn't actually need to consult userbuf. It just does for more debug mode assertions.
Comment 6 :Benjamin Peterson 2012-07-27 14:40:19 PDT
Created attachment 646726 [details] [diff] [review]
add a tokenizer error flag
Comment 7 :Benjamin Peterson 2012-07-27 14:40:52 PDT
Created attachment 646727 [details] [diff] [review]
don't call endOffset if the tokenizer is in error
Comment 8 Jason Orendorff [:jorendorff] 2012-07-31 08:36:02 PDT
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] 2012-07-31 09:05:05 PDT
Comment on attachment 646727 [details] [diff] [review]
don't call endOffset if the tokenizer is in error

All right.
Comment 12 Christian Holler (:decoder) 2013-01-14 08:42:26 PST
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/function-tosource-exprbody-bug777834.js.

Note You need to log in before you can comment on or make changes to this bug.