[array] Remove CHECK_LE from RemoveArrayHolesGeneric

This CL removes a CHECK_LE that does not hold in all cases. After
moving all elements to the front, current_pos will point to the next
free spot. In the case where an object is 'packed', i.e. each index
has a non-undefined value, and the length is smaller then the max
index, current_pos will be greater than the length (limit in the code).

Sidenote: The block after taking the minimum (where the counted
undefineds get set) will not be affected. In the case where
num_undefined > 0, current_pos should be guaranteed to be smaller
than limit, as long there are no accessors with side-effects.

R=jgruber@chromium.org

Bug: chromium:923265
Change-Id: Id533cdc4db6c6c6f266cf7c6a8ab6ecbbeee7016
Reviewed-on: https://chromium-review.googlesource.com/c/1420679
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58912}
This commit is contained in:
Simon Zünd 2019-01-18 09:34:29 +01:00 committed by Commit Bot
parent 697885b9df
commit e38faab1c7
2 changed files with 17 additions and 5 deletions

View File

@ -139,8 +139,15 @@ Object RemoveArrayHolesGeneric(Isolate* isolate, Handle<JSReceiver> receiver,
}
}
// current_pos points to the next free space in the array/object. In most
// cases this corresponds to the 'length' or to the number of non-undefined
// elements.
// In cases where an object is 'packed' and 'length' is smaller, e.g.:
// { 0: 5, 1: 4, 2: 3, length: 2 }
// current_pos will be greater than limit, thus, we need to take the minimum.
uint32_t result = std::min(current_pos, limit);
// Set [current_pos, current_pos + num_undefined) to undefined.
uint32_t result = current_pos;
for (uint32_t i = 0; i < num_undefined; ++i) {
RETURN_FAILURE_ON_EXCEPTION(
isolate, Object::SetElement(isolate, receiver, current_pos++,
@ -162,10 +169,6 @@ Object RemoveArrayHolesGeneric(Isolate* isolate, Handle<JSReceiver> receiver,
MAYBE_RETURN(delete_result, ReadOnlyRoots(isolate).exception());
}
// The number of non-undefined elements MUST always be smaller then limit.
// Violating this may cause OOB reads/writes.
CHECK_LE(result, limit);
return *isolate->factory()->NewNumberFromUint(result);
}

View File

@ -0,0 +1,9 @@
// Copyright 2019 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
let a = {0: 5, 1: 4, 2: 3, length: 2};
Object.freeze(a);
assertThrows(() => Array.prototype.sort.call(a));
assertPropertiesEqual({0: 5, 1: 4, 2: 3, length: 2}, a);