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

[heap] Remove Factory::NewUninitializedFixedArray

Browse Source 
All existing usages are changed to Factory::NewFixedArray(). The
motivation for the removal is that the function is unsafe and easy
to misuse.

Note that NewUninitializedFixedArray has been already changed to
initialize the result as an experiment with 3%-13% regression on
a few SixSpeed microbenchmarks and no impact on larger benchmarks.

Change-Id: I2e084bc03b2636aa6d368ca255970566a7ce222e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2846895
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74186}
This commit is contained in:
Ulan Degenbaev 2021-04-26 14:53:09 +02:00 committed by Commit Bot
parent 311c022a8b
commit 764515cdb5
13 changed files with 22 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/heap/factory-base.cc
View File

@ -72,8 +72,11 @@ Handle<AccessorPair> FactoryBase<Impl>::NewAccessorPair() {
template <typename Impl>
Handle<FixedArray> FactoryBase<Impl>::NewFixedArray(int length,
AllocationType allocation) {
DCHECK_LE(0, length);
if (length == 0) return impl()->empty_fixed_array();
if (length < 0 || length > FixedArray::kMaxLength) {
FATAL("Fatal JavaScript invalid size error %d", length);
UNREACHABLE();
}
return NewFixedArrayWithFiller(
read_only_roots().fixed_array_map_handle(), length,
read_only_roots().undefined_value_handle(), allocation);

19
src/heap/factory.cc
View File

@ -411,21 +411,6 @@ MaybeHandle<FixedArray> Factory::TryNewFixedArray(
return handle(array, isolate());
}
Handle<FixedArray> Factory::NewUninitializedFixedArray(int length) {
if (length == 0) return empty_fixed_array();
if (length < 0 || length > FixedArray::kMaxLength) {
FATAL("Fatal JavaScript invalid size error %d", length);
UNREACHABLE();
}
// TODO(ulan): As an experiment this temporarily returns an initialized fixed
// array. After getting canary/performance coverage, either remove the
// function or revert to returning uninitilized array.
return NewFixedArrayWithFiller(read_only_roots().fixed_array_map_handle(),
length, undefined_value(),
AllocationType::kYoung);
}
Handle<ClosureFeedbackCellArray> Factory::NewClosureFeedbackCellArray(
int length) {
if (length == 0) return empty_closure_feedback_cell_array();
@ -1386,7 +1371,7 @@ Handle<WasmTypeInfo> Factory::NewWasmTypeInfo(Address type_address,
Handle<ArrayList> subtypes = ArrayList::New(isolate(), 0);
Handle<FixedArray> supertypes;
if (opt_parent.is_null()) {
supertypes = NewUninitializedFixedArray(0);
supertypes = NewFixedArray(0);
} else {
supertypes = CopyArrayAndGrow(
handle(opt_parent->wasm_type_info().supertypes(), isolate()), 1,
@ -2369,7 +2354,7 @@ Handle<FixedArrayBase> Factory::NewJSArrayStorage(
} else {
DCHECK(IsSmiOrObjectElementsKind(elements_kind));
if (mode == DONT_INITIALIZE_ARRAY_ELEMENTS) {
elms = NewUninitializedFixedArray(capacity);
elms = NewFixedArray(capacity);
} else {
DCHECK(mode == INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
elms = NewFixedArrayWithHoles(capacity);

3
src/heap/factory.h
View File

@ -131,9 +131,6 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
MaybeHandle<FixedArray> TryNewFixedArray(
int length, AllocationType allocation = AllocationType::kYoung);
// Allocates an uninitialized fixed array. It must be filled by the caller.
Handle<FixedArray> NewUninitializedFixedArray(int length);
// Allocates a closure feedback cell array whose feedback cells are
// initialized with undefined values.
Handle<ClosureFeedbackCellArray> NewClosureFeedbackCellArray(int num_slots);

2
src/objects/elements.cc
View File

@ -781,7 +781,7 @@ class ElementsAccessorBase : public InternalElementsAccessor {
if (IsDoubleElementsKind(kind())) {
new_elements = isolate->factory()->NewFixedDoubleArray(capacity);
} else {
new_elements = isolate->factory()->NewUninitializedFixedArray(capacity);
new_elements = isolate->factory()->NewFixedArray(capacity);
}
int packed_size = kPackedSizeNotKnown;

3
src/objects/js-objects.cc
View File

@ -2054,8 +2054,7 @@ MaybeHandle<FixedArray> GetOwnValuesOrEntries(Isolate* isolate,
MaybeHandle<FixedArray>());
if (get_entries) {
Handle<FixedArray> entry_storage =
isolate->factory()->NewUninitializedFixedArray(2);
Handle<FixedArray> entry_storage = isolate->factory()->NewFixedArray(2);
entry_storage->set(0, *key);
entry_storage->set(1, *value);
value = isolate->factory()->NewJSArrayWithElements(entry_storage,

6
src/objects/objects-inl.h
View File

@ -1096,8 +1096,7 @@ static inline uint32_t ObjectAddressForHashing(Address object) {
static inline Handle<Object> MakeEntryPair(Isolate* isolate, size_t index,
Handle<Object> value) {
Handle<Object> key = isolate->factory()->SizeToString(index);
Handle<FixedArray> entry_storage =
isolate->factory()->NewUninitializedFixedArray(2);
Handle<FixedArray> entry_storage = isolate->factory()->NewFixedArray(2);
{
entry_storage->set(0, *key, SKIP_WRITE_BARRIER);
entry_storage->set(1, *value, SKIP_WRITE_BARRIER);
@ -1108,8 +1107,7 @@ static inline Handle<Object> MakeEntryPair(Isolate* isolate, size_t index,
static inline Handle<Object> MakeEntryPair(Isolate* isolate, Handle<Object> key,
Handle<Object> value) {
Handle<FixedArray> entry_storage =
isolate->factory()->NewUninitializedFixedArray(2);
Handle<FixedArray> entry_storage = isolate->factory()->NewFixedArray(2);
{
entry_storage->set(0, *key, SKIP_WRITE_BARRIER);
entry_storage->set(1, *value, SKIP_WRITE_BARRIER);

3
src/objects/objects.cc
View File

@ -3958,8 +3958,7 @@ Handle<FixedArray> FixedArray::SetAndGrow(Isolate* isolate,
do {
capacity = JSObject::NewElementsCapacity(capacity);
} while (capacity <= index);
Handle<FixedArray> new_array =
isolate->factory()->NewUninitializedFixedArray(capacity);
Handle<FixedArray> new_array = isolate->factory()->NewFixedArray(capacity);
array->CopyTo(0, *new_array, 0, array->length());
new_array->FillWithHoles(array->length(), new_array->length());
new_array->set(index, *value);

2
src/runtime/runtime-regexp.cc
View File

@ -1696,7 +1696,7 @@ RUNTIME_FUNCTION(Runtime_RegExpSplit) {
if (!result->IsNull(isolate)) return *factory->NewJSArray(0);
Handle<FixedArray> elems = factory->NewUninitializedFixedArray(1);
Handle<FixedArray> elems = factory->NewFixedArray(1);
elems->set(0, *string);
return *factory->NewJSArrayWithElements(elems);
}

2
src/runtime/runtime-scopes.cc
View File

@ -510,7 +510,7 @@ RUNTIME_FUNCTION(Runtime_NewStrictArguments) {
isolate->factory()->NewArgumentsObject(callee, argument_count);
if (argument_count) {
Handle<FixedArray> array =
isolate->factory()->NewUninitializedFixedArray(argument_count);
isolate->factory()->NewFixedArray(argument_count);
DisallowGarbageCollection no_gc;
WriteBarrierMode mode = array->GetWriteBarrierMode(no_gc);
for (int i = 0; i < argument_count; i++) {

2
src/runtime/runtime-strings.cc
View File

@ -315,7 +315,7 @@ RUNTIME_FUNCTION(Runtime_StringToArray) {
int position = 0;
if (s->IsFlat() && s->IsOneByteRepresentation()) {
// Try using cached chars where possible.
elements = isolate->factory()->NewUninitializedFixedArray(length);
elements = isolate->factory()->NewFixedArray(length);
DisallowGarbageCollection no_gc;
String::FlatContent content = s->GetFlatContent(no_gc);

2
src/wasm/module-instantiate.cc
View File

@ -615,7 +615,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
// list.
//--------------------------------------------------------------------------
if (enabled_.has_gc()) {
Handle<FixedArray> maps = isolate_->factory()->NewUninitializedFixedArray(
Handle<FixedArray> maps = isolate_->factory()->NewFixedArray(
static_cast<int>(module_->type_kinds.size()));
for (int map_index = 0;
map_index < static_cast<int>(module_->type_kinds.size());

10
test/cctest/test-api.cc
View File

@ -26224,8 +26224,9 @@ TEST(DynamicImport) {
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
i::Handle<i::Script> referrer = i_isolate->factory()->NewScript(source);
referrer->set_name(*url);
referrer->set_host_defined_options(*i_isolate->factory()->NewFixedArray(
kCustomHostDefinedOptionsLengthForTesting));
i::Handle<i::FixedArray> options = i_isolate->factory()->NewFixedArray(
kCustomHostDefinedOptionsLengthForTesting);
referrer->set_host_defined_options(*options);
i::MaybeHandle<i::JSPromise> maybe_promise =
i_isolate->RunHostImportModuleDynamicallyCallback(
referrer, specifier, i::MaybeHandle<i::Object>());
@ -26310,8 +26311,9 @@ TEST(DynamicImportWithAssertions) {
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
i::Handle<i::Script> referrer = i_isolate->factory()->NewScript(source);
referrer->set_name(*url);
referrer->set_host_defined_options(*i_isolate->factory()->NewFixedArray(
kCustomHostDefinedOptionsLengthForTesting));
i::Handle<i::FixedArray> options = i_isolate->factory()->NewFixedArray(
kCustomHostDefinedOptionsLengthForTesting);
referrer->set_host_defined_options(*options);
i::MaybeHandle<i::JSPromise> maybe_promise =
i_isolate->RunHostImportModuleDynamicallyCallback(referrer, specifier,
i_import_assertions);

2
test/unittests/regress/regress-crbug-1041240-unittest.cc
View File

@ -12,7 +12,7 @@ namespace internal {
using NewUninitializedFixedArrayTest = TestWithIsolateAndZone;
TEST_F(NewUninitializedFixedArrayTest, ThrowOnNegativeLength) {
ASSERT_DEATH_IF_SUPPORTED({ factory()->NewUninitializedFixedArray(-1); },
ASSERT_DEATH_IF_SUPPORTED({ factory()->NewFixedArray(-1); },
"Fatal JavaScript invalid size error -1");
}