[vm, gc] Remove data race updating TypedDataView::pointer_ during parallel scavenge.

TEST=tsan

Bug: https://github.com/dart-lang/sdk/issues/45217
Change-Id: I0045b793767b8397c62ce992cc775eff989e8642
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/189800
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

runtime/vm/heap/scavenger.cc

@@ -124,12 +124,28 @@ class ScavengerVisitorBase : public ObjectPointerVisitor {
   virtual void VisitTypedDataViewPointers(TypedDataViewPtr view,
                                           ObjectPtr* first,
                                           ObjectPtr* last) {
-    // First we forward all fields of the typed data view.
+    // TypedDataViews require extra processing to update their
+    // PointerBase::data_ pointer. If the underlying typed data is external, no
+    // update is needed. If the underlying typed data is internal, the pointer
+    // must be updated if the typed data was copied or promoted. We cannot
+    // safely dereference the underlying typed data to make this distinction.
+    // It may have been forwarded by a different scavanger worker, so the access
+    // could have a data race. Rather than checking the CID of the underlying
+    // typed data, which requires dereferencing the copied/promoted header, we
+    // compare the view's internal pointer to what it should be if the
+    // underlying typed data was internal, and assume that external typed data
+    // never points into the Dart heap. We must do this before VisitPointers
+    // because we want to compare the old pointer and old typed data.
+    const bool is_external =
+        view->untag()->data_ != view->untag()->DataFieldForInternalTypedData();
+
+    // Forward all fields of the typed data view.
     VisitPointers(first, last);
 
     if (view->untag()->data_ == nullptr) {
       ASSERT(RawSmiValue(view->untag()->offset_in_bytes_) == 0 &&
              RawSmiValue(view->untag()->length_) == 0);
+      ASSERT(is_external);
       return;
     }
 
@@ -146,17 +162,21 @@ class ScavengerVisitorBase : public ObjectPointerVisitor {
         *reinterpret_cast<uword*>(UntaggedObject::ToAddr(td));
     ASSERT(!IsForwarding(td_header) || td->IsOldObject());
 
-    // We can always obtain the class id from the forwarded backing store.
-    const classid_t cid = td->GetClassId();
+    if (!parallel) {
+      // When there is only one worker, there is no data race.
+      ASSERT_EQUAL(IsExternalTypedDataClassId(td->GetClassId()), is_external);
+    }
 
     // If we have external typed data we can simply return since the backing
     // store lives in C-heap and will not move.
-    if (IsExternalTypedDataClassId(cid)) {
+    if (is_external) {
       return;
     }
 
     // Now we update the inner pointer.
-    ASSERT(IsTypedDataClassId(cid));
+    if (!parallel) {
+      ASSERT(IsTypedDataClassId(td->GetClassId()));
+    }
     view->untag()->RecomputeDataFieldForInternalTypedData();
   }
 

runtime/vm/raw_object.h

@@ -2646,18 +2646,22 @@ class UntaggedTypedDataView : public UntaggedTypedDataBase {
     data_ = payload + offset_in_bytes;
   }
 
-  // Recopute [data_] based on internal [typed_data_] - needs to be called by GC
-  // whenever the backing store moved.
+  // Recompute [data_] based on internal [typed_data_] - needs to be called by
+  // GC whenever the backing store moved.
   //
   // NOTICE: This method assumes [this] is the forwarded object and the
   // [typed_data_] pointer points to the new backing store. The backing store's
   // fields don't need to be valid - only it's address.
   void RecomputeDataFieldForInternalTypedData() {
+    data_ = DataFieldForInternalTypedData();
+  }
+
+  uint8_t* DataFieldForInternalTypedData() const {
     const intptr_t offset_in_bytes = RawSmiValue(offset_in_bytes_);
     uint8_t* payload =
         reinterpret_cast<uint8_t*>(UntaggedObject::ToAddr(typed_data()) +
                                    UntaggedTypedData::payload_offset());
-    data_ = payload + offset_in_bytes;
+    return payload + offset_in_bytes;
   }
 
   void ValidateInnerPointer() {