[compiler] Use kAssumeMemoryFence in two critical ref creation spots

Using kAssumeMemoryFence works around the fact that the graph stores
handles (and not refs). The assumption is that any handle inserted
into the graph is safe to read; but we don't preserve the reason why
it is safe to read. Thus we must over-approximate here and assume the
existence of a memory fence.

Note this is only valid if all spots that insert handles into the
graph ensure that the handle can safely be read.

In the future, we should consider having the graph store ObjectRefs or
ObjectData pointer instead, which would make new ref construction here
unnecessary.

Bug: v8:7790,chromium:1209798
Change-Id: Ic22340ea9f34a24be530a3c62c8309d25e108f3f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2902742Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74653}

src/compiler/node-matchers.h

@@ -238,7 +238,14 @@ struct HeapObjectMatcherImpl final
   }
 
   HeapObjectRef Ref(JSHeapBroker* broker) const {
-    return MakeRef(broker, this->ResolvedValue());
+    // TODO(jgruber,chromium:1209798): Using kAssumeMemoryFence works around
+    // the fact that the graph stores handles (and not refs). The assumption is
+    // that any handle inserted into the graph is safe to read; but we don't
+    // preserve the reason why it is safe to read. Thus we must over-approximate
+    // here and assume the existence of a memory fence. In the future, we should
+    // consider having the graph store ObjectRefs or ObjectData pointer instead,
+    // which would make new ref construction here unnecessary.
+    return MakeRefAssumeMemoryFence(broker, this->ResolvedValue());
   }
 };
 

src/compiler/types.cc

@@ -6,6 +6,7 @@
 
 #include <iomanip>
 
+#include "src/compiler/js-heap-broker.h"
 #include "src/handles/handles-inl.h"
 #include "src/objects/instance-type.h"
 #include "src/objects/objects-inl.h"
@@ -837,7 +838,14 @@ Type Type::Constant(double value, Zone* zone) {
 }
 
 Type Type::Constant(JSHeapBroker* broker, Handle<i::Object> value, Zone* zone) {
-  ObjectRef ref(broker, value);
+  // TODO(jgruber,chromium:1209798): Using kAssumeMemoryFence works around
+  // the fact that the graph stores handles (and not refs). The assumption is
+  // that any handle inserted into the graph is safe to read; but we don't
+  // preserve the reason why it is safe to read. Thus we must over-approximate
+  // here and assume the existence of a memory fence. In the future, we should
+  // consider having the graph store ObjectRefs or ObjectData pointer instead,
+  // which would make new ref construction here unnecessary.
+  ObjectRef ref = MakeRefAssumeMemoryFence(broker, value);
   if (ref.IsSmi()) {
     return Constant(static_cast<double>(ref.AsSmi()), zone);
   }