Skip to content

Commit 9c8b479

Browse files
committed
deps: cherry-pick 76cab5f from upstream V8
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 <jgruber@chromium.org> Commit-Queue: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/master@{#52786} Refs: v8/v8@76cab5f Fixes: #20278 PR-URL: #20350 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 557f570 commit 9c8b479

File tree

3 files changed

+29
-10
lines changed

3 files changed

+29
-10
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
# Reset this number to 0 on major V8 upgrades.
2929
# Increment by one for each non-official patch applied to deps/v8.
30-
'v8_embedder_string': '-node.5',
30+
'v8_embedder_string': '-node.6',
3131

3232
# Enable disassembler for `--print-code` v8 options
3333
'v8_enable_disassembler': 1,

deps/v8/src/builtins/builtins-object-gen.cc

+10-9
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
268268
object_enum_length, IntPtrConstant(kInvalidEnumCacheSentinel));
269269

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

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

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

318318
// Skip Symbols.
319-
GotoIf(IsSymbol(next_key), &loop_condition);
319+
GotoIf(IsSymbol(next_key), &next_descriptor);
320320

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

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

332333
VARIABLE(var_property_value, MachineRepresentation::kTagged,
333334
UndefinedConstant());
@@ -357,12 +358,12 @@ TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
357358
StoreFixedArrayElement(values_or_entries, var_result_index.value(),
358359
value);
359360
Increment(&var_result_index, 1);
360-
Goto(&loop_condition);
361+
Goto(&next_descriptor);
361362

362-
BIND(&loop_condition);
363+
BIND(&next_descriptor);
363364
{
364365
Increment(&var_descriptor_number, 1);
365-
Branch(IntPtrEqual(var_descriptor_number.value(), object_enum_length),
366+
Branch(IntPtrEqual(var_result_index.value(), object_enum_length),
366367
&after_loop, &loop);
367368
}
368369
}

deps/v8/test/mjsunit/es8/object-entries.js

+18
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,24 @@ function TestPropertyFilter(withWarmup) {
210210
TestPropertyFilter();
211211
TestPropertyFilter(true);
212212

213+
function TestPropertyFilter2(withWarmup) {
214+
var object = { };
215+
Object.defineProperty(object, "prop1", { value: 10 });
216+
Object.defineProperty(object, "prop2", { value: 20 });
217+
object.prop3 = 30;
218+
219+
if (withWarmup) {
220+
for (const key in object) {}
221+
}
222+
223+
values = Object.entries(object);
224+
assertEquals(1, values.length);
225+
assertEquals([
226+
[ "prop3", 30 ],
227+
], values);
228+
}
229+
TestPropertyFilter2();
230+
TestPropertyFilter2(true);
213231

214232
function TestWithProxy(withWarmup) {
215233
var obj1 = {prop1:10};

0 commit comments

Comments
 (0)