From 603acc3f49e2190e8ec253ea259cb94da6ad30d7 Mon Sep 17 00:00:00 2001
From: rmcilroy <rmcilroy@chromium.org>
Date: Tue, 19 Jan 2016 07:51:04 -0800
Subject: [PATCH] [Interpreter] Ensure that block breaks are within the correct
 context scope.

Fixes a bug where the context would be popped before labeled block break target
location.

BUG=v8:4280
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#33388}
---
 src/interpreter/bytecode-generator.cc         | 32 ++++++++---------
 src/interpreter/bytecode-generator.h          |  1 +
 .../interpreter/test-bytecode-generator.cc    | 36 ++++++++++++++++++-
 3 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index 6e9b454e8c..6e94b5f079 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -422,24 +422,24 @@ void BytecodeGenerator::MakeBytecodeBody() {
 
 
 void BytecodeGenerator::VisitBlock(Block* stmt) {
-  BlockBuilder block_builder(this->builder());
-  ControlScopeForBreakable execution_control(this, stmt, &block_builder);
-
-  if (stmt->scope() == NULL) {
-    // Visit statements in the same scope, no declarations.
-    VisitStatements(stmt->statements());
+  // Visit declarations and statements.
+  if (stmt->scope() != nullptr && stmt->scope()->NeedsContext()) {
+    VisitNewLocalBlockContext(stmt->scope());
+    ContextScope scope(this, stmt->scope());
+    VisitBlockDeclarationsAndStatements(stmt);
   } else {
-    // Visit declarations and statements in a block scope.
-    if (stmt->scope()->NeedsContext()) {
-      VisitNewLocalBlockContext(stmt->scope());
-      ContextScope scope(this, stmt->scope());
-      VisitDeclarations(stmt->scope()->declarations());
-      VisitStatements(stmt->statements());
-    } else {
-      VisitDeclarations(stmt->scope()->declarations());
-      VisitStatements(stmt->statements());
-    }
+    VisitBlockDeclarationsAndStatements(stmt);
   }
+}
+
+
+void BytecodeGenerator::VisitBlockDeclarationsAndStatements(Block* stmt) {
+  BlockBuilder block_builder(builder());
+  ControlScopeForBreakable execution_control(this, stmt, &block_builder);
+  if (stmt->scope() != nullptr) {
+    VisitDeclarations(stmt->scope()->declarations());
+  }
+  VisitStatements(stmt->statements());
   if (stmt->labels() != nullptr) block_builder.EndBlock();
 }
 
diff --git a/src/interpreter/bytecode-generator.h b/src/interpreter/bytecode-generator.h
index 8bda7be301..a4b9caba0f 100644
--- a/src/interpreter/bytecode-generator.h
+++ b/src/interpreter/bytecode-generator.h
@@ -79,6 +79,7 @@ class BytecodeGenerator final : public AstVisitor {
   void VisitNewTargetVariable(Variable* variable);
   void VisitNewLocalFunctionContext();
   void VisitBuildLocalActivationContext();
+  void VisitBlockDeclarationsAndStatements(Block* stmt);
   void VisitNewLocalBlockContext(Scope* scope);
   void VisitFunctionClosureForContext();
   void VisitSetHomeObject(Register value, Register home_object,
diff --git a/test/cctest/interpreter/test-bytecode-generator.cc b/test/cctest/interpreter/test-bytecode-generator.cc
index e32f8fd921..2749ba7837 100644
--- a/test/cctest/interpreter/test-bytecode-generator.cc
+++ b/test/cctest/interpreter/test-bytecode-generator.cc
@@ -2188,7 +2188,10 @@ TEST(BreakableBlocks) {
   InitializedHandleScope handle_scope;
   BytecodeGeneratorHelper helper;
 
-  ExpectedSnippet<int> snippets[] = {
+  int closure = Register::function_closure().index();
+  int context = Register::function_context().index();
+
+  ExpectedSnippet<InstanceType> snippets[] = {
       {"var x = 0;\n"
        "label: {\n"
        "  x = x + 1;\n"
@@ -2266,6 +2269,37 @@ TEST(BreakableBlocks) {
            B(Ldar), R(0),           //
            B(Return),               //
        }},
+      {"outer: {\n"
+       "  let y = 10;"
+       "  function f() { return y; }\n"
+       "  break outer;\n"
+       "}\n",
+       5 * kPointerSize,
+       1,
+       39,
+       {
+           B(LdaConstant), U8(0),                                         //
+           B(Star), R(3),                                                 //
+           B(Ldar), R(closure),                                           //
+           B(Star), R(4),                                                 //
+           B(CallRuntime), U16(Runtime::kPushBlockContext), R(3), U8(2),  //
+           B(PushContext), R(2),                                          //
+           B(LdaTheHole),                                                 //
+           B(StaContextSlot), R(2), U8(4),                                //
+           B(CreateClosure), U8(1), U8(0),                                //
+           B(Star), R(0),                                                 //
+           B(LdaSmi8), U8(10),                                            //
+           B(StaContextSlot), R(2), U8(4),                                //
+           B(Ldar), R(0),                                                 //
+           B(Star), R(1),                                                 //
+           B(Jump), U8(2),                                                //
+           B(PopContext), R(context),                                     //
+           B(LdaUndefined),                                               //
+           B(Return),                                                     //
+       },
+       2,
+       {InstanceType::FIXED_ARRAY_TYPE,
+        InstanceType::SHARED_FUNCTION_INFO_TYPE}},
   };
 
   for (size_t i = 0; i < arraysize(snippets); i++) {