[turbofan] Prepare broker for the next steps.

- Add a new broker mode kRetired, in which the heap can
  again be accessed.
- Change the way modes work. We now always start in kDisabled.
  If FLAG_concurrent_compiler_frontend is on, we eventually move
  to kSerializing, then to kSerialized, then to kRetired.
- Add an ObjectDataKind to ObjectData that indicates whether the
  data is just a dummy (i.e. created while broker was in kDisabled
  mode).

This also happens to fix a bug found by clusterfuzz.

Bug: v8:7790, chromium:889722
Change-Id: I38833fe7ad26d2d3efb15ba560576defb82f673a
Reviewed-on: https://chromium-review.googlesource.com/1245425
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56260}

src/compiler/js-heap-broker.cc

@@ -25,11 +25,28 @@ HEAP_BROKER_OBJECT_LIST(FORWARD_DECL)
 // TODO(neis): It would be nice to share the serialized data for read-only
 // objects.
 
+// There are three kinds of ObjectData values.
+//
+// kSmi: The underlying V8 object is a Smi and the data is an instance of the
+//   base class (ObjectData), i.e. it's basically just the handle.  Because the
+//   object is a Smi, it's safe to access the handle in order to extract the
+//   number value, and AsSmi() does exactly that.
+//
+// kSerializedHeapObject: The underlying V8 object is a HeapObject and the
+//   data is an instance of the corresponding (most-specific) subclass, e.g.
+//   JSFunctionData, which provides serialized information about the object.
+//
+// kUnserializedHeapObject: The underlying V8 object is a HeapObject and the
+//   data is an instance of the base class (ObjectData), i.e. it basically
+//   carries no information other than the handle.
+//
+enum ObjectDataKind { kSmi, kSerializedHeapObject, kUnserializedHeapObject };
+
 class ObjectData : public ZoneObject {
  public:
   ObjectData(JSHeapBroker* broker, ObjectData** storage, Handle<Object> object,
-             bool is_smi)
-      : broker_(broker), object_(object), is_smi_(is_smi) {
+             ObjectDataKind kind)
+      : broker_(broker), object_(object), kind_(kind) {
     *storage = this;
 
     broker->Trace("Creating data %p for handle %" V8PRIuPTR " (", this,
@@ -49,12 +66,13 @@ class ObjectData : public ZoneObject {
 
   JSHeapBroker* broker() const { return broker_; }
   Handle<Object> object() const { return object_; }
-  bool is_smi() const { return is_smi_; }
+  ObjectDataKind kind() const { return kind_; }
+  bool is_smi() const { return kind_ == kSmi; }
 
  private:
   JSHeapBroker* const broker_;
   Handle<Object> const object_;
-  bool const is_smi_;
+  ObjectDataKind const kind_;
 };
 
 class HeapObjectData : public ObjectData {
@@ -638,7 +656,7 @@ void AllocationSiteData::SerializeBoilerplate() {
 
 HeapObjectData::HeapObjectData(JSHeapBroker* broker, ObjectData** storage,
                                Handle<HeapObject> object)
-    : ObjectData(broker, storage, object, false),
+    : ObjectData(broker, storage, object, kSerializedHeapObject),
       boolean_value_(object->BooleanValue(broker->isolate())),
       // We have to use a raw cast below instead of AsMap() because of
       // recursion. AsMap() would call IsMap(), which accesses the
@@ -1321,10 +1339,7 @@ ObjectRef ContextRef::get(int index) const {
 }
 
 JSHeapBroker::JSHeapBroker(Isolate* isolate, Zone* zone)
-    : isolate_(isolate),
-      zone_(zone),
-      refs_(zone, kInitialRefsBucketCount),
-      mode_(FLAG_concurrent_compiler_frontend ? kSerializing : kDisabled) {
+    : isolate_(isolate), zone_(zone), refs_(zone, kInitialRefsBucketCount) {
   Trace("Constructing heap broker.\n");
 }
 
@@ -1339,21 +1354,42 @@ void JSHeapBroker::Trace(const char* format, ...) const {
   }
 }
 
+void JSHeapBroker::StartSerializing() {
+  CHECK_EQ(mode_, kDisabled);
+  Trace("Starting serialization.\n");
+  mode_ = kSerializing;
+  refs_.clear();
+  SetNativeContextRef();
+}
+
+void JSHeapBroker::StopSerializing() {
+  CHECK_EQ(mode_, kSerializing);
+  Trace("Stopping serialization.\n");
+  mode_ = kSerialized;
+}
+
+void JSHeapBroker::Retire() {
+  CHECK_EQ(mode_, kSerialized);
+  Trace("Retiring.\n");
+  mode_ = kRetired;
+}
+
 bool JSHeapBroker::SerializingAllowed() const {
   return mode() == kSerializing ||
          (!FLAG_strict_heap_broker && mode() == kSerialized);
 }
 
-void JSHeapBroker::SerializeStandardObjects() {
-  if (!native_context_.has_value()) {
-    native_context_ = NativeContextRef(this, isolate()->native_context());
-    native_context_->Serialize();
-  }
+void JSHeapBroker::SetNativeContextRef() {
+  native_context_ = NativeContextRef(this, isolate()->native_context());
+}
 
+void JSHeapBroker::SerializeStandardObjects() {
   if (mode() == kDisabled) return;
 
   TraceScope tracer(this, "JSHeapBroker::SerializeStandardObjects");
 
+  native_context().Serialize();
+
   Builtins* const b = isolate()->builtins();
   Factory* const f = isolate()->factory();
 
@@ -1473,7 +1509,7 @@ ObjectData* JSHeapBroker::GetOrCreateData(Handle<Object> object) {
     AllowHandleAllocation handle_allocation;
     AllowHandleDereference handle_dereference;
     if (object->IsSmi()) {
-      new (zone()) ObjectData(this, data_storage, object, true);
+      new (zone()) ObjectData(this, data_storage, object, kSmi);
 #define CREATE_DATA_IF_MATCH(name)                                             \
     } else if (object->Is##name()) {                                           \
       new (zone()) name##Data(this, data_storage, Handle<name>::cast(object));
@@ -2026,16 +2062,20 @@ ObjectRef::ObjectRef(JSHeapBroker* broker, Handle<Object> object) {
     case JSHeapBroker::kSerializing:
       data_ = broker->GetOrCreateData(object);
       break;
-    case JSHeapBroker::kDisabled:
+    case JSHeapBroker::kDisabled: {
       auto insertion_result = broker->refs_.insert({object.address(), nullptr});
       ObjectData** data_storage = &(insertion_result.first->second);
       if (insertion_result.second) {
         AllowHandleDereference handle_dereference;
         new (broker->zone())
-            ObjectData(broker, data_storage, object, object->IsSmi());
+            ObjectData(broker, data_storage, object,
+                       object->IsSmi() ? kSmi : kUnserializedHeapObject);
       }
       data_ = *data_storage;
       break;
+    }
+    case JSHeapBroker::kRetired:
+      UNREACHABLE();
   }
   CHECK_NOT_NULL(data_);
 }
@@ -2161,7 +2201,19 @@ Handle<Object> ObjectRef::object() const { return data()->object(); }
 
 JSHeapBroker* ObjectRef::broker() const { return data()->broker(); }
 
-ObjectData* ObjectRef::data() const { return data_; }
+ObjectData* ObjectRef::data() const {
+  switch (data_->broker()->mode()) {
+    case JSHeapBroker::kDisabled:
+      CHECK_NE(data_->kind(), kSerializedHeapObject);
+      return data_;
+    case JSHeapBroker::kSerializing:
+    case JSHeapBroker::kSerialized:
+      CHECK_NE(data_->kind(), kUnserializedHeapObject);
+      return data_;
+    case JSHeapBroker::kRetired:
+      return data_;
+  }
+}
 
 Reduction NoChangeBecauseOfMissingData(JSHeapBroker* broker,
                                        const char* function, int line) {

src/compiler/js-heap-broker.h

@@ -472,18 +472,18 @@ class InternalizedStringRef : public StringRef {
 class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
  public:
   JSHeapBroker(Isolate* isolate, Zone* zone);
+  void SetNativeContextRef();
   void SerializeStandardObjects();
 
   Isolate* isolate() const { return isolate_; }
   Zone* zone() const { return zone_; }
   NativeContextRef native_context() const { return native_context_.value(); }
 
-  enum BrokerMode { kDisabled, kSerializing, kSerialized };
+  enum BrokerMode { kDisabled, kSerializing, kSerialized, kRetired };
   BrokerMode mode() const { return mode_; }
-  void StopSerializing() {
-    CHECK_EQ(mode_, kSerializing);
-    mode_ = kSerialized;
-  }
+  void StartSerializing();
+  void StopSerializing();
+  void Retire();
   bool SerializingAllowed() const;
 
   // Returns nullptr iff handle unknown.
@@ -506,7 +506,8 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
   Zone* const zone_;
   base::Optional<NativeContextRef> native_context_;
   ZoneUnorderedMap<Address, ObjectData*> refs_;
-  BrokerMode mode_;
+
+  BrokerMode mode_ = kDisabled;
   unsigned tracing_indentation_ = 0;
 
   static const size_t kInitialRefsBucketCount = 1000;

src/compiler/pipeline.cc

@@ -1328,8 +1328,6 @@ struct CopyMetadataForConcurrentCompilePhase {
     NodeVector cached_nodes(temp_zone);
     data->jsgraph()->GetCachedNodes(&cached_nodes);
     for (Node* const node : cached_nodes) graph_reducer.ReduceNode(node);
-
-    data->js_heap_broker()->StopSerializing();
   }
 };
 
@@ -2011,8 +2009,6 @@ bool PipelineImpl::CreateGraph() {
     data->node_origins()->AddDecorator();
   }
 
-  Run<SerializeStandardObjectsPhase>();
-
   Run<GraphBuilderPhase>();
   RunPrintAndVerify(GraphBuilderPhase::phase_name(), true);
 
@@ -2040,8 +2036,12 @@ bool PipelineImpl::CreateGraph() {
   // Run the type-sensitive lowerings and optimizations on the graph.
   {
     if (FLAG_concurrent_compiler_frontend) {
+      data->js_heap_broker()->StartSerializing();
+      Run<SerializeStandardObjectsPhase>();
       Run<CopyMetadataForConcurrentCompilePhase>();
+      data->js_heap_broker()->StopSerializing();
     } else {
+      data->js_heap_broker()->SetNativeContextRef();
       // Type the graph and keep the Typer running such that new nodes get
       // automatically typed when they are created.
       Run<TyperPhase>(data->CreateTyper());
@@ -2571,6 +2571,9 @@ std::ostream& operator<<(std::ostream& out, const BlockStartsAsJSON& s) {
 
 MaybeHandle<Code> PipelineImpl::FinalizeCode() {
   PipelineData* data = this->data_;
+  if (data->js_heap_broker() && FLAG_concurrent_compiler_frontend) {
+    data->js_heap_broker()->Retire();
+  }
   Run<FinalizeCodePhase>();
 
   MaybeHandle<Code> maybe_code = data->code();

test/mjsunit/regress/regress-889722.js

@@ -0,0 +1,11 @@
+// Copyright 2018 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.
+
+function getRandomProperty(v, rand) {
+  var properties = Object.getOwnPropertyNames(v);
+  return properties[rand % properties.length];
+}
+r = Realm.create();
+o = Realm.eval(r, "() => { return Realm.global(-10) instanceof Object }");
+o.__p_211203344 = o[getRandomProperty(o, 211203344)];

test/unittests/compiler/graph-unittest.cc

@@ -25,6 +25,7 @@ GraphTest::GraphTest(int num_parameters)
       node_origins_(&graph_) {
   graph()->SetStart(graph()->NewNode(common()->Start(num_parameters)));
   graph()->SetEnd(graph()->NewNode(common()->End(1), graph()->start()));
+  js_heap_broker()->SetNativeContextRef();
 }
 
 
@@ -70,8 +71,6 @@ Node* GraphTest::HeapConstant(const Handle<HeapObject>& value) {
   Node* node = graph()->NewNode(common()->HeapConstant(value));
   Type type = Type::NewConstant(js_heap_broker(), value, zone());
   NodeProperties::SetType(node, type);
-  JSHeapCopyReducer heap_copy_reducer(js_heap_broker());
-  heap_copy_reducer.Reduce(node);
   return node;
 }
 

test/unittests/compiler/js-create-lowering-unittest.cc

@@ -34,7 +34,6 @@ class JSCreateLoweringTest : public TypedGraphTest {
         javascript_(zone()),
         deps_(isolate(), zone()),
         handle_scope_(isolate()) {
-    js_heap_broker()->SerializeStandardObjects();
   }
   ~JSCreateLoweringTest() override = default;