Use synchronized accessors for Map::layout_descriptor field.

This fixes a potential data race that can happen when a freshly
allocated layout descriptor is set on an existing map while the
concurrent marker is using the map to visit an object.

Change-Id: If8ff1c9368b24beb15fefe4a78a21e0f0784d2e8
Reviewed-on: https://chromium-review.googlesource.com/c/1278726Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56604}

src/objects/map-inl.h

@@ -29,8 +29,13 @@ namespace internal {
 CAST_ACCESSOR(Map)
 
 ACCESSORS(Map, instance_descriptors, DescriptorArray, kDescriptorsOffset)
-ACCESSORS_CHECKED(Map, layout_descriptor, LayoutDescriptor,
-                  kLayoutDescriptorOffset, FLAG_unbox_double_fields)
+// A freshly allocated layout descriptor can be set on an existing map.
+// We need to use release-store and acquire-load accessor pairs to ensure
+// that the concurrent marking thread observes initializing stores of the
+// layout descriptor.
+SYNCHRONIZED_ACCESSORS_CHECKED(Map, layout_descriptor, LayoutDescriptor,
+                               kLayoutDescriptorOffset,
+                               FLAG_unbox_double_fields)
 WEAK_ACCESSORS(Map, raw_transitions, kTransitionsOrPrototypeInfoOffset)
 
 // |bit_field| fields.
@@ -546,12 +551,17 @@ void Map::set_prototype(Object* value, WriteBarrierMode mode) {
 
 LayoutDescriptor* Map::layout_descriptor_gc_safe() const {
   DCHECK(FLAG_unbox_double_fields);
-  Object* layout_desc = RELAXED_READ_FIELD(this, kLayoutDescriptorOffset);
+  // The loaded value can be dereferenced on background thread to load the
+  // bitmap. We need acquire load in order to ensure that the bitmap
+  // initializing stores are also visible to the background thread.
+  Object* layout_desc = ACQUIRE_READ_FIELD(this, kLayoutDescriptorOffset);
   return LayoutDescriptor::cast_gc_safe(layout_desc);
 }
 
 bool Map::HasFastPointerLayout() const {
   DCHECK(FLAG_unbox_double_fields);
+  // The loaded value is used for SMI check only and is not dereferenced,
+  // so relaxed load is safe.
   Object* layout_desc = RELAXED_READ_FIELD(this, kLayoutDescriptorOffset);
   return LayoutDescriptor::IsFastPointerLayout(layout_desc);
 }

src/objects/object-macros.h

@@ -97,6 +97,26 @@
 #define ACCESSORS(holder, name, type, offset) \
   ACCESSORS_CHECKED(holder, name, type, offset, true)
 
+#define SYNCHRONIZED_ACCESSORS_CHECKED2(holder, name, type, offset,   \
+                                        get_condition, set_condition) \
+  type* holder::name() const {                                        \
+    type* value = type::cast(ACQUIRE_READ_FIELD(this, offset));       \
+    DCHECK(get_condition);                                            \
+    return value;                                                     \
+  }                                                                   \
+  void holder::set_##name(type* value, WriteBarrierMode mode) {       \
+    DCHECK(set_condition);                                            \
+    RELEASE_WRITE_FIELD(this, offset, value);                         \
+    CONDITIONAL_WRITE_BARRIER(this, offset, value, mode);             \
+  }
+
+#define SYNCHRONIZED_ACCESSORS_CHECKED(holder, name, type, offset, condition) \
+  SYNCHRONIZED_ACCESSORS_CHECKED2(holder, name, type, offset, condition,      \
+                                  condition)
+
+#define SYNCHRONIZED_ACCESSORS(holder, name, type, offset) \
+  SYNCHRONIZED_ACCESSORS_CHECKED(holder, name, type, offset, true)
+
 #define WEAK_ACCESSORS_CHECKED2(holder, name, offset, get_condition,   \
                                 set_condition)                         \
   MaybeObject* holder::name() const {                                  \