Closed Bug 591437 Opened 10 years ago Closed 9 years ago

Reflect.parse(): can't reliably use pn_cookie for function args

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dherman, Assigned: dherman)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

js> Reflect.parse("function f(a) { function a() { } }")
Assertion failure: !isFree(), at ../../jsscript.h:105
Segmentation fault
Comment on attachment 469988 [details] [diff] [review]
don't check frameSlot() on formal args, only on destructuring args

>From: Dave Herman <dherman@mozilla.com>
>
>bug 591437, r=?: can't reliably use pn_cookie for function args
>
>diff --git a/js/src/jsreflect.cpp b/js/src/jsreflect.cpp
>--- a/js/src/jsreflect.cpp
>+++ b/js/src/jsreflect.cpp
>@@ -2712,26 +2712,37 @@ ASTSerializer::functionArgs(JSParseNode 
>      * Arguments are found in potentially two different places: 1) the
>      * argsbody sequence (which ends with the body node), or 2) a
>      * destructuring initialization at the beginning of the body. Loop
>      * |arg| through the argsbody and |destruct| through the initial
>      * destructuring assignments, stopping only when we've exhausted
>      * both.
>      */
>     while ((arg && arg != pnbody) || destruct) {
>-        if (arg && arg != pnbody && arg->frameSlot() == i) {
>+        if (destruct && destruct->pn_right->frameSlot() == i) {
>+            if (!pattern(destruct->pn_left, NULL, &node) ||
>+                !args.append(node))
>+                return false;

Nit, pre-existing: house style would put the if condition on one line, but if it didn't fit, then even though the "then" takes one line it would be braced.

>+            destruct = destruct->pn_next;
>+        } else if (arg && arg != pnbody) {
>+            /*
>+             * We don't check that arg->frameSlot() == i since we
>+             * can't call that method if the arg def has been turned
>+             * into a use, e.g.:
>+             *
>+             *     function(a) { function a() { } }
>+             *
>+             * There's no other way to ask a non-destructuring arg its
>+             * index in the formals list, so we rely on the ability to
>+             * ask destructuring args their index above.
>+             */
>             if (!identifier(arg, &node) ||
>                 !args.append(node))
>                 return false;

Same nit.

Looks good to me with nits picked, pre-emptive r+.

/be
Attachment #469988 - Flags: review+
http://hg.mozilla.org/tracemonkey/rev/4f0f0d22e3c5
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/4f0f0d22e3c5
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.