1471788 - Inconsistent output compared with other JS engines

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[sunlili]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.87 Safari/537.36

Steps to reproduce:

Execute following code:

v1=new (Float64Array)();
v2={
    valueOf : function (){ 
        v3.y="bar";
        return 42;}
    };  
v3=v1;
v3[0]=v2;
print(JSON.stringify(v1));


Actual results:

{}


Expected results:

{"y":"bar"}

Comment 1

[André Bargull [:anba]]

The test case is correct, but I'm still going to file a spec issue for this one, because I don't like how [[DefineOwnProperty]]  [1] performs bounds checks before calling IntegerIndexedElementSet, whereas [[Set]] [2] directly calls IntegerIndexedElementSet and then relies on IntegerIndexedElementSet [3] performing the bounds checks. Maybe 9.4.5.3 [[DefineOwnProperty]], steps 3.b.i-v should just be removed?

[1] https://tc39.github.io/ecma262/#sec-integer-indexed-exotic-objects-defineownproperty-p-desc
[2] https://tc39.github.io/ecma262/#sec-integer-indexed-exotic-objects-set-p-v-receiver
[3] https://tc39.github.io/ecma262/#sec-integerindexedelementset

Comment 2

[André Bargull [:anba]]

Attachment: bug1471788.patch

Applies on top of bug 1471841.


Adds the missing ToNumber call to DefineNonexistentProperty.

Also updates/adds step comments and corrects the spec reference in DefineNonexistentProperty: Despite its name, DefineNonexistentProperty when called with a typed array, implements 9.4.5.5 [[Set]], not 9.4.5.3 [[DefineOwnProperty]]!

And a drive-by change in IsTypedArrayIndex to use a more sensible cast.

Comment 3

[Tom S [:evilpie]]

Comment on attachment 8988483 [details] [diff] [review]
bug1471788.patch

Review of attachment 8988483 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks for adding the steps.

Comment 4

[André Bargull [:anba]]

Attachment: bug1471788.patch

Rebased to apply cleanly on inbound, carrying r+.

Comment 5

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=49c631ff0b3550ec9d8b8036a922c50f647e8d88

Comment 6

[Dorel Luca [:dluca]]

Hi, I am unable to apply your patch after importing it.

This is the error I get when doing so:

hg qpush -a 
applying bug1471788.patch
patching file js/src/vm/NativeObject.cpp
Hunk #2 FAILED at 2657
1 out of 2 hunks FAILED -- saving rejects to file js/src/vm/NativeObject.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh bug1471788.patch.

Can you please check it?

Comment 7

[André Bargull [:anba]]

Attachment: bug1471788.patch

Rebased patch to apply cleanly on inbound.

Comment 8

[André Bargull [:anba]]

(In reply to Dorel Luca [:dluca] from comment #6)
> Hi, I am unable to apply your patch after importing it.

The patch needed to be rebased again, sorry for the trouble!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3030106b14f7
Apply ToNumber conversion even when the typed array write is out-of-bounds. r=evilpie

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3030106b14f7