Skip to content

Commit

Permalink
[1.6>1.7] [MERGE #3452 @akroshg] Splice helper function should check …
Browse files Browse the repository at this point in the history
…for side-effect in the prototype.

Merge pull request #3452 from akroshg:os12444598

Splice has fast path which did not check the prototype (say you have proxy) properly which resulted to incorrect result. Fixed that by using the side-effect macro to take the slower and observable code path.
  • Loading branch information
akroshg committed Jul 29, 2017
2 parents 0c739a7 + 0dd871b commit 8a61170
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 16 deletions.
3 changes: 2 additions & 1 deletion lib/Runtime/Library/JavascriptArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6901,7 +6901,8 @@ namespace Js
Var* insertArgs = args.Info.Count > 3 ? &args.Values[3] : nullptr;
uint32 insertLen = args.Info.Count > 3 ? args.Info.Count - 3 : 0;

if (pArr != nullptr)
// Force check the prototype as we may insert values more than current elements
if (pArr != nullptr && !HasAnyES5ArrayInPrototypeChain(pArr, true /*forceCheckProtoChain*/))
{
// Since we get the length from an array and that cannot be more than uint32.
_Analysis_assume_(length <= UINT_MAX);
Expand Down
14 changes: 14 additions & 0 deletions test/Array/Array_TypeConfusion_bugs.js
Original file line number Diff line number Diff line change
Expand Up @@ -799,5 +799,19 @@ var tests = [
assert.areEqual(16, ret[0]);
}
},
{
name: "splice : the method splice should get property from prototype which is a proxy",
body: function ()
{
var v1 = [];
v1.length = 20;
var hasCalled = 0;
v1.__proto__ = new Proxy([222], {has : function() { hasCalled++;} });
v1.push(1);
assert.areEqual(222, v1[0]);
var ret = v1.splice(0, 10);
assert.areEqual(20, hasCalled);
}
},
];
testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
18 changes: 9 additions & 9 deletions test/Array/protoLookupWithGetters.baseline
Original file line number Diff line number Diff line change
@@ -1,35 +1,35 @@
Test case 1
d1,d2,d3,d4,p5,p6,p7,p8,p9
p0,p1,p2,p3,p4,p5,p6,p7,p8
9
p0,p1,p2,p3,p4
5

Test case 2
d1,d2,d3,d4,d5,d6,d7,p5,p6,p7,p8,p9
p0,p1,p2,p3,p4,p5,p6,p7,p8,p9,p8,p9
12
p0,p1,p2,p3,p4
5

Test case 3
d1,d2,d3,d4,d5,d6,d7,p5,,p7,,p9
d1,p1,d3,p3,d5,p5,d7,p7,,p9,,p9
12
,p1,,p3,
5

Test case 4
P0,P1,P2,d1,d2,d3,P8,P9
P0,P1,P2,P3,P4,P5,P6,P7
8
P3,P4,P5,P6,P7
5

Test case 5
P0,P1,P2,d1,d2,d3,P8,P9
P0,P1,P2,P3,P4,P5,P6,P7
8
P3,P4,P5,P6,P7
5

Test case 6
P0,P1,P2,d1,d2,d3,P4,P5,P6,P7,P8,P9
P0,P1,P2,P3,P4,P5,P6,P7,P8,P9,P8,P9
12
P3
1
Expand Down Expand Up @@ -60,13 +60,13 @@ Test case 10
20
a4,O6
2
10,a1,a1,O3,a4,a,b,a,b,a13,a13,O15,a16,a16,O18,a19
10,a1,a1,O3,a4,a,b,a7,b,a13,a10,O15,a16,a13,O18,a19
16
a7,a7,O9,a10,a10,O12
6
10,a1,a1,O3,a4,a,b,a,b,a13,a,b,c,e,f,a16,a16,O18,a19
10,a1,a1,O3,a4,a,b,a7,b,a13,a10,b,c,a13,f,a16,a16,O18,O18
19
a13,O15
a10,O15
2

Test case 11
Expand Down
12 changes: 6 additions & 6 deletions test/Array/protoLookupWithGetters.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ write("Test case 1");

for(var i =0;i<10;i++)
{
Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), configurable: true, enumerable: true });
Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), set : function(a) {}, configurable: true, enumerable: true });
}

var arr=new Array(10);
Expand All @@ -33,7 +33,7 @@ write("Test case 2");

for(var i =0;i<10;i++)
{
Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), configurable: true, enumerable: true });
Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), set : function(a) {}, configurable: true, enumerable: true });
}

var arr=new Array(10);
Expand All @@ -54,7 +54,7 @@ write("Test case 3");
for(var i =0;i<10;i++)
{
i++;
Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), configurable: true, enumerable: true });
Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), set : function(a) {}, configurable: true, enumerable: true });
}

var arr=new Array(10);
Expand All @@ -73,7 +73,7 @@ write("");
write("Test case 4");
for(var k=0;k<10;k++)
{
Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), configurable: true, enumerable: true });
Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), set : function(a) {}, configurable: true, enumerable: true });
}
var arr=new Array(10);
var newarr=arr.splice(3,5,"d1","d2","d3")
Expand All @@ -90,7 +90,7 @@ write("");
write("Test case 5");
for(var k=0;k<10;k++)
{
Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), configurable: true, enumerable: true });
Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), set : function(a) {}, configurable: true, enumerable: true });
}
var arr=new Array(10);
var newarr=arr.splice(3,5,"d1","d2","d3")
Expand All @@ -108,7 +108,7 @@ write("");
write("Test case 6");
for(var k=0;k<10;k++)
{
Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), configurable: true, enumerable: true });
Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), set : function(a) {}, configurable: true, enumerable: true });
}
var arr=new Array(10);
var newarr=arr.splice(3,1,"d1","d2","d3")
Expand Down

0 comments on commit 8a61170

Please sign in to comment.