[debugger] correctly find function context.

In the debugger we are interested in getting the context for the
current frame, which is usually a function context. To do that,
we used to call Context::declaration_context, which may also
return a block context. This is wrong and can lead to crashes.
Instead, we now use a newly introduced Context::closure_context,
which skips block contexts. This works fine for the debugger,
since we have other means to find and materialize block contexts.

R=rossberg@chromium.org
BUG=chromium:582051
LOG=N

Review URL: https://codereview.chromium.org/1648263002

Cr-Commit-Position: refs/heads/master@{#33627}

src/contexts.cc

@@ -79,6 +79,15 @@ Context* Context::declaration_context() {
   return current;
 }
 
+Context* Context::closure_context() {
+  Context* current = this;
+  while (!current->IsFunctionContext() && !current->IsScriptContext() &&
+         !current->IsNativeContext()) {
+    current = current->previous();
+    DCHECK(current->closure() == closure());
+  }
+  return current;
+}
 
 JSObject* Context::extension_object() {
   DCHECK(IsNativeContext() || IsFunctionContext() || IsBlockContext());

src/contexts.h

@@ -450,6 +450,9 @@ class Context: public FixedArray {
   Context* declaration_context();
   bool is_declaration_context();
 
+  // Get the next closure's context on the context chain.
+  Context* closure_context();
+
   // Returns a JSGlobalProxy object or null.
   JSObject* global_proxy();
   void set_global_proxy(JSObject* global);

src/debug/debug-scopes.cc

@@ -467,7 +467,7 @@ MaybeHandle<JSObject> ScopeIterator::MaterializeLocalScope() {
   if (!scope_info->HasContext()) return local_scope;
 
   // Third fill all context locals.
-  Handle<Context> function_context(frame_context->declaration_context());
+  Handle<Context> function_context(frame_context->closure_context());
   CopyContextLocalsToScopeObject(scope_info, function_context, local_scope);
 
   // Finally copy any properties from the function context extension.

src/runtime/runtime-debug.cc

@@ -554,7 +554,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
   if (local < local_count) {
     // Get the context containing declarations.
     Handle<Context> context(
-        Context::cast(frame_inspector.GetContext())->declaration_context());
+        Context::cast(frame_inspector.GetContext())->closure_context());
     for (; i < scope_info->LocalCount(); ++i) {
       if (scope_info->LocalIsSynthetic(i)) continue;
       Handle<String> name(scope_info->LocalName(i));

test/mjsunit/mjsunit.status

@@ -940,6 +940,7 @@
   'regress/regress-crbug-568477-4': [SKIP],
   'regress/regress-crbug-575080': [SKIP],
   'regress/regress-crbug-580506': [SKIP],
+  'regress/regress-crbug-582051': [SKIP],
   'regress/regress-deopt-in-array-literal-spread': [SKIP],
   'regress/regress-fast-literal-transition': [SKIP],
   'regress/regress-function-constructor-receiver': [SKIP],

test/mjsunit/regress/regress-crbug-582051.js

+// Copyright 2016 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: --expose-debug-as debug
+
+var test_y = false;
+
+function foo(a = 1) {
+  var x = 2;
+  debugger;
+  eval("var y = 3");
+  test_y = true;
+  debugger;
+}
+
+var exception = null;
+var break_count = 0;
+var Debug = debug.Debug;
+var ScopeType = debug.ScopeType;
+
+function listener(event, exec_state) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    var scopes = exec_state.frame(0).allScopes();
+    var expectation = [ ScopeType.Block,
+                        ScopeType.Local,
+                        ScopeType.Script,
+                        ScopeType.Global ];
+    assertEquals(expectation, scopes.map(x => x.scopeType()));
+    assertEquals(2, scopes[0].scopeObject().value().x);
+    if (test_y) assertEquals(3, scopes[0].scopeObject().value().y);
+    assertEquals(1, scopes[1].scopeObject().value().a);
+    break_count++;
+  } catch (e) {
+    print(e);
+    exception = e;
+  }
+}
+Debug.setListener(listener);
+foo();
+
+assertNull(exception);
+assertEquals(2, break_count);