From 670fa86fd171670c86528fc0fad2b262ea19be74 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Fri, 27 Jul 2018 08:42:31 -0400 Subject: [PATCH] [CloneObjectIC] overwrite monomorphic/polymorphic feedback if deprecated An object with a deprecated Map which has already been cached in CloneObjectIC feedback is still a valid Map for fast cloning --- but to be consistent with other ICs, deprecated maps are ignored, and are expected to be transitioned away from. If the source object has a deprecated map, the instance is migrated. BUG=v8:7611, chromium:867958 R=jkummerow@chromium.org, mvstanton@chromium.org Change-Id: I9771b00400fb4dda45a62e874a31d9b50630d847 Reviewed-on: https://chromium-review.googlesource.com/1152414 Reviewed-by: Jakob Kummerow Commit-Queue: Caitlin Potter Cr-Commit-Position: refs/heads/master@{#54758} --- src/feedback-vector.cc | 11 +++++++---- src/ic/ic.cc | 3 +++ test/mjsunit/es9/regress/regress-867958.js | 13 +++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 test/mjsunit/es9/regress/regress-867958.js diff --git a/src/feedback-vector.cc b/src/feedback-vector.cc index 41a6c49c63..ec156ba8e8 100644 --- a/src/feedback-vector.cc +++ b/src/feedback-vector.cc @@ -756,13 +756,14 @@ void FeedbackNexus::ConfigureCloneObject(Handle source_map, SetFeedbackExtra(*result_map); break; case MONOMORPHIC: - if (maybe_feedback->IsClearedWeakHeapObject()) { + if (maybe_feedback->IsClearedWeakHeapObject() || + Map::cast(*feedback)->is_deprecated()) { // Remain in MONOMORPHIC state if previous feedback has been collected. SetFeedback(HeapObjectReference::Weak(*source_map)); SetFeedbackExtra(*result_map); } else { // Transition to POLYMORPHIC. - DCHECK_NE(*source_map, *feedback); + DCHECK(!source_map.is_identical_to(feedback)); Handle array = EnsureArrayOfSize(2 * kCloneObjectPolymorphicEntrySize); array->Set(0, maybe_feedback); @@ -778,10 +779,12 @@ void FeedbackNexus::ConfigureCloneObject(Handle source_map, Handle array = Handle::cast(feedback); int i = 0; for (; i < array->length(); i += kCloneObjectPolymorphicEntrySize) { - if (array->Get(i)->IsClearedWeakHeapObject()) { + MaybeObjectHandle feedback(array->Get(i), isolate); + if (feedback->IsClearedWeakHeapObject() || + Map::cast(*feedback.object())->is_deprecated()) { break; } - DCHECK_NE(array->Get(i)->GetHeapObject(), *source_map); + DCHECK(!feedback.object().is_identical_to(source_map)); } if (i >= array->length()) { diff --git a/src/ic/ic.cc b/src/ic/ic.cc index 20d178c9b2..53e9ab7962 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -2549,6 +2549,9 @@ RUNTIME_FUNCTION(Runtime_CloneObjectIC_Miss) { DCHECK_EQ(4, args.length()); Handle source = args.at(0); int flags = args.smi_at(1); + + MigrateDeprecated(source); + FeedbackSlot slot = FeedbackVector::ToSlot(args.smi_at(2)); Handle vector = args.at(3); diff --git a/test/mjsunit/es9/regress/regress-867958.js b/test/mjsunit/es9/regress/regress-867958.js new file mode 100644 index 0000000000..eca3f1e8b8 --- /dev/null +++ b/test/mjsunit/es9/regress/regress-867958.js @@ -0,0 +1,13 @@ +// Copyright 2018 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. + +// Check that encountering deprecated Maps does not cause CloneObjectIC to +// crash. +var obj1 = { x: 1 }; +var obj2 = { x: 2 }; // same map +obj2.x = null; // deprecate map + +function f() { return { ...obj1 } }; +assertEquals({ x: 1 }, f()); // missed, object migrated to cached new map +assertEquals({ x: 1 }, f()); // monomorphic cache-hit