Object destructuring shorthand can cause "Assertion failure: pn != tc->parseContext->nodeList"

RESOLVED FIXED in mozilla1.9

Status

()

RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: nanto, Assigned: igor)

Tracking

({assertion, crash, testcase})

Trunk
mozilla1.9
assertion, crash, testcase
Points:
---
Bug Flags:
blocking1.9 -
wanted1.9 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [RC2+], URL)

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
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

11 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 :(
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

Updated

11 years ago
Summary: Object destructuring shorthand can cause assertion failure → Object destructuring shorthand can cause "Assertion failure: pn != tc->parseContext->nodeList"

Comment 3

11 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

11 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-
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 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+

Comment 11

11 years ago
I did some fuzzing with this patch.  I didn't see any new assertions, and this assertion went away.

Comment 12

11 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

11 years ago
Whiteboard: [RC2?] → [RC2?][RC2+]

Comment 13

11 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+
Fixed on cvs.mozilla.org trunk:

js/src/jsparse.c 3.348

/be
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: [RC2?][RC2+] → [RC2+]
Target Milestone: --- → mozilla1.9

Comment 15

10 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-
Flags: wanted1.9.0.x+
You need to log in before you can comment on or make changes to this bug.