Skip to content

Commit

Permalink
deps: cherry-pick 76cab5f from upstream V8
Browse files Browse the repository at this point in the history
Original commit message:

    Fix Object.entries/.values with non-enumerable properties

    Iterate over all descriptors instead of bailing out early and missing
    enumerable properties later.

    Bug: chromium:836145
    Change-Id: I104f7ea89480383b6b4b9204942a166bdf8e0597
    Reviewed-on: https://chromium-review.googlesource.com/1027832
    Reviewed-by: Jakob Gruber <[email protected]>
    Commit-Queue: Camillo Bruni <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#52786}

Refs: v8/v8@76cab5f
Fixes: #20278

PR-URL: #20350
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
targos authored and MylesBorins committed May 4, 2018
1 parent 42bbaa3 commit da8bc6a
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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.5',
'v8_embedder_string': '-node.6',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
19 changes: 10 additions & 9 deletions deps/v8/src/builtins/builtins-object-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
object_enum_length, IntPtrConstant(kInvalidEnumCacheSentinel));

// In case, we found enum_cache in object,
// we use it as array_length becuase it has same size for
// we use it as array_length because it has same size for
// Object.(entries/values) result array object length.
// So object_enum_length use less memory space than
// NumberOfOwnDescriptorsBits value.
Expand All @@ -285,7 +285,7 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
INTPTR_PARAMETERS, kAllowLargeObjectAllocation));

// If in case we have enum_cache,
// we can't detect accessor of object until loop through descritpros.
// we can't detect accessor of object until loop through descriptors.
// So if object might have accessor,
// we will remain invalid addresses of FixedArray.
// Because in that case, we need to jump to runtime call.
Expand All @@ -299,7 +299,7 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
Variable* vars[] = {&var_descriptor_number, &var_result_index};
// Let desc be ? O.[[GetOwnProperty]](key).
TNode<DescriptorArray> descriptors = LoadMapDescriptors(map);
Label loop(this, 2, vars), after_loop(this), loop_condition(this);
Label loop(this, 2, vars), after_loop(this), next_descriptor(this);
Branch(IntPtrEqual(var_descriptor_number.value(), object_enum_length),
&after_loop, &loop);

Expand All @@ -316,7 +316,7 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
Node* next_key = DescriptorArrayGetKey(descriptors, descriptor_index);

// Skip Symbols.
GotoIf(IsSymbol(next_key), &loop_condition);
GotoIf(IsSymbol(next_key), &next_descriptor);

TNode<Uint32T> details = TNode<Uint32T>::UncheckedCast(
DescriptorArrayGetDetails(descriptors, descriptor_index));
Expand All @@ -326,8 +326,9 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
GotoIf(IsPropertyKindAccessor(kind), if_call_runtime_with_fast_path);
CSA_ASSERT(this, IsPropertyKindData(kind));

// If desc is not undefined and desc.[[Enumerable]] is true, then
GotoIfNot(IsPropertyEnumerable(details), &loop_condition);
// If desc is not undefined and desc.[[Enumerable]] is true, then skip to
// the next descriptor.
GotoIfNot(IsPropertyEnumerable(details), &next_descriptor);

VARIABLE(var_property_value, MachineRepresentation::kTagged,
UndefinedConstant());
Expand Down Expand Up @@ -357,12 +358,12 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
StoreFixedArrayElement(values_or_entries, var_result_index.value(),
value);
Increment(&var_result_index, 1);
Goto(&loop_condition);
Goto(&next_descriptor);

BIND(&loop_condition);
BIND(&next_descriptor);
{
Increment(&var_descriptor_number, 1);
Branch(IntPtrEqual(var_descriptor_number.value(), object_enum_length),
Branch(IntPtrEqual(var_result_index.value(), object_enum_length),
&after_loop, &loop);
}
}
Expand Down
18 changes: 18 additions & 0 deletions deps/v8/test/mjsunit/es8/object-entries.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,24 @@ function TestPropertyFilter(withWarmup) {
TestPropertyFilter();
TestPropertyFilter(true);

function TestPropertyFilter2(withWarmup) {
var object = { };
Object.defineProperty(object, "prop1", { value: 10 });
Object.defineProperty(object, "prop2", { value: 20 });
object.prop3 = 30;

if (withWarmup) {
for (const key in object) {}
}

values = Object.entries(object);
assertEquals(1, values.length);
assertEquals([
[ "prop3", 30 ],
], values);
}
TestPropertyFilter2();
TestPropertyFilter2(true);

function TestWithProxy(withWarmup) {
var obj1 = {prop1:10};
Expand Down

0 comments on commit da8bc6a

Please sign in to comment.