Write barrier audit: const raw_ptr()

runtime/vm/object.h

Index: runtime/vm/object.h
===================================================================
--- runtime/vm/object.h	(revision 40889)
+++ runtime/vm/object.h	(working copy)
@@ -187,7 +187,7 @@
 
 #define HEAP_OBJECT_IMPLEMENTATION(object, super)                              \
   OBJECT_IMPLEMENTATION(object, super);                                        \
-  Raw##object* raw_ptr() const {                                               \
+  const Raw##object* raw_ptr() const {                                         \
     ASSERT(raw() != null());                                                   \
     return raw()->ptr();                                                       \
   }                                                                            \
@@ -209,7 +209,7 @@
  private:  /* NOLINT */                                                        \
   object() : super() {}                                                        \
   BASE_OBJECT_IMPLEMENTATION(object, super)                                    \
-  Raw##object* raw_ptr() const {                                               \
+  const Raw##object* raw_ptr() const {                                         \
     ASSERT(raw() != null());                                                   \
     return raw()->ptr();                                                       \
   }                                                                            \
@@ -229,6 +229,10 @@
     initializeHandle(this, value);
   }
 
+  uword CompareAndSwapTags(uword old_tags, uword new_tags) const {
+    return AtomicOperations::CompareAndSwapWord(
+        &raw()->ptr()->tags_, old_tags, new_tags);
+  }
   void set_tags(intptr_t value) const {
     ASSERT(!IsNull());
     // TODO(asiva): Remove the capability of setting tags in general. The mask
@@ -239,8 +243,7 @@
     do {
       old_tags = tags;
       uword new_tags = (old_tags & ~0x0000000c) | value;
-      tags = AtomicOperations::CompareAndSwapWord(
-          &raw()->ptr()->tags_, old_tags, new_tags);
+      tags = CompareAndSwapTags(old_tags, new_tags);
     } while (tags != old_tags);
   }
   void SetCreatedFromSnapshot() const {
@@ -580,10 +583,16 @@
     return (addr >= this_addr) && (addr < (this_addr + this_size));
   }
 
-  template<typename type> void StorePointer(type* addr, type value) const {
+  // Start of field mutator guards.
+  //
+  // All writes to heap objects should ultimately pass through one of the
+  // methods below, to ensure that the write barrier is correctly applied.
+
+  template<typename type>
+  void StorePointer(const type* addr, type value) const {

[Ivan Posva, 2014/10/10 23:52:11]

type const* addr

to match the untemplated versions below.

[koda, 2014/10/11 00:26:33]

Done.

     // Ensure that this object contains the addr.
     ASSERT(Contains(reinterpret_cast<uword>(addr)));
-    *addr = value;
+    *const_cast<type*>(addr) = value;
     // Filter stores based on source and target.
     if (!value->IsHeapObject()) return;
     if (value->IsNewObject() && raw()->IsOldObject() &&
@@ -593,32 +602,75 @@
     }
   }
 
+  // Store a range of pointers [from, from + count) into [to, to + count).
+  // TODO(koda): Use this to fix Object::Clone's broken store buffer logic.
+  void StorePointers(RawObject* const* to,
+                     RawObject* const* from,
+                     intptr_t count) {
+    ASSERT(Contains(reinterpret_cast<uword>(to)));
+    if (raw()->IsNewObject()) {
+      memmove(const_cast<RawObject**>(to), from, count * kWordSize);
+    } else {
+      for (intptr_t i = 0; i < count; ++i) {
+        StorePointer(&to[i], from[i]);
+      }
+    }
+  }
+
   // Use for storing into an explicitly Smi-typed field of an object
   // (i.e., both the previous and new value are Smis).
-  void StoreSmi(RawSmi** addr, RawSmi* value) const {
+  void StoreSmi(RawSmi* const* addr, RawSmi* value) const {
     // Can't use Contains, as array length is initialized through this method.
     ASSERT(reinterpret_cast<uword>(addr) >= RawObject::ToAddr(raw()));
-    *addr = value;
+    *const_cast<RawSmi**>(addr) = value;
   }
 
+  template<typename FieldType>
+  void StoreSimd128(const FieldType* addr, simd128_value_t value) const {
+    ASSERT(Contains(reinterpret_cast<uword>(addr)));
+    value.writeTo(const_cast<FieldType*>(addr));
+  }
+
   // Needs two template arguments to allow assigning enums to fixed-size ints.
   template<typename FieldType, typename ValueType>
-  void StoreNonPointer(FieldType* addr, ValueType value) const {
+  void StoreNonPointer(const FieldType* addr, ValueType value) const {
     // Can't use Contains, as it uses tags_, which is set through this method.
     ASSERT(reinterpret_cast<uword>(addr) >= RawObject::ToAddr(raw()));
-    *addr = value;
+    *const_cast<FieldType*>(addr) = value;
   }
 
-  // Fail at link time if StoreNonPointer is called with an object pointer.
+  // Provides non-const access to non-pointer fields within the object. Such
+  // access does not need a write barrier, but it is *not* GC-safe (since the
+  // object might move), hence must be fully contained within a NoGCScope.
+  template<typename FieldType>
+  FieldType* UnsafeMutableNonPointer(const FieldType* addr) const {
+    // Allow pointers at the end of variable-length data, and disallow pointers
+    // within the header word.
+    ASSERT(Contains(reinterpret_cast<uword>(addr) - 1) &&
+           Contains(reinterpret_cast<uword>(addr) - kWordSize));
+    // At least check that there is a NoGCScope, and hope it's big enough.
+    ASSERT(Isolate::Current()->no_gc_scope_depth() > 0);
+    return const_cast<FieldType*>(addr);
+  }
+
+  // Fail at link time if StoreNonPointer or UnsafeMutableNonPointer is
+  // instantiated with an object pointer type.
 #define STORE_NON_POINTER_ILLEGAL_TYPE(type)                                   \
   template<typename ValueType>                                                 \
-  void StoreNonPointer(Raw##type** addr, ValueType value) const {              \
+  void StoreNonPointer(Raw##type* const* addr, ValueType value) const {        \
     UnimplementedMethod();                                                     \
+  }                                                                            \
+  Raw##type** UnsafeMutableNonPointer(Raw##type* const* addr) const {          \
+    UnimplementedMethod();                                                     \
+    return NULL;                                                               \
   }
+
   CLASS_LIST(STORE_NON_POINTER_ILLEGAL_TYPE);
   void UnimplementedMethod() const;
 #undef STORE_NON_POINTER_ILLEGAL_TYPE
 
+  // End of field mutator guards.
+
   RawObject* raw_;  // The raw object reference.
 
  protected:
@@ -1574,7 +1626,7 @@
 
   RawArray* instantiations() const;
   void set_instantiations(const Array& value) const;
-  RawAbstractType** TypeAddr(intptr_t index) const;
+  RawAbstractType* const* TypeAddr(intptr_t index) const;
   void SetLength(intptr_t value) const;
 
   FINAL_HEAP_OBJECT_IMPLEMENTATION(TypeArguments, Object);
@@ -3159,6 +3211,7 @@
                      int64_t deopt_id,
                      int64_t token_pos,  // Or deopt reason.
                      intptr_t try_index) const {  // Or deopt index.
+    NoGCScope no_gc;
     RawPcDescriptors::PcDescriptorRec* rec = recAt(index);
     rec->set_pc(pc);
     rec->set_kind(kind);
@@ -3224,17 +3277,24 @@
       }
     }
 
-    uword Pc() const { return descriptors_.recAt(current_ix_)->pc(); }
+    uword Pc() const {
+      NoGCScope no_gc;
+      return descriptors_.recAt(current_ix_)->pc();
+    }
     intptr_t DeoptId() const {
+      NoGCScope no_gc;
       return descriptors_.recAt(current_ix_)->deopt_id();
     }
     intptr_t TokenPos() const {
+      NoGCScope no_gc;
       return descriptors_.recAt(current_ix_)->token_pos();
     }
     intptr_t TryIndex() const {
+      NoGCScope no_gc;
       return descriptors_.recAt(current_ix_)->try_index();
     }
     RawPcDescriptors::Kind Kind() const {
+      NoGCScope no_gc;
       return descriptors_.recAt(current_ix_)->kind();
     }
 
@@ -3253,6 +3313,7 @@
 
     // Moves to record that matches kind_mask_.
     void MoveToMatching() {
+      NoGCScope no_gc;
       while (next_ix_ < descriptors_.Length()) {
         const RawPcDescriptors::PcDescriptorRec& rec =
             *descriptors_.recAt(next_ix_);
@@ -3281,7 +3342,8 @@
 
   RawPcDescriptors::PcDescriptorRec* recAt(intptr_t ix) const {
     ASSERT((0 <= ix) && (ix < Length()));
-    uint8_t* d = raw_ptr()->data() + (ix * RecordSizeInBytes());
+    uint8_t* d = UnsafeMutableNonPointer(raw_ptr()->data()) +
+        (ix * RecordSizeInBytes());
     return reinterpret_cast<RawPcDescriptors::PcDescriptorRec*>(d);
   }
 
@@ -3478,7 +3540,7 @@
   intptr_t* EntryAddr(intptr_t index, intptr_t entry_offset) const {
     ASSERT((index >=0) && (index < Length()));
     intptr_t data_index = (index * kNumberOfEntries) + entry_offset;
-    return &raw_ptr()->data()[data_index];
+    return &UnsafeMutableNonPointer(raw_ptr()->data())[data_index];
   }
 
   void SetLength(intptr_t value) const;
@@ -3899,6 +3961,7 @@
   static RawCode* FindCode(uword pc, int64_t timestamp);
 
   int32_t GetPointerOffsetAt(int index) const {
+    NoGCScope no_gc;
     return *PointerOffsetAddrAt(index);
   }
   intptr_t GetTokenIndexOfPC(uword pc) const;
@@ -3928,7 +3991,7 @@
     return raw_ptr()->entry_patch_pc_offset_;
   }
   void set_entry_patch_pc_offset(intptr_t pc) const {
-    raw_ptr()->entry_patch_pc_offset_ = pc;
+    StoreNonPointer(&raw_ptr()->entry_patch_pc_offset_, pc);
   }
 
 
@@ -3936,7 +3999,7 @@
     return raw_ptr()->patch_code_pc_offset_;
   }
   void set_patch_code_pc_offset(intptr_t pc) const {
-    raw_ptr()->patch_code_pc_offset_ = pc;
+    StoreNonPointer(&raw_ptr()->patch_code_pc_offset_, pc);
   }
 
 
@@ -3944,7 +4007,7 @@
     return raw_ptr()->lazy_deopt_pc_offset_;
   }
   void set_lazy_deopt_pc_offset(intptr_t pc) const {
-    raw_ptr()->lazy_deopt_pc_offset_ = pc;
+    StoreNonPointer(&raw_ptr()->lazy_deopt_pc_offset_, pc);
   }
 
  private:
@@ -4002,9 +4065,10 @@
     ASSERT(index >= 0);
     ASSERT(index < pointer_offsets_length());
     // TODO(iposva): Unit test is missing for this functionality.
-    return &raw_ptr()->data()[index];
+    return &UnsafeMutableNonPointer(raw_ptr()->data())[index];
   }
   void SetPointerOffsetAt(int index, int32_t offset_in_instructions) {
+    NoGCScope no_gc;
     *PointerOffsetAddrAt(index) = offset_in_instructions;
   }
 
@@ -4072,7 +4136,7 @@
                          Heap::Space space = Heap::kNew);
 
  private:
-  RawInstance** InstanceAddr(intptr_t context_index) const {
+  RawInstance* const* InstanceAddr(intptr_t context_index) const {
     ASSERT((context_index >= 0) && (context_index < num_variables()));
     return &raw_ptr()->data()[context_index];
   }
@@ -5803,10 +5867,12 @@
 class OneByteString : public AllStatic {
  public:
   static uint16_t CharAt(const String& str, intptr_t index) {
+    NoGCScope no_gc;
     return *CharAddr(str, index);
   }
 
   static void SetCharAt(const String& str, intptr_t index, uint8_t code_unit) {
+    NoGCScope no_gc;
     *CharAddr(str, index) = code_unit;
   }
   static RawOneByteString* EscapeSpecialCharacters(const String& str);
@@ -5905,15 +5971,14 @@
     return reinterpret_cast<RawOneByteString*>(str.raw());
   }
 
-  static RawOneByteString* raw_ptr(const String& str) {
-    return reinterpret_cast<RawOneByteString*>(str.raw_ptr());
+  static const RawOneByteString* raw_ptr(const String& str) {
+    return reinterpret_cast<const RawOneByteString*>(str.raw_ptr());
   }
 
   static uint8_t* CharAddr(const String& str, intptr_t index) {
     ASSERT((index >= 0) && (index < str.Length()));
     ASSERT(str.IsOneByteString());
-    NoGCScope no_gc;
-    return &raw_ptr(str)->data()[index];
+    return &str.UnsafeMutableNonPointer(raw_ptr(str)->data())[index];
   }
 
   static RawOneByteString* ReadFrom(SnapshotReader* reader,
@@ -5932,10 +5997,12 @@
 class TwoByteString : public AllStatic {
  public:
   static uint16_t CharAt(const String& str, intptr_t index) {
+    NoGCScope no_gc;
     return *CharAddr(str, index);
   }
 
   static void SetCharAt(const String& str, intptr_t index, uint16_t ch) {
+    NoGCScope no_gc;
     *CharAddr(str, index) = ch;
   }
 
@@ -6007,15 +6074,14 @@
     return reinterpret_cast<RawTwoByteString*>(str.raw());
   }
 
-  static RawTwoByteString* raw_ptr(const String& str) {
-    return reinterpret_cast<RawTwoByteString*>(str.raw_ptr());
+  static const RawTwoByteString* raw_ptr(const String& str) {
+    return reinterpret_cast<const RawTwoByteString*>(str.raw_ptr());
   }
 
   static uint16_t* CharAddr(const String& str, intptr_t index) {
     ASSERT((index >= 0) && (index < str.Length()));
     ASSERT(str.IsTwoByteString());
-    NoGCScope no_gc;
-    return &raw_ptr(str)->data()[index];
+    return &str.UnsafeMutableNonPointer(raw_ptr(str)->data())[index];
   }
 
   static RawTwoByteString* ReadFrom(SnapshotReader* reader,
@@ -6032,6 +6098,7 @@
 class ExternalOneByteString : public AllStatic {
  public:
   static uint16_t CharAt(const String& str, intptr_t index) {
+    NoGCScope no_gc;
     return *CharAddr(str, index);
   }
 
@@ -6068,21 +6135,19 @@
     return reinterpret_cast<RawExternalOneByteString*>(str.raw());
   }
 
-  static RawExternalOneByteString* raw_ptr(const String& str) {
-    return reinterpret_cast<RawExternalOneByteString*>(str.raw_ptr());
+  static const RawExternalOneByteString* raw_ptr(const String& str) {
+    return reinterpret_cast<const RawExternalOneByteString*>(str.raw_ptr());
   }
 
   static const uint8_t* CharAddr(const String& str, intptr_t index) {
     ASSERT((index >= 0) && (index < str.Length()));
     ASSERT(str.IsExternalOneByteString());
-    NoGCScope no_gc;
     return &(raw_ptr(str)->external_data_->data()[index]);
   }
 
   static void SetExternalData(const String& str,
                               ExternalStringData<uint8_t>* data) {
     ASSERT(str.IsExternalOneByteString());
-    NoGCScope no_gc;
     str.StoreNonPointer(&raw_ptr(str)->external_data_, data);
   }
 
@@ -6109,6 +6174,7 @@
 class ExternalTwoByteString : public AllStatic {
  public:
   static uint16_t CharAt(const String& str, intptr_t index) {
+    NoGCScope no_gc;
     return *CharAddr(str, index);
   }
 
@@ -6141,21 +6207,19 @@
     return reinterpret_cast<RawExternalTwoByteString*>(str.raw());
   }
 
-  static RawExternalTwoByteString* raw_ptr(const String& str) {
-    return reinterpret_cast<RawExternalTwoByteString*>(str.raw_ptr());
+  static const RawExternalTwoByteString* raw_ptr(const String& str) {
+    return reinterpret_cast<const RawExternalTwoByteString*>(str.raw_ptr());
   }
 
   static const uint16_t* CharAddr(const String& str, intptr_t index) {
     ASSERT((index >= 0) && (index < str.Length()));
     ASSERT(str.IsExternalTwoByteString());
-    NoGCScope no_gc;
     return &(raw_ptr(str)->external_data_->data()[index]);
   }
 
   static void SetExternalData(const String& str,
                               ExternalStringData<uint16_t>* data) {
     ASSERT(str.IsExternalTwoByteString());
-    NoGCScope no_gc;
     str.StoreNonPointer(&raw_ptr(str)->external_data_, data);
   }
 
@@ -6310,7 +6374,7 @@
                        Heap::Space space = Heap::kNew);
 
  private:
-  RawObject** ObjectAddr(intptr_t index) const {
+  RawObject* const* ObjectAddr(intptr_t index) const {
     // TODO(iposva): Determine if we should throw an exception here.
     ASSERT((index >= 0) && (index < Length()));
     return &raw_ptr()->data()[index];
@@ -6650,16 +6714,19 @@
   void* DataAddr(intptr_t byte_offset) const {
     ASSERT((byte_offset == 0) ||
            ((byte_offset > 0) && (byte_offset < LengthInBytes())));
-    return reinterpret_cast<void*>(raw_ptr()->data() + byte_offset);
+    return reinterpret_cast<void*>(
+        UnsafeMutableNonPointer(raw_ptr()->data()) + byte_offset);
   }
 
   virtual bool CanonicalizeEquals(const Instance& other) const;
 
 #define TYPED_GETTER_SETTER(name, type)                                        \
   type Get##name(intptr_t byte_offset) const {                                 \
+    NoGCScope no_gc;                                                           \
     return *reinterpret_cast<type*>(DataAddr(byte_offset));                    \
   }                                                                            \
   void Set##name(intptr_t byte_offset, type value) const {                     \
+    NoGCScope no_gc;                                                           \
     *reinterpret_cast<type*>(DataAddr(byte_offset)) = value;                   \
   }
   TYPED_GETTER_SETTER(Int8, int8_t)
@@ -6907,18 +6974,20 @@
 
   static RawInstance* Data(const Instance& view_obj) {
     ASSERT(!view_obj.IsNull());
-    return *reinterpret_cast<RawInstance**>(view_obj.raw_ptr() + kDataOffset);
+    return *reinterpret_cast<RawInstance* const*>(
+        view_obj.raw_ptr() + kDataOffset);
   }
 
   static RawSmi* OffsetInBytes(const Instance& view_obj) {
     ASSERT(!view_obj.IsNull());
-    return *reinterpret_cast<RawSmi**>(
+    return *reinterpret_cast<RawSmi* const*>(
         view_obj.raw_ptr() + kOffsetInBytesOffset);
   }
 
   static RawSmi* Length(const Instance& view_obj) {
     ASSERT(!view_obj.IsNull());
-    return *reinterpret_cast<RawSmi**>(view_obj.raw_ptr() + kLengthOffset);
+    return *reinterpret_cast<RawSmi* const*>(
+        view_obj.raw_ptr() + kLengthOffset);
   }
 
   static bool IsExternalTypedDataView(const Instance& view_obj) {
@@ -6965,7 +7034,8 @@
  public:
   static RawInstance* Data(const Instance& view_obj) {
     ASSERT(!view_obj.IsNull());
-    return *reinterpret_cast<RawInstance**>(view_obj.raw_ptr() + kDataOffset);
+    return *reinterpret_cast<RawInstance* const*>(
+        view_obj.raw_ptr() + kDataOffset);
   }
 
   static intptr_t NumberOfFields() {