94506 - parameter named "arguments" is not accessible in JavaScript method

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[deneen]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.2+)
Gecko/20010731
BuildID:    2001073103

the following function does not do what is expected.

javascript:function f(arguments) { return arguments; }; f(5)



Reproducible: Always
Steps to Reproduce:
1. paste javascript line from description into location bar, press enter


Actual Results:  [object Object]

Expected Results:  5

this example works as expected in NS4 and IE5.5

Comment 1

[Mike Shaver (:shaver emeritus)]

At first, I thought it was enough to just tag the function as heavyweight when
we saw a formal parameter named "arguments".  Turns out that doesn't work,
because we never bind the formal parameter's name in a visible scope, due to the
righteous GETARG optimizations.

Instead, I moved the check for "arguments" in LookupArgOrVar to only run if we
can't optimize to a slot access.  This seems to fix the problem, though I'll
have to find the time to run the test suite to be sure (or beg Phil nicely):

js> function f(arguments) { return arguments; } 
js> dis(f)
main:
00000:  getarg 0
00003:  return
js> f(5);
5
js> function g() { var arguments = 5; return arguments; }
js> g()
5
js> dis(g)
flags: HEAVYWEIGHT
00000:  defvar "arguments"
main:
00003:  bindname "arguments"
00006:  uint16 5
00009:  setname "arguments"
00012:  pop
00013:  arguments
00014:  return
js> g()
5
js> function h() { return arguments; var arguments = 5; }js> dis(h)
flags: HEAVYWEIGHT
00000:  defvar "arguments"
main:
00003:  arguments
00004:  return
00005:  bindname "arguments"
00008:  uint16 5
00011:  setname "arguments"
00014:  pop
js> h()
[object Object]

Patch coming presently.

Comment 2

[Mike Shaver (:shaver emeritus)]

Attachment: allow "arguments" references to be optimized to argument slots if appropriate

Comment 3

[Brendan Eich [:brendan]]

shaver, you want to take this bug?  Else maybe khanson will, it's yet another
jsemit.c patch.

sr=[e-mail] on the patch, although comment punctuation style is
*slightly* off (two spaces after full stop, less ragged-right through judicious
word choice :-).

/be

Comment 4

[Phil Schwartau]

Testcase added to JS testsuite:

       mozilla/js/tests/ecma_3/Function/regress-94506.js


It includes another possiblity: an identifier "arguments" defined
as a global variable:

var arguments = 5;
function F() { return arguments; }


In this case we WOULD expect the return value of F to be the 
arguments object of F, and NOT the value 5  -  right ? 
This seems obvious, but I just want to be absolutely sure. 


Will now run the testsuite with Mike's patch applied - 

Comment 5

[Phil Schwartau]

Ran the JS testsuite on WinNT and Linux with and without Mike's patch.
With the patch applied, no new regressions were introduced, and the 
testcase for this bug passed.

Comment 6

[Phil Schwartau]

Here is the above testcase for this bug:

var arguments = 5555;
function F1(arguments) { return arguments; }
function F2() { var arguments = 55; return arguments; }
function F3() { return arguments; var arguments = 555; }
function F4() { return arguments; }

I am expecting these return values:

F1(5) == 5
F2()  == 55
F3()  == arguments object (not 555)
F4()  == arguments object (not 5555)


Questions: 

1. Do I have the expected values right?

2. Without Mike's patch, case F1 fails, but case F2 passes.
   Is that to be expected?
 

Comment 7

[Phil Schwartau]

Added this to test as well: invoking F1 with too few; too many parameters.
I am expecting:

F1()          == undefined
F1(3,33,333)  == 3

Comment 8

[Phil Schwartau]

If I write a script consisting of this one line:

                print('x = '  +  x);

The output is: 

                ReferenceError: x is not defined


If I write a script of two lines:

                print('x = '  +  x);
                var x = 555;

The output is:

                x = undefined


Thus, the var x statement is hoisted above the print() and x is recognized
as an identifier. The variable x has the value undefined, however, 
because the assignment to 555 is not done until after the print().


So in case F3 above:

      function F3() { return arguments; var arguments = 555; } 

Shouldn't we have the same hoisting taking place within the function's scope?
Thus, shouldn't F3() == undefined ?

Comment 9

[Brendan Eich [:brendan]]

Phil wrote:
>So in case F3 above:
>
>      function F3() { return arguments; var arguments = 555; }
>
>Shouldn't we have the same hoisting taking place within the function's scope?
>Thus, shouldn't F3() == undefined ?

Only if there is no pre-existing property.  Recall that var (by itself,
initialization is a separate issue) never changes the binding of a property that
already exists.

/be

Comment 10

[Phil Schwartau]

cc'ing Norris. Testcase is failing in Rhino as well as in SpiderMonkey.
 
           mozilla/js/tests/ecma_3/Function/regress-94506.js

Comment 11

[Brendan Eich [:brendan]]

shaver, you want to pass this off to someone else?  It shouldn't miss 0.9.5 (it
has missed 0.9.4 unless you wanna carry water to drivers :-).

/be

Comment 12

[rogerl (gone)]

r=rogerl

Comment 13

[Mike Shaver (:shaver emeritus)]

Fixed in jsemit.c 3.61.

Comment 14

[Phil Schwartau]

Verified with smdebug and smopt shells built 2001-09-27 on WinNT, Linux.
The above testcase now passes - 

Comment 15

[Phil Schwartau]

Note: testcase still fails in Rhino; bug 102093 has been filed against Rhino
for this issue -