608543 - Arguments elements should alias formals even for indexes beyond the range of actuals that were passed (in ES5 non-strict)

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Nicholas C. Zakas]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101030 Firefox/4.0b8pre
Build Identifier: 

Section 10.6 Note 1 says:

"For non-strict mode functions the array index (defined in 15.4) named data properties of an arguments object whose numeric name values are less than the number of formal parameters of the corresponding function object initially share their values with the corresponding argument bindings in the function’s execution context. This means that changing the property changes the corresponding value of the argument binding and vice-versa. "

In strict mode, assigning to an arguments property should not cause changes in the named parameter. However, in the latest Minefield (2010-Oct-30), this behavior is also seen in non-strict mode.


Reproducible: Always

Steps to Reproduce:
1. Run attached test script
Actual Results:  
Result of doAdd(10) is NaN because num2 is undefined.

Expected Results:  
Result of doAdd(10) should be 20, because num2 should reflect the value assigned to arguments[1].

Comment 1

[Tom S [:evilpie]]

Could not really explain over twitter, but i think this is actually correct 
behavior.

10.6
>args the actual arguments passed to the [[Call]] internal method
>1. Let len be the number of elements in args.
...
>10. Let indx = len - 1.
>11. Repeat while indx >= 0, 

So all these fancy getter/setter will never be created even in non strict mode.

Comment 2

[Brendan Eich [:brendan]]

10.6 definition: "... args the actual arguments passed to the [[Call]] internal method...."

10.6 step 1: "Let len be the number of elements in args."

10.6 steps 10 and 11 then loop from len-1 down to 0 (inclusive), defining the magic getter and setter. Thus the call doAdd(10) has args [10] and len 1, so the loop iterates over only arg 0, num1 is its formal parameter, and assignment to arguments[1] does not alias formal paremeter num2.

BTW, ES1-3 spec'ed this too, less formally. SpiderMonkey has followed this part of the spec for quite a while.

/be

Comment 3

[Nicholas C. Zakas]

Thanks for the explanation. Guess I was thrown off by the interop issue.

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #3)
> Thanks for the explanation. Guess I was thrown off by the interop issue.

Did you file a webkit bug? Please link here so everyone can track. Thanks,

/be

Comment 5

[Dmitry Soshnikov]

(In reply to comment #4)
> (In reply to comment #3)
> > Thanks for the explanation. Guess I was thrown off by the interop issue.
> 
> Did you file a webkit bug? Please link here so everyone can track. Thanks,
> 

Already was filed (for V8) http://code.google.com/p/chromium/issues/detail?id=52484

@Nicholas C. Zakas here's a description of the correct semantics implemented in ES itself http://gist.github.com/539974 

Dmitry.

Comment 6

[Jeff Walden [:Waldo]]

Either I'm skimming too hard and not seeing the earlier reference, or this was likely from IRC discussion -- but does "webkit" in the context of this bug refer to Safari/WebKit/Nitro or to Chrome/V8?  Of course a bug filed against the latter wouldn't be relevant to the former, assuming "webkit" indeed meant Safari/WebKit/Nitro -- just looking for someone to clarify.

Comment 7

[Dmitry Soshnikov]

(In reply to comment #6)
> does "webkit" in the context of this bug
> refer to Safari/WebKit/Nitro or to Chrome/V8?

Safari (WebKit) has a correct behavior. The bug relates only to Chrome (V8).

Notice: the mentioned V8's bugs refers to incorrect `delete` behavior of index-properties of `arguments`, however it covers also the case with incorrect setting of values (I mentioned it before in variable object analysis article), since the whole behavior of setters/getters is broken.

Dmitry.

Comment 8

[Brendan Eich [:brendan]]

Dmitry: is that v8 issue about delete enough? This bug's testcase at the URL does not involve delete.

/be

Comment 9

[Dmitry Soshnikov]

(In reply to comment #8)
> Dmitry: is that v8 issue about delete enough? This bug's testcase at the URL
> does not involve delete.
> 

Yeah, as I mentioned above -- in its exact case, the V8's bug is about incorrect `delete` behavior on `arguments`, but since it involves common setters/getters issue, it covers assignment (with sharing) too. One can open a separate issue, but when the first bug will be fixed, the later won't be reproducible. However, for the tracking it's possibly good to open the separate one.

Dmitry.

Comment 10

[Brendan Eich [:brendan]]

I think v8 needs another "issue". The effect and cause are different AFAICT. While a fix for both effects might appear as a patch for the existing issue, it is better protocol to file separate bugs to track distinct symptoms that may have distinct fixes.

/be

Comment 11

[Dmitry Soshnikov]

Yes, I think so. Though, it's already also on file, found it (http://code.google.com/p/chromium/issues/detail?id=36350); and V8's team refers it as a kind of duplicate of the issue with `delete` (mentioned above bug refers it too: http://code.google.com/p/chromium/issues/detail?id=32184).

Dmitry.