[turbofan] Fix property cell dependencies.

Fail IsInvalid check if the property cell has been invalidated.

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

src/compiler/compilation-dependencies.cc

@@ -203,6 +203,15 @@ class GlobalPropertyDependency final
 
   bool IsValid() const override {
     Handle<PropertyCell> cell = cell_.object();
+    // The dependency is never valid if the cell is 'invalidated'. This is
+    // marked by setting the value to the hole.
+    if (cell->value() == *(cell_.isolate()->factory()->the_hole_value())) {
+      DCHECK(cell->property_details().cell_type() ==
+                 PropertyCellType::kInvalidated ||
+             cell->property_details().cell_type() ==
+                 PropertyCellType::kUninitialized);
+      return false;
+    }
     return type_ == cell->property_details().cell_type() &&
            read_only_ == cell->property_details().IsReadOnly();
   }

src/compiler/js-native-context-specialization.cc

@@ -861,6 +861,8 @@ Reduction JSNativeContextSpecialization::ReduceGlobalAccess(
       if (property_details.cell_type() == PropertyCellType::kConstant ||
           property_details.cell_type() == PropertyCellType::kUndefined) {
         value = jsgraph()->Constant(property_cell_value);
+        CHECK(
+            !property_cell_value.is_identical_to(factory()->the_hole_value()));
       } else {
         // Load from constant type cell can benefit from type feedback.
         MaybeHandle<Map> map;

test/mjsunit/compiler/regress-905555-2.js

+// 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 --block-concurrent-recompilation --noalways-opt
+
+global = 1;
+
+function boom(value) {
+  return global;
+}
+
+assertEquals(1, boom());
+assertEquals(1, boom());
+%OptimizeFunctionOnNextCall(boom, "concurrent");
+assertEquals(1, boom());
+
+delete this.global;
+
+%UnblockConcurrentRecompilation();
+
+// boom should be deoptimized because the global property cell has changed.
+assertUnoptimized(boom, "sync");
+
+assertThrows(boom);

test/mjsunit/compiler/regress-905555.js

+// 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 --block-concurrent-recompilation --noalways-opt
+
+global = 1;
+
+function boom(value) {
+  return global;
+}
+
+assertEquals(1, boom());
+assertEquals(1, boom());
+%OptimizeFunctionOnNextCall(boom, "concurrent");
+assertEquals(1, boom());
+
+this.__defineGetter__("global", () => 42);
+
+%UnblockConcurrentRecompilation();
+
+// boom should be deoptimized because the global property cell has changed.
+assertUnoptimized(boom, "sync");
+
+assertEquals(42, boom());