Closed
Bug 777834
Opened 12 years ago
Closed 12 years ago
"Assertion failure: ptr,"
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: gkw, Assigned: Benjamin)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [js:p1:fx17])
Attachments
(3 files, 1 obsolete file)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
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•12 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: savesource
Assignee | ||
Comment 2•12 years ago
|
||
Assignee: general → bpeterson
Attachment #646478 -
Flags: review?(jorendorff)
Updated•12 years ago
|
Whiteboard: [js:p1:fx17]
Comment 3•12 years ago
|
||
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-
Comment 4•12 years ago
|
||
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•12 years ago
|
||
Any suggestions? endOffset() doesn't actually need to consult userbuf. It just does for more debug mode assertions.
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #646726 -
Flags: review?(jorendorff)
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #646478 -
Attachment is obsolete: true
Attachment #646727 -
Flags: review?(jorendorff)
Comment 8•12 years ago
|
||
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 9•12 years ago
|
||
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•12 years ago
|
||
Comment 11•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0b03576b81b5
https://hg.mozilla.org/mozilla-central/rev/c2817ed24402
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Comment 12•12 years ago
|
||
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.
Description
•