[turbofan] Install code dependencies atomically.

Split the Install methods into PrepareInstall and Install, such that
all heap mutations (besides the actual installation) are done in
PrepareInstall and only the actual installation in Install. This
ensures that the code object in question doesn't get deoptimized while
we're still installing its dependencies.

Bug: chromium:903697
Change-Id: I4da97d89d0707fa3c00c97c092af0d0faa7a4946
Reviewed-on: https://chromium-review.googlesource.com/c/1329162
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57419}

src/compiler/compilation-dependencies.cc

@@ -17,6 +17,7 @@ CompilationDependencies::CompilationDependencies(Isolate* isolate, Zone* zone)
 class CompilationDependencies::Dependency : public ZoneObject {
  public:
   virtual bool IsValid() const = 0;
+  virtual void PrepareInstall() {}
   virtual void Install(const MaybeObjectHandle& code) = 0;
 };
 
@@ -68,13 +69,16 @@ class PrototypePropertyDependency final
            function->prototype() == *prototype_.object();
   }
 
+  void PrepareInstall() override {
+    SLOW_DCHECK(IsValid());
+    Handle<JSFunction> function = function_.object();
+    if (!function->has_initial_map()) JSFunction::EnsureHasInitialMap(function);
+  }
+
   void Install(const MaybeObjectHandle& code) override {
     SLOW_DCHECK(IsValid());
     Handle<JSFunction> function = function_.object();
-    // Note that EnsureHasInitialMap can invalidate other dependencies, whether
-    // installed already or not, because it may change the map of the prototype
-    // object.
-    if (!function->has_initial_map()) JSFunction::EnsureHasInitialMap(function);
+    DCHECK(function->has_initial_map());
     Handle<Map> initial_map(function->initial_map(), function_.isolate());
     DependentCode::InstallDependency(function_.isolate(), code, initial_map,
                                      DependentCode::kInitialMapChangedGroup);
@@ -285,12 +289,18 @@ class InitialMapInstanceSizePredictionDependency final
     return instance_size == instance_size_;
   }
 
-  void Install(const MaybeObjectHandle& code) override {
-    DCHECK(IsValid());
-    // Finish the slack tracking.
+  void PrepareInstall() override {
+    SLOW_DCHECK(IsValid());
     function_.object()->CompleteInobjectSlackTrackingIfActive();
   }
 
+  void Install(const MaybeObjectHandle& code) override {
+    SLOW_DCHECK(IsValid());
+    DCHECK(!function_.object()
+                ->initial_map()
+                ->IsInobjectSlackTrackingInProgress());
+  }
+
  private:
   JSFunctionRef function_;
   int instance_size_;
@@ -376,23 +386,29 @@ bool CompilationDependencies::AreValid() const {
 }
 
 bool CompilationDependencies::Commit(Handle<Code> code) {
-  // Check validity of all dependencies first, such that we can avoid installing
-  // anything when there's already an invalid dependency.
-  if (!AreValid()) {
-    dependencies_.clear();
-    return false;
+  for (auto dep : dependencies_) {
+    if (!dep->IsValid()) {
+      dependencies_.clear();
+      return false;
+    }
+    dep->PrepareInstall();
   }
 
+  DisallowCodeDependencyChange no_dependency_change;
   for (auto dep : dependencies_) {
     // Check each dependency's validity again right before installing it,
-    // because a GC can trigger invalidation for some dependency kinds (e.g.,
-    // for PretenureModeDependency).
+    // because the first iteration above might have invalidated some
+    // dependencies. For example, PrototypePropertyDependency::PrepareInstall
+    // can call EnsureHasInitialMap, which can invalidate a StableMapDependency
+    // on the prototype object's map.
     if (!dep->IsValid()) {
       dependencies_.clear();
       return false;
     }
     dep->Install(MaybeObjectHandle::Weak(code));
   }
+  SLOW_DCHECK(AreValid());
+
   dependencies_.clear();
   return true;
 }

test/mjsunit/regress/regress-903697.js

@@ -0,0 +1,12 @@
+// 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.
+
+// Flags: --allow-natives-syntax --expose-gc --verify-heap
+
+C = class {};
+for (var i = 0; i < 5; ++i) {
+  gc();
+  if (i == 2) %OptimizeOsr();
+  C.prototype.foo = i + 9000000000000000;
+}