Closed
Bug 433279
Opened 14 years ago
Closed 14 years ago
Object destructuring shorthand can cause "Assertion failure: pn != tc->parseContext->nodeList"
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla1.9
People
(Reporter: nanto, Assigned: igor)
References
()
Details
(Keywords: assertion, crash, testcase, Whiteboard: [RC2+])
Attachments
(1 file)
954 bytes,
patch
|
mrbkap
:
review+
shaver
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
js> var { sin, PI } = Math; sin(PI / 2); Assertion failure: pn != tc->parseContext->nodeList, at jsparse.c:249 In a browser, it causes a "too much recursion" error. This bug doesn't occur if you split line in two in JavaScript shell or introduce a temporary variable. js> var { sin, PI } = Math; js> sin(PI / 2); 1 js> var { sin, PI } = Math, halfPI = PI / 2; sin(halfPI); 1
Comment 1•14 years ago
|
||
Here's an example that doesn't use Math: js> var {a} = b; c(d + 1); Assertion failure: pn != tc->parseContext->nodeList, at jsparse.c:249 This seems like it could affect a lot of the people who try to take advantage of the shorthand introduced in bug 404734 :( I think I forgot to add this destructuring shorthand to jsfunfuzz :(
Key JS 1.8 feature busted, not sure we can get a fix that has low-enough risk profile that we'd take it, but this is the first thing I've seen go by recently that I thought was worth putting on drivers' radar. :/
Flags: blocking1.9?
Updated•14 years ago
|
Version: unspecified → Trunk
Updated•14 years ago
|
Summary: Object destructuring shorthand can cause assertion failure → Object destructuring shorthand can cause "Assertion failure: pn != tc->parseContext->nodeList"
Comment 3•14 years ago
|
||
The examples above seem to work fine in opt, but here's one jsfunfuzz found that crashes opt with a null deref [@ Variables]: ({a}) = b; with({}) { for(let y in z) { } }
Keywords: crash
Assignee | ||
Updated•14 years ago
|
Assignee: general → igor
mrbkap found the bug, but I note that he didn't say anything in here; can you help us out, Blake? If we have a clean patch and we respin 3, it'd be good to take it. If we don't, we can put it in a quickish 3.0.1, since there are no web-compat issues for new features. Igor: do you have an ETA for a fix?
Flags: wanted1.9.0.x+
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Comment 5•14 years ago
|
||
This is caused by calling NewBinary with left == right. This happens in the object destructuring shorthand case: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsparse.c&rev=3.347&root=/cvsroot&mark=5573,5587#5557 The simplest fix for this bug is to clone the name node that we have in the object destructuring case instead of using the exact same node. I think a better fix is to let the list of destructuring names be either binary or unary nodes and deal with the special case in the emitter, but that seems riskier if we want this for Firefox 3.
Comment 6•14 years ago
|
||
Comment 7•14 years ago
|
||
Comment on attachment 320776 [details] [diff] [review] fix This is the minimal fix. It's zero-risk and does the job. Please help get it in ASAP. Thanks, /be
Attachment #320776 -
Attachment is patch: true
Attachment #320776 -
Attachment mime type: application/octet-stream → text/plain
Attachment #320776 -
Flags: review?(mrbkap)
Comment 8•14 years ago
|
||
How do you nominate in case there's an RC2? /be
Flags: blocking1.9- → blocking1.9?
Comment 9•14 years ago
|
||
(In reply to comment #8) > How do you nominate in case there's an RC2? [RC2?] in the whiteboard.
Whiteboard: [RC2?]
Comment on attachment 320776 [details] [diff] [review] fix r=shaver
Attachment #320776 -
Flags: review+
Updated•14 years ago
|
Attachment #320776 -
Flags: review?(mrbkap) → review+
Comment 11•14 years ago
|
||
I did some fuzzing with this patch. I didn't see any new assertions, and this assertion went away.
Comment 12•14 years ago
|
||
Ok this is marked with RC2? and Wanted1.9.0.x so it will go in whichever one of those is the next release but won't force an RC2 on it's own. So I'm removing the nom flag.
Flags: blocking1.9? → blocking1.9-
Updated•14 years ago
|
Whiteboard: [RC2?] → [RC2?][RC2+]
Comment 13•14 years ago
|
||
Comment on attachment 320776 [details] [diff] [review] fix a+ schrep for 3.0.1 or RC2 please land on cvs trunk.
Attachment #320776 -
Flags: approval1.9+
Comment 14•14 years ago
|
||
Fixed on cvs.mozilla.org trunk: js/src/jsparse.c 3.348 /be
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Whiteboard: [RC2?][RC2+] → [RC2+]
Target Milestone: --- → mozilla1.9
Comment 15•14 years ago
|
||
/cvsroot/mozilla/js/tests/js1_8/regress/regress-433279-01.js,v <-- regress-433279-01.js initial revision: 1.1 /cvsroot/mozilla/js/tests/js1_8/regress/regress-433279-02.js,v <-- regress-433279-02.js initial revision: 1.1 /cvsroot/mozilla/js/tests/js1_8/regress/regress-433279-03.js,v <-- regress-433279-03.js initial revision: 1.1 mozilla-central: changeset: 16439:c19194ae08ea
Flags: in-testsuite+
Flags: in-litmus-
Updated•14 years ago
|
Flags: wanted1.9.0.x+
You need to log in
before you can comment on or make changes to this bug.
Description
•