392593 - Wrong type for arguments object

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Marc]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.6) Gecko/20060601 Firefox/2.0.0.6 (Ubuntu-edgy)
Build Identifier: 

The "arguments" object available within function execution should be a "normal" object not an own type.

This leads currently to errors for instance for typeof or toString() (see unit test in attached patch)

Reproducible: Always

Steps to Reproduce:
1.
2.
3.



[seems I can't attach a patch here. I try to open the issue first and attach then]

Comment 1

[Marc]

Attachment: Patch for arguments object including unit test

Comment 2

[Marc]

any comment on bug and provided patch?

Comment 3

[Attila Szegedi]

Could you point out why is the current behaviour a problem?

Comment 4

[Marc]

Extracted from the unit test in the provided patch;

// 1: Rhino fails here as it returns Arguments
function f() {
  var t = typeof arguments;
  if (t != 'object') throw 'Failed: got ' + t;
}
f();

// 2: Rhino fails here too. Again "Arguments" where it should be "Object"

function f() {
  var t = arguments.toString();
  if (t != '[object Object]') throw 'Failed: got ' + t;
}
f();

Comment 5

[Attila Szegedi]

Ah, okay, ECMA-262 section 10.1.8 actually prescribes:

"The value of the internal [[Prototype]] property of the arguments object is the original Object 
prototype object, the one that is the initial value of Object.prototype"

So this is indeed a bug against the ECMA spec.

Comment 6

[Attila Szegedi]

Your patch changes it from IdScriptableObject to NativeObject, but doesn't address the problem that "callee", "length", and "caller" were provided on the arguments object via the IdScriptableObject mechanism.

Comment 7

[Marc]

Isn't this already handled because NativeObject extends IdScriptableObject? (I haven't checked)

Comment 8

[Norris Boyd]

Attachment: Proposed patch

In my testing Rhino only fails "// 2" on comment #4. 

The expression 

(function () { return arguments.__proto__ === Object.prototype; })() 

is true on both Rhino and SpiderMonkey, so I'm hoping the attached patch is sufficient to fix this. Comments?

Comment 9

[Attila Szegedi]

Norris, I'll show my ignorance of some of the Rhino implementation here, but is it safe to further subclass a class that is already subclassing IdScriptableObject? Will the IDs from the superclass (NativeObject) coexist peacefully with IDs in the subclass (Arguments)?

Comment 10

[Norris Boyd]

The problem is the usage models are different. "Object" has a constructor and defines a prototype, but "arguments" is an object that has Object.prototype as its prototype. So we don't want all the NativeObject features in this case.

Comment 11

[Marc]

Norris, your patch seems ok to me. 

I don't remember how I got the failing result for "// 1" on comment #4, it's long ago. No matter.

Comment 12

[Norris Boyd]

Committed my patch:

Checking in src/org/mozilla/javascript/Arguments.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/Arguments.java,v  <--  Argu
ments.java
new revision: 1.32; previous revision: 1.31
done

Comment 13

[Marc]

Cool!

And the unit test?

Comment 14

[Norris Boyd]

Attachment: ecma/ExecutionContexts/10.1.8-3.js

Proposed test for typeof arguments.

Comment 15

[Bob Clary [:bc] (inactive)]

Comment on attachment 300005 [details]
ecma/ExecutionContexts/10.1.8-3.js

flipping type so it can be viewed.

Comment 16

[Bob Clary [:bc] (inactive)]

Comment on attachment 300005 [details]
ecma/ExecutionContexts/10.1.8-3.js

>
>var SECTION = "10.1.8-2";
>var VERSION = "ECMA_1";
>startTest();
>var TITLE   = "Arguments Object";
>
>var expected = "object";
>var actual = (function () { return typeof arguments; })();
>reportCompare(expected, actual, "typeof arguments == object");
>
>writeHeaderToLog( SECTION + " "+ TITLE);

nit: move the writeHeaderToLog to just below the var TITLE.  r+ with that.

Comment 17

[Norris Boyd]

Change made, test committed to cvs. Thanks for the quick review!