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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: nanto, Assigned: igor)

References

()

Details

(Keywords: assertion, crash, testcase, Whiteboard: [RC2+])

Attachments

(1 file)

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
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 :(
Keywords: assertion, testcase
OS: Windows XP → All
Hardware: PC → All
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?
Version: unspecified → Trunk
Summary: Object destructuring shorthand can cause assertion failure → Object destructuring shorthand can cause "Assertion failure: pn != tc->parseContext->nodeList"
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: 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-
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.
Attached patch fixSplinter Review
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)
How do you nominate in case there's an RC2?

/be
Flags: blocking1.9- → blocking1.9?
(In reply to comment #8)
> How do you nominate in case there's an RC2?

[RC2?] in the whiteboard.
Whiteboard: [RC2?]
Attachment #320776 - Flags: review?(mrbkap) → review+
I did some fuzzing with this patch.  I didn't see any new assertions, and this assertion went away.
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-
Whiteboard: [RC2?] → [RC2?][RC2+]
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+
Fixed on cvs.mozilla.org trunk:

js/src/jsparse.c 3.348

/be
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [RC2?][RC2+] → [RC2+]
Target Milestone: --- → mozilla1.9
/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-
Flags: wanted1.9.0.x+
You need to log in before you can comment on or make changes to this bug.