Skip to content
Permalink
Browse files

deps: cherry-pick 88f8fe1 from upstream V8

Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

[The backport to v10.x resolves merge conflicts due to a different way
of accessing the “hole” value in V8, different signatures of the
`Handle` constructor and the `Shrink()` method, and neighbouring-line
conflicts in the test file.]

Refs: v8/v8@88f8fe1
Fixes: #27882

Backport-PR-URL: #27894
PR-URL: #24514
Refs: #24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information...
hashseed authored and BethGriggs committed Nov 20, 2018
1 parent cc3ca08 commit 8f780e8f99742e4a2d3a482033de42ee3fe85637
Showing with 246 additions and 28 deletions.
  1. +1 −1 common.gypi
  2. +31 −27 deps/v8/src/api.cc
  3. +214 −0 deps/v8/test/cctest/test-api.cc
@@ -33,7 +33,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.52',
'v8_embedder_string': '-node.53',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
@@ -7143,31 +7143,32 @@ enum class MapAsArrayKind {
i::Handle<i::JSArray> MapAsArray(i::Isolate* isolate, i::Object* table_obj,
int offset, MapAsArrayKind kind) {
i::Factory* factory = isolate->factory();
i::Handle<i::OrderedHashMap> table(i::OrderedHashMap::cast(table_obj));
if (offset >= table->NumberOfElements()) return factory->NewJSArray(0);
int length = (table->NumberOfElements() - offset) *
(kind == MapAsArrayKind::kEntries ? 2 : 1);
i::Handle<i::FixedArray> result = factory->NewFixedArray(length);
i::Handle<i::OrderedHashMap> table(i::OrderedHashMap::cast(table_obj),
isolate);
const bool collect_keys =
kind == MapAsArrayKind::kEntries || kind == MapAsArrayKind::kKeys;
const bool collect_values =
kind == MapAsArrayKind::kEntries || kind == MapAsArrayKind::kValues;
int capacity = table->UsedCapacity();
int max_length =
(capacity - offset) * ((collect_keys && collect_values) ? 2 : 1);
i::Handle<i::FixedArray> result = factory->NewFixedArray(max_length);
int result_index = 0;
{
i::DisallowHeapAllocation no_gc;
int capacity = table->UsedCapacity();
i::Oddball* the_hole = isolate->heap()->the_hole_value();
for (int i = 0; i < capacity; ++i) {
for (int i = offset; i < capacity; ++i) {
i::Object* key = table->KeyAt(i);
if (key == the_hole) continue;
if (offset-- > 0) continue;
if (kind == MapAsArrayKind::kEntries || kind == MapAsArrayKind::kKeys) {
result->set(result_index++, key);
}
if (kind == MapAsArrayKind::kEntries || kind == MapAsArrayKind::kValues) {
result->set(result_index++, table->ValueAt(i));
}
if (collect_keys) result->set(result_index++, key);
if (collect_values) result->set(result_index++, table->ValueAt(i));
}
}
DCHECK_EQ(result_index, result->length());
DCHECK_EQ(result_index, length);
return factory->NewJSArrayWithElements(result, i::PACKED_ELEMENTS, length);
DCHECK_GE(max_length, result_index);
if (result_index == 0) return factory->NewJSArray(0);
result->Shrink(result_index);
return factory->NewJSArrayWithElements(result, i::PACKED_ELEMENTS,
result_index);
}

} // namespace
@@ -7250,25 +7251,28 @@ namespace {
i::Handle<i::JSArray> SetAsArray(i::Isolate* isolate, i::Object* table_obj,
int offset) {
i::Factory* factory = isolate->factory();
i::Handle<i::OrderedHashSet> table(i::OrderedHashSet::cast(table_obj));
int length = table->NumberOfElements() - offset;
if (length <= 0) return factory->NewJSArray(0);
i::Handle<i::FixedArray> result = factory->NewFixedArray(length);
i::Handle<i::OrderedHashSet> table(i::OrderedHashSet::cast(table_obj),
isolate);
// Elements skipped by |offset| may already be deleted.
int capacity = table->UsedCapacity();
int max_length = capacity - offset;
if (max_length == 0) return factory->NewJSArray(0);
i::Handle<i::FixedArray> result = factory->NewFixedArray(max_length);
int result_index = 0;
{
i::DisallowHeapAllocation no_gc;
int capacity = table->UsedCapacity();
i::Oddball* the_hole = isolate->heap()->the_hole_value();
for (int i = 0; i < capacity; ++i) {
for (int i = offset; i < capacity; ++i) {
i::Object* key = table->KeyAt(i);
if (key == the_hole) continue;
if (offset-- > 0) continue;
result->set(result_index++, key);
}
}
DCHECK_EQ(result_index, result->length());
DCHECK_EQ(result_index, length);
return factory->NewJSArrayWithElements(result, i::PACKED_ELEMENTS, length);
DCHECK_GE(max_length, result_index);
if (result_index == 0) return factory->NewJSArray(0);
result->Shrink(result_index);
return factory->NewJSArrayWithElements(result, i::PACKED_ELEMENTS,
result_index);
}
} // namespace

@@ -28066,3 +28066,217 @@ TEST(BigIntAPI) {
CHECK_EQ(word_count, 2);
}
}

TEST(PreviewSetIteratorEntriesWithDeleted) {
LocalContext env;
v8::HandleScope handle_scope(env->GetIsolate());
v8::Local<v8::Context> context = env.local();

{
// Create set, delete entry, create iterator, preview.
v8::Local<v8::Object> iterator =
CompileRun("var set = new Set([1,2,3]); set.delete(1); set.keys()")
->ToObject(context)
.ToLocalChecked();
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(2, entries->Length());
CHECK_EQ(2, entries->Get(context, 0)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
CHECK_EQ(3, entries->Get(context, 1)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
{
// Create set, create iterator, delete entry, preview.
v8::Local<v8::Object> iterator =
CompileRun("var set = new Set([1,2,3]); set.keys()")
->ToObject(context)
.ToLocalChecked();
CompileRun("set.delete(1);");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(2, entries->Length());
CHECK_EQ(2, entries->Get(context, 0)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
CHECK_EQ(3, entries->Get(context, 1)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
{
// Create set, create iterator, delete entry, iterate, preview.
v8::Local<v8::Object> iterator =
CompileRun("var set = new Set([1,2,3]); var it = set.keys(); it")
->ToObject(context)
.ToLocalChecked();
CompileRun("set.delete(1); it.next();");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(1, entries->Length());
CHECK_EQ(3, entries->Get(context, 0)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
{
// Create set, create iterator, delete entry, iterate until empty, preview.
v8::Local<v8::Object> iterator =
CompileRun("var set = new Set([1,2,3]); var it = set.keys(); it")
->ToObject(context)
.ToLocalChecked();
CompileRun("set.delete(1); it.next(); it.next();");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(0, entries->Length());
}
{
// Create set, create iterator, delete entry, iterate, trigger rehash,
// preview.
v8::Local<v8::Object> iterator =
CompileRun("var set = new Set([1,2,3]); var it = set.keys(); it")
->ToObject(context)
.ToLocalChecked();
CompileRun("set.delete(1); it.next();");
CompileRun("for (var i = 4; i < 20; i++) set.add(i);");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(17, entries->Length());
for (uint32_t i = 0; i < 17; i++) {
CHECK_EQ(i + 3, entries->Get(context, i)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
}
}

TEST(PreviewMapIteratorEntriesWithDeleted) {
LocalContext env;
v8::HandleScope handle_scope(env->GetIsolate());
v8::Local<v8::Context> context = env.local();

{
// Create map, delete entry, create iterator, preview.
v8::Local<v8::Object> iterator = CompileRun(
"var map = new Map();"
"var key = {}; map.set(key, 1);"
"map.set({}, 2); map.set({}, 3);"
"map.delete(key);"
"map.values()")
->ToObject(context)
.ToLocalChecked();
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(2, entries->Length());
CHECK_EQ(2, entries->Get(context, 0)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
CHECK_EQ(3, entries->Get(context, 1)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
{
// Create map, create iterator, delete entry, preview.
v8::Local<v8::Object> iterator = CompileRun(
"var map = new Map();"
"var key = {}; map.set(key, 1);"
"map.set({}, 2); map.set({}, 3);"
"map.values()")
->ToObject(context)
.ToLocalChecked();
CompileRun("map.delete(key);");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(2, entries->Length());
CHECK_EQ(2, entries->Get(context, 0)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
CHECK_EQ(3, entries->Get(context, 1)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
{
// Create map, create iterator, delete entry, iterate, preview.
v8::Local<v8::Object> iterator = CompileRun(
"var map = new Map();"
"var key = {}; map.set(key, 1);"
"map.set({}, 2); map.set({}, 3);"
"var it = map.values(); it")
->ToObject(context)
.ToLocalChecked();
CompileRun("map.delete(key); it.next();");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(1, entries->Length());
CHECK_EQ(3, entries->Get(context, 0)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
{
// Create map, create iterator, delete entry, iterate until empty, preview.
v8::Local<v8::Object> iterator = CompileRun(
"var map = new Map();"
"var key = {}; map.set(key, 1);"
"map.set({}, 2); map.set({}, 3);"
"var it = map.values(); it")
->ToObject(context)
.ToLocalChecked();
CompileRun("map.delete(key); it.next(); it.next();");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(0, entries->Length());
}
{
// Create map, create iterator, delete entry, iterate, trigger rehash,
// preview.
v8::Local<v8::Object> iterator = CompileRun(
"var map = new Map();"
"var key = {}; map.set(key, 1);"
"map.set({}, 2); map.set({}, 3);"
"var it = map.values(); it")
->ToObject(context)
.ToLocalChecked();
CompileRun("map.delete(key); it.next();");
CompileRun("for (var i = 4; i < 20; i++) map.set({}, i);");
bool is_key;
v8::Local<v8::Array> entries =
iterator->PreviewEntries(&is_key).ToLocalChecked();
CHECK(!is_key);
CHECK_EQ(17, entries->Length());
for (uint32_t i = 0; i < 17; i++) {
CHECK_EQ(i + 3, entries->Get(context, i)
.ToLocalChecked()
->Int32Value(context)
.FromJust());
}
}
}

0 comments on commit 8f780e8

Please sign in to comment.
You can’t perform that action at this time.