[debugger] Hide scopes that originate from desugaring.

Some scopes are introduced by the parser for desugaring and do not
have any positions associated. The debugger should not make them
visible.

Also add some missing source positions.

R=kozyatinskiy@chromium.org, rossberg@chromium.org
BUG=chromium:604458
LOG=Y

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

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

src/ast/scopes.cc

@@ -193,6 +193,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
   scope_info_ = scope_info;
   start_position_ = RelocInfo::kNoPosition;
   end_position_ = RelocInfo::kNoPosition;
+  is_hidden_ = false;
   if (!scope_info.is_null()) {
     scope_calls_eval_ = scope_info->CallsEval();
     language_mode_ = scope_info->language_mode();
@@ -287,6 +288,7 @@ bool Scope::Analyze(ParseInfo* info) {
                                : FLAG_print_scopes) {
     scope->Print();
   }
+  scope->CheckScopePositions();
 #endif
 
   info->set_scope(scope);
@@ -1007,6 +1009,16 @@ void Scope::Print(int n) {
 
   Indent(n0, "}\n");
 }
+
+void Scope::CheckScopePositions() {
+  // A scope is allowed to have invalid positions if it is hidden and has no
+  // inner scopes
+  if (!is_hidden() && inner_scopes_.length() == 0) {
+    CHECK_NE(RelocInfo::kNoPosition, start_position());
+    CHECK_NE(RelocInfo::kNoPosition, end_position());
+  }
+  for (Scope* scope : inner_scopes_) scope->CheckScopePositions();
+}
 #endif  // DEBUG
 
 

src/ast/scopes.h

@@ -295,6 +295,10 @@ class Scope: public ZoneObject {
     end_position_ = statement_pos;
   }
 
+  // Scopes created for desugaring are hidden. I.e. not visible to the debugger.
+  bool is_hidden() const { return is_hidden_; }
+  void set_is_hidden() { is_hidden_ = true; }
+
   // In some cases we want to force context allocation for a whole scope.
   void ForceContextAllocation() {
     DCHECK(!already_resolved());
@@ -574,6 +578,9 @@ class Scope: public ZoneObject {
 
 #ifdef DEBUG
   void Print(int n = 0);  // n = indentation; n < 0 => don't print recursively
+
+  // Check that the scope has positions assigned.
+  void CheckScopePositions();
 #endif
 
   // ---------------------------------------------------------------------------
@@ -645,6 +652,7 @@ class Scope: public ZoneObject {
   // Source positions.
   int start_position_;
   int end_position_;
+  bool is_hidden_;
 
   // Computed via PropagateScopeInfo.
   bool outer_scope_calls_sloppy_eval_;

src/debug/debug-scopes.cc

@@ -842,7 +842,7 @@ bool ScopeIterator::CopyContextExtensionToScopeObject(
 
 void ScopeIterator::GetNestedScopeChain(Isolate* isolate, Scope* scope,
                                         int position) {
-  if (!scope->is_eval_scope()) {
+  if (!scope->is_eval_scope() && !scope->is_hidden()) {
     nested_scope_chain_.Add(ExtendedScopeInfo(scope->GetScopeInfo(isolate),
                                               scope->start_position(),
                                               scope->end_position()));
@@ -851,7 +851,7 @@ void ScopeIterator::GetNestedScopeChain(Isolate* isolate, Scope* scope,
     Scope* inner_scope = scope->inner_scopes()->at(i);
     int beg_pos = inner_scope->start_position();
     int end_pos = inner_scope->end_position();
-    DCHECK(beg_pos >= 0 && end_pos >= 0);
+    DCHECK((beg_pos >= 0 && end_pos >= 0) || inner_scope->is_hidden());
     if (beg_pos <= position && position < end_pos) {
       GetNestedScopeChain(isolate, inner_scope, position);
       return;

src/parsing/parser.cc

@@ -3435,6 +3435,7 @@ Statement* Parser::ParseScopedStatement(ZoneList<const AstRawString*>* labels,
     // Make a block around the statement for a lexical binding
     // is introduced by a FunctionDeclaration.
     Scope* body_scope = NewScope(scope_, BLOCK_SCOPE);
+    body_scope->set_start_position(scanner()->location().beg_pos);
     BlockState block_state(&scope_, body_scope);
     Block* block = factory()->NewBlock(NULL, 1, false, RelocInfo::kNoPosition);
     Statement* body = ParseFunctionDeclaration(NULL, CHECK_OK);
@@ -3459,6 +3460,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
   Expect(Token::FOR, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);
   for_scope->set_start_position(scanner()->location().beg_pos);
+  for_scope->set_is_hidden();
   DeclarationParsingResult parsing_result;
   if (peek() != Token::SEMICOLON) {
     if (peek() == Token::VAR || peek() == Token::CONST ||
@@ -4381,8 +4383,8 @@ Block* Parser::BuildParameterInitializationBlock(
     if (!parameter.is_simple() && scope_->calls_sloppy_eval()) {
       param_scope = NewScope(scope_, BLOCK_SCOPE);
       param_scope->set_is_declaration_scope();
-      param_scope->set_start_position(parameter.pattern->position());
-      param_scope->set_end_position(RelocInfo::kNoPosition);
+      param_scope->set_start_position(descriptor.initialization_pos);
+      param_scope->set_end_position(parameter.initializer_end_position);
       param_scope->RecordEvalCall();
       param_block = factory()->NewBlock(NULL, 8, true, RelocInfo::kNoPosition);
       param_block->set_scope(param_scope);
@@ -5971,6 +5973,7 @@ Expression* ParserTraits::RewriteYieldStar(
     catch_block->statements()->Add(set_mode_throw, zone);
 
     Scope* catch_scope = NewScope(scope, CATCH_SCOPE);
+    catch_scope->set_is_hidden();
     const AstRawString* name = avfactory->dot_catch_string();
     Variable* catch_variable =
         catch_scope->DeclareLocal(name, VAR, kCreatedInitialized,
@@ -6452,6 +6455,7 @@ void ParserTraits::FinalizeIteratorUse(Variable* completion,
     Variable* catch_variable =
         catch_scope->DeclareLocal(avfactory->dot_catch_string(), VAR,
                                   kCreatedInitialized, Variable::NORMAL);
+    catch_scope->set_is_hidden();
 
     Statement* rethrow;
     // We use %ReThrow rather than the ordinary throw because we want to
@@ -6563,6 +6567,7 @@ void ParserTraits::BuildIteratorCloseForCompletion(
     Variable* catch_variable = catch_scope->DeclareLocal(
         avfactory->dot_catch_string(), VAR, kCreatedInitialized,
         Variable::NORMAL);
+    catch_scope->set_is_hidden();
 
     try_call_return = factory->NewTryCatchStatement(
         try_block, catch_scope, catch_variable, catch_block, nopos);

test/mjsunit/debug-scopes.js

@@ -1179,11 +1179,10 @@ var code3 = "function for_statement() {         \n" +
 
 listener_delegate = function(exec_state) {
   CheckScopeChain([debug.ScopeType.Block,
-                   debug.ScopeType.Block,
                    debug.ScopeType.Local,
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
-  CheckScopeChainPositions([{start: 52, end: 111}, {start: 42, end: 111}, {start: 22, end: 145}, {}, {}], exec_state);
+  CheckScopeChainPositions([{start: 52, end: 111}, {start: 22, end: 145}, {}, {}], exec_state);
 }
 eval(code3);
 EndTest();

test/mjsunit/es6/debug-blockscopes.js

@@ -52,6 +52,7 @@ function listener(event, exec_state, event_data, data) {
       listener_delegate(exec_state);
     }
   } catch (e) {
+    print(e, e.stack);
     exception = e;
   }
 }
@@ -380,16 +381,12 @@ function for_loop_1() {
 
 listener_delegate = function(exec_state) {
   CheckScopeChain([debug.ScopeType.Block,
-                   debug.ScopeType.Block,
                    debug.ScopeType.Local,
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
   CheckScopeContent({x:'y'}, 0, exec_state);
   // The function scope contains a temporary iteration variable, but it is
   // hidden to the debugger.
-  // TODO(adamk): This variable is only used to provide a TDZ for the enumerable
-  // expression and should not be visible to the debugger.
-  CheckScopeContent({x:undefined}, 1, exec_state);
 };
 for_loop_1();
 EndTest();
@@ -408,7 +405,6 @@ function for_loop_2() {
 
 listener_delegate = function(exec_state) {
   CheckScopeChain([debug.ScopeType.Block,
-                   debug.ScopeType.Block,
                    debug.ScopeType.Block,
                    debug.ScopeType.Local,
                    debug.ScopeType.Script,
@@ -417,9 +413,6 @@ listener_delegate = function(exec_state) {
   CheckScopeContent({x:'y'}, 1, exec_state);
   // The function scope contains a temporary iteration variable, hidden to the
   // debugger.
-  // TODO(adamk): This variable is only used to provide a TDZ for the enumerable
-  // expression and should not be visible to the debugger.
-  CheckScopeContent({x:undefined}, 2, exec_state);
 };
 for_loop_2();
 EndTest();
@@ -436,13 +429,11 @@ function for_loop_3() {
 
 listener_delegate = function(exec_state) {
   CheckScopeChain([debug.ScopeType.Block,
-                   debug.ScopeType.Block,
                    debug.ScopeType.Local,
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
   CheckScopeContent({x:3}, 0, exec_state);
-  CheckScopeContent({x:3}, 1, exec_state);
-  CheckScopeContent({}, 2, exec_state);
+  CheckScopeContent({}, 1, exec_state);
 };
 for_loop_3();
 EndTest();
@@ -460,15 +451,13 @@ function for_loop_4() {
 
 listener_delegate = function(exec_state) {
   CheckScopeChain([debug.ScopeType.Block,
-                   debug.ScopeType.Block,
                    debug.ScopeType.Block,
                    debug.ScopeType.Local,
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
   CheckScopeContent({x:5}, 0, exec_state);
   CheckScopeContent({x:3}, 1, exec_state);
-  CheckScopeContent({x:3}, 2, exec_state);
-  CheckScopeContent({}, 3, exec_state);
+  CheckScopeContent({}, 2, exec_state);
 };
 for_loop_4();
 EndTest();
@@ -485,13 +474,11 @@ function for_loop_5() {
 
 listener_delegate = function(exec_state) {
   CheckScopeChain([debug.ScopeType.Block,
-                   debug.ScopeType.Block,
                    debug.ScopeType.Local,
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
   CheckScopeContent({x:3,y:5}, 0, exec_state);
-  CheckScopeContent({x:3,y:5}, 1, exec_state);
-  CheckScopeContent({}, 2, exec_state);
+  CheckScopeContent({}, 1, exec_state);
 };
 for_loop_5();
 EndTest();

test/mjsunit/es6/debug-scope-default-param-with-eval.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
+
+// Test that the parameter initialization block scope set up for
+// sloppy eval is visible to the debugger.
+
+var Debug = debug.Debug;
+var exception = null;
+var break_count = 0;
+
+function call_for_break() {
+  return 5;
+}
+
+function test(x = eval("var y = 7; debugger; y") + call_for_break()) {
+  return x;
+}
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    var frame = exec_state.frame(0);
+    var top_scope = frame.scope(0);
+    assertTrue(top_scope.scopeObject().propertyNames().includes('y'));
+    assertEquals(7, top_scope.scopeObject().property('y').value().value());
+    if (break_count++ == 0) {
+      // Inside eval.
+      assertEquals([ debug.ScopeType.Block,
+                     debug.ScopeType.Closure,
+                     debug.ScopeType.Script,
+                     debug.ScopeType.Global ],
+                   frame.allScopes().map(s => s.scopeType()));
+      exec_state.prepareStep(Debug.StepAction.StepOut);
+    } else {
+      // Outside of eval.
+      assertEquals([ debug.ScopeType.Block,
+                     debug.ScopeType.Local,
+                     debug.ScopeType.Script,
+                     debug.ScopeType.Global ],
+                   frame.allScopes().map(s => s.scopeType()));
+    }
+  } catch (e) {
+    exception = e;
+  }
+}
+
+Debug.setListener(listener);
+
+assertEquals(12, test());
+
+Debug.setListener(null);
+
+assertNull(exception);
+assertEquals(2, break_count);

test/mjsunit/es6/regress/regress-468661.js

@@ -9,7 +9,8 @@ var exception = null;
 var break_count = 0;
 
 var expected_values =
-  [ReferenceError, undefined, 0, 0, 0, 0, 1, ReferenceError, ReferenceError];
+  [ReferenceError, ReferenceError, 0, 0, 0, 0, 1,
+   ReferenceError, ReferenceError];
 
 function listener(event, exec_state, event_data, data) {
   try {
@@ -39,7 +40,6 @@ function listener(event, exec_state, event_data, data) {
         assertTrue(v instanceof ReferenceError);
       } else {
         assertSame(expected_values[break_count], v);
-
       }
       ++break_count;