[ic] Check Double representation on store

For stores with Double feedback, StoreIC needs to check that the
representation is still Double before doing the store, in case it
accidentally tries to write to an object or worse, mutate a non-mutable
HeapNumber.

Bug: v8:9606
Bug: chromium:997485
Change-Id: I51e0953b40f752648c5e86b8644c23baf636367e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1768373
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63402}

src/codegen/code-stub-assembler.h

@@ -2731,6 +2731,16 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
     return Word32Equal(Word32And(word32, const_mask), const_mask);
   }
 
+  // Returns true if the bit field |BitField| in |word32| is equal to a given.
+  // constant |value|. Avoids a shift compared to using DecodeWord32.
+  template <typename BitField>
+  TNode<BoolT> IsEqualInWord32(TNode<Word32T> word32,
+                               typename BitField::FieldType value) {
+    TNode<Word32T> masked_word32 =
+        Word32And(word32, Int32Constant(BitField::kMask));
+    return Word32Equal(masked_word32, Int32Constant(BitField::encode(value)));
+  }
+
   // Returns true if any of the |T|'s bits in given |word| are set.
   template <typename T>
   TNode<BoolT> IsSetWord(SloppyTNode<WordT> word) {

src/ic/accessor-assembler.cc

@@ -16,6 +16,7 @@
 #include "src/objects/heap-number.h"
 #include "src/objects/module.h"
 #include "src/objects/objects-inl.h"
+#include "src/objects/property-details.h"
 #include "src/objects/smi.h"
 
 namespace v8 {
@@ -1663,6 +1664,24 @@ Node* AccessorAssembler::PrepareValueForStore(Node* handler_word, Node* holder,
   if (representation.IsDouble()) {
     value = TryTaggedToFloat64(value, bailout);
 
+    // We have to check that the representation is still Double. Checking the
+    // value is nor enough, as we could have transitioned to Tagged but still
+    // be holding a HeapNumber, which would no longer be allowed to be mutable.
+
+    // TODO(leszeks): We could skip the representation check in favor of a
+    // constant value check in StoreNamedField here, but then StoreNamedField
+    // would need an IsHeapNumber check in case both the representation changed
+    // and the value is no longer a HeapNumber.
+    TNode<IntPtrT> descriptor_entry =
+        Signed(DecodeWord<StoreHandler::DescriptorBits>(handler_word));
+    TNode<DescriptorArray> descriptors = LoadMapDescriptors(LoadMap(holder));
+    TNode<Uint32T> details =
+        LoadDetailsByDescriptorEntry(descriptors, descriptor_entry);
+
+    GotoIfNot(IsEqualInWord32<PropertyDetails::RepresentationField>(
+                  details, Representation::kDouble),
+              bailout);
+
   } else if (representation.IsHeapObject()) {
     GotoIf(TaggedIsSmi(value), bailout);
 

test/mjsunit/regress/regress-997485.js

+// Copyright 2019 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.
+
+// Flags: --allow-natives-syntax
+
+(function doubleToTaggedWithTaggedValueStoresCorrectly() {
+
+  function setX_Double(o) { o.x = 4.2; }
+
+  function foo() {
+    // o.x starts off as Double
+    const o = { x: 0.1 };
+
+    // Write to it a few times with setX_Double, to make sure setX_Double has
+    // Double feedback.
+    setX_Double(o);
+    setX_Double(o);
+
+    // Transition o.x to Tagged.
+    o.x = {};
+
+    // setX_Double will still have Double feedback, so make sure it works with
+    // the new Tagged representation o.x.
+    setX_Double(o);
+
+    assertEquals(o.x, 4.2);
+  }
+
+  %EnsureFeedbackVectorForFunction(setX_Double);
+  foo();
+
+})();
+
+(function doubleToTaggedWithDoubleValueDoesNotMutate() {
+
+  function setX_Double(o) { o.x = 4.2; }
+
+  function foo() {
+    // o.x starts off as Double
+    const o = { x: 0.1 };
+
+    // Write to it a few times with setX_Double, to make sure setX_Double has
+    // Double feedback.
+    setX_Double(o);
+    setX_Double(o);
+
+    // Transition o.x to Tagged.
+    o.x = {};
+
+    // Write the HeapNumber val to o.x.
+    const val = 1.25;
+    o.x = val;
+
+    // setX_Double will still have Double feedback, which expects to be able to
+    // mutate o.x's HeapNumber, so make sure it does not mutate val.
+    setX_Double(o);
+
+    assertEquals(o.x, 4.2);
+    assertNotEquals(val, 4.2);
+  }
+
+  %EnsureFeedbackVectorForFunction(setX_Double);
+  foo();
+
+})();
+
+(function doubleToTaggedWithTaggedValueStoresSmiCorrectly() {
+
+  function setX_Smi(o) { o.x = 42; }
+
+  function foo() {
+    // o.x starts off as Double
+    const o = { x: 0.1 };
+
+    // Write to it a few times with setX_Smi, to make sure setX_Smi has
+    // Double feedback.
+    setX_Smi(o);
+    setX_Smi(o);
+
+    // Transition o.x to Tagged.
+    o.x = {};
+
+    // setX_Smi will still have Double feedback, so make sure it works with
+    // the new Tagged representation o.x.
+    setX_Smi(o);
+
+    assertEquals(o.x, 42);
+  }
+
+  %EnsureFeedbackVectorForFunction(setX_Smi);
+  foo();
+
+})();
+
+(function doubleToTaggedWithSmiValueDoesNotMutate() {
+
+  function setX_Smi(o) { o.x = 42; }
+
+  function foo() {
+    // o.x starts off as Double
+    const o = { x: 0.1 };
+
+    // Write to it a few times with setX_Smi, to make sure setX_Smi has
+    // Double feedback.
+    setX_Smi(o);
+    setX_Smi(o);
+
+    // Transition o.x to Tagged.
+    o.x = {};
+
+    // Write the HeapNumber val to o.x.
+    const val = 1.25;
+    o.x = val;
+
+    // setX_Smi will still have Double feedback, which expects to be able to
+    // mutate o.x's HeapNumber, so make sure it does not mutate val.
+    setX_Smi(o);
+
+    assertEquals(o.x, 42);
+    assertNotEquals(val, 42);
+  }
+
+  %EnsureFeedbackVectorForFunction(setX_Smi);
+  foo();
+
+})();