AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[Builtins] Array.prototype.reduce missing length check

Browse Source 
In the recent port of reduce() and reduceRight(), a check for a length
change during the loop (standard for iterating builtins) was omitted.

We did get array bounds check protection, however it didn't expose
the issue in our tests because the bounds check is against the
backing store length, not against the length in the referring JSArray.

Also added a test for reduceRight().

R=jgruber@chromium.org

Bug: chromium:937676
Change-Id: I76e22e0d71965bff84a0822b1df5dc818a00b50e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1503732
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60033}
This commit is contained in:
Mike Stanton 2019-03-05 15:22:06 +01:00 committed by Commit Bot
parent 7103c19432
commit 2222a9d67e
3 changed files with 13 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/builtins/array-reduce-right.tq
View File

@ -119,6 +119,9 @@ namespace array {
for (let k: Smi = smiLen - 1; k >= 0; k--) { for (let k: Smi = smiLen - 1; k >= 0; k--) {
fastOW.Recheck() otherwise goto Bailout(k, accumulator); fastOW.Recheck() otherwise goto Bailout(k, accumulator);
// Ensure that we haven't walked beyond a possibly updated length.
if (k >= fastOW.Get().length) goto Bailout(k, accumulator);
const value: Object = fastOW.LoadElementNoHole(k) otherwise continue; const value: Object = fastOW.LoadElementNoHole(k) otherwise continue;
if (accumulator == Hole) { if (accumulator == Hole) {
accumulator = value; accumulator = value;

3
src/builtins/array-reduce.tq
View File

@ -118,6 +118,9 @@ namespace array {
for (let k: Smi = 0; k < len; k++) { for (let k: Smi = 0; k < len; k++) {
fastOW.Recheck() otherwise goto Bailout(k, accumulator); fastOW.Recheck() otherwise goto Bailout(k, accumulator);
// Ensure that we haven't walked beyond a possibly updated length.
if (k >= fastOW.Get().length) goto Bailout(k, accumulator);
const value: Object = fastOW.LoadElementNoHole(k) otherwise continue; const value: Object = fastOW.LoadElementNoHole(k) otherwise continue;
if (accumulator == Hole) { if (accumulator == Hole) {
accumulator = value; accumulator = value;

7
test/mjsunit/array-reduce.js
View File

@ -509,6 +509,13 @@ testReduce("reduce", "ArrayManipulationShort", 3,
[1, 2, 1, [1, 2], 3], [1, 2, 1, [1, 2], 3],
], arr, manipulator, 0); ], arr, manipulator, 0);
var arr = [1, 2, 3, 4];
testReduce("reduceRight", "RightArrayManipulationShort", 7,
[[0, 4, 3, [1, 2, 3, 4], 4],
[4, 2, 1, [1, 2], 6],
[6, 1, 0, [1], 7],
], arr, manipulator, 0);
var arr = [1, 2, 3, 4, 5]; var arr = [1, 2, 3, 4, 5];
testReduce("reduce", "ArrayManipulationLonger", 10, testReduce("reduce", "ArrayManipulationLonger", 10,
[[0, 1, 0, [1, 2, 3, 4, 5], 1], [[0, 1, 0, [1, 2, 3, 4, 5], 1],