[ignition] Fix building lookup graph when search depth is 0

In some (rare) cases, the context depth passed to a dynamic variable lookup can
be zero. In these cases, the fast path for the lookup (i.e. load from context or
global) can always be taken, as there is no need to search the current context.

However, with no slow path checks, the bytecode graph builder had a null
environment for the slow path, causing segfaults when this graph was built.

This patch adds a null check for the slow path environment, and skips building
the slow path if the environment is null.

BUG=chromium:652186

Review-Url: https://codereview.chromium.org/2385123002
Cr-Commit-Position: refs/heads/master@{#39949}

src/compiler/bytecode-graph-builder.cc

@@ -896,8 +896,6 @@ BytecodeGraphBuilder::Environment* BytecodeGraphBuilder::CheckContextExtensions(
   // Output environment where the context has an extension
   Environment* slow_environment = nullptr;
 
-  DCHECK_GT(depth, 0u);
-
   // We only need to check up to the last-but-one depth, because the an eval in
   // the same scope as the variable itself has no way of shadowing it.
   for (uint32_t d = 0; d < depth; d++) {
@@ -931,7 +929,9 @@ BytecodeGraphBuilder::Environment* BytecodeGraphBuilder::CheckContextExtensions(
     }
   }
 
-  DCHECK_NOT_NULL(slow_environment);
+  // The depth can be zero, in which case no slow-path checks are built, and the
+  // slow path environment can be null.
+  DCHECK(depth == 0 || slow_environment != nullptr);
 
   return slow_environment;
 }
@@ -949,28 +949,33 @@ void BytecodeGraphBuilder::BuildLdaLookupContextSlot(TypeofMode typeof_mode) {
     const Operator* op = javascript()->LoadContext(depth, slot_index, false);
     Node* context = environment()->Context();
     environment()->BindAccumulator(NewNode(op, context));
-    NewMerge();
   }
-  Environment* fast_environment = environment();
 
-  // Slow path, do a runtime load lookup.
-  set_environment(slow_environment);
-  {
-    FrameStateBeforeAndAfter states(this);
+  // Only build the slow path if there were any slow-path checks.
+  if (slow_environment != nullptr) {
+    // Add a merge to the fast environment.
+    NewMerge();
+    Environment* fast_environment = environment();
 
-    Node* name =
-        jsgraph()->Constant(bytecode_iterator().GetConstantForIndexOperand(0));
+    // Slow path, do a runtime load lookup.
+    set_environment(slow_environment);
+    {
+      FrameStateBeforeAndAfter states(this);
 
-    const Operator* op =
-        javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
-                                      ? Runtime::kLoadLookupSlot
-                                      : Runtime::kLoadLookupSlotInsideTypeof);
-    Node* value = NewNode(op, name);
-    environment()->BindAccumulator(value, &states);
-  }
+      Node* name = jsgraph()->Constant(
+          bytecode_iterator().GetConstantForIndexOperand(0));
+
+      const Operator* op =
+          javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
+                                        ? Runtime::kLoadLookupSlot
+                                        : Runtime::kLoadLookupSlotInsideTypeof);
+      Node* value = NewNode(op, name);
+      environment()->BindAccumulator(value, &states);
+    }
 
-  fast_environment->Merge(environment());
-  set_environment(fast_environment);
+    fast_environment->Merge(environment());
+    set_environment(fast_environment);
+  }
 }
 
 void BytecodeGraphBuilder::VisitLdaLookupContextSlot() {
@@ -993,29 +998,33 @@ void BytecodeGraphBuilder::BuildLdaLookupGlobalSlot(TypeofMode typeof_mode) {
     Node* node =
         BuildLoadGlobal(bytecode_iterator().GetIndexOperand(1), typeof_mode);
     environment()->BindAccumulator(node, &states);
+  }
 
+  // Only build the slow path if there were any slow-path checks.
+  if (slow_environment != nullptr) {
+    // Add a merge to the fast environment.
     NewMerge();
-  }
-  Environment* fast_environment = environment();
+    Environment* fast_environment = environment();
 
-  // Slow path, do a runtime load lookup.
-  set_environment(slow_environment);
-  {
-    FrameStateBeforeAndAfter states(this);
+    // Slow path, do a runtime load lookup.
+    set_environment(slow_environment);
+    {
+      FrameStateBeforeAndAfter states(this);
 
-    Node* name =
-        jsgraph()->Constant(bytecode_iterator().GetConstantForIndexOperand(0));
+      Node* name = jsgraph()->Constant(
+          bytecode_iterator().GetConstantForIndexOperand(0));
 
-    const Operator* op =
-        javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
-                                      ? Runtime::kLoadLookupSlot
-                                      : Runtime::kLoadLookupSlotInsideTypeof);
-    Node* value = NewNode(op, name);
-    environment()->BindAccumulator(value, &states);
-  }
+      const Operator* op =
+          javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
+                                        ? Runtime::kLoadLookupSlot
+                                        : Runtime::kLoadLookupSlotInsideTypeof);
+      Node* value = NewNode(op, name);
+      environment()->BindAccumulator(value, &states);
+    }
 
-  fast_environment->Merge(environment());
-  set_environment(fast_environment);
+    fast_environment->Merge(environment());
+    set_environment(fast_environment);
+  }
 }
 
 void BytecodeGraphBuilder::VisitLdaLookupGlobalSlot() {

test/mjsunit/regress/regress-crbug-652186-global.js

+// Copyright 2015 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: --always-opt
+
+x = 1;
+print(eval("eval('var x = 2'); x;"));

test/mjsunit/regress/regress-crbug-652186-local.js

+// Copyright 2015 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: --always-opt
+
+function f() {
+  var x = 1;
+  return eval("eval('var x = 2'); x;");
+}
+f();