350884: KeyedStoreIC miss didn't handle a transitioning case.

It's possible to get a transitioned map with no links to the origin
map if it's a shared map. Code in KeyedStoreIC::StoreElementStub
assumes it can check if two maps are in the same family by
traversing the transition array. Long term, the "family" relationship
should be recognized with the Normalized Map Cache. For now, allow
the IC to remain monomorphic in this case if the receiver map and
the previous receiver map are the same.

Filed V8 issue 3210 (https://code.google.com/p/v8/issues/detail?id=3210)
to track the issue with the Normalized Map Cache.

BUG=350884
LOG=N
R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/194623005

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19847 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/ic.cc b/src/ic.cc
index a70ac1c7760dcd2690e35fcd02ded9e5b2df62e1..9e30aeaf3a77e8c9fda86b37b02b5b23dac0d40d 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1435,19 +1435,19 @@ Handle<Code> KeyedStoreIC::StoreElementStub(Handle<JSObject> receiver,
       KeyedStoreIC::GetKeyedAccessStoreMode(target()->extra_ic_state());
   Handle<Map> previous_receiver_map = target_receiver_maps.at(0);
   if (state() == MONOMORPHIC) {
-      // If the "old" and "new" maps are in the same elements map family, stay
-      // MONOMORPHIC and use the map for the most generic ElementsKind.
-    Handle<Map> transitioned_receiver_map = receiver_map;
     if (IsTransitionStoreMode(store_mode)) {
-      transitioned_receiver_map =
+      // If the "old" and "new" maps are in the same elements map family, or
+      // if they at least come from the same origin for a transitioning store,
+      // stay MONOMORPHIC and use the map for the most generic ElementsKind.
+      Handle<Map> transitioned_receiver_map =
           ComputeTransitionedMap(receiver, store_mode);
-    }
-    if (IsTransitionOfMonomorphicTarget(
-            MapToType<HeapType>(transitioned_receiver_map, isolate()))) {
-      // Element family is the same, use the "worst" case map.
-      store_mode = GetNonTransitioningStoreMode(store_mode);
-      return isolate()->stub_cache()->ComputeKeyedStoreElement(
-          transitioned_receiver_map, strict_mode(), store_mode);
+      if (*previous_receiver_map == receiver->map() ||
+          IsTransitionOfMonomorphicTarget(
+              MapToType<HeapType>(transitioned_receiver_map, isolate()))) {
+        store_mode = GetNonTransitioningStoreMode(store_mode);
+        return isolate()->stub_cache()->ComputeKeyedStoreElement(
+            transitioned_receiver_map, strict_mode(), store_mode);
+      }
     } else if (*previous_receiver_map == receiver->map() &&
                old_store_mode == STANDARD_STORE &&
                (IsGrowStoreMode(store_mode) ||

diff --git a/test/mjsunit/regress/regress-350884.js b/test/mjsunit/regress/regress-350884.js
new file mode 100644 (file)
index 0000000..8656853
--- /dev/null
+++ b/test/mjsunit/regress/regress-350884.js
@@ -0,0 +1,15 @@
+// Copyright 2014 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.
+
+var obj = new Array(1);
+obj[0] = 0;
+obj[1] = 0;
+function foo(flag_index) {
+  obj[flag_index]++;
+}
+
+// Force dictionary properties on obj.
+obj[-8] = 3;
+foo(1);
+foo(2);