Debugger: fix crash when debugger is enabled between parsing and compiling.

The background parser checks for debugger state in its constructor. This
is not good enough, since the debugger state may change afterwards, but
before compiling takes place. As the background parser can only parse
lazily, this could mean that due to debugging, we try to eagerly compile
an inner function we have not eagerly parsed.

R=jochen@chromium.org

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

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

diff --git a/src/background-parsing-task.cc b/src/background-parsing-task.cc
index a79ce52..c0dba93 100644 (file)
--- a/src/background-parsing-task.cc
+++ b/src/background-parsing-task.cc
@@ -31,15 +31,8 @@ BackgroundParsingTask::BackgroundParsingTask(
   info->set_hash_seed(isolate->heap()->HashSeed());
   info->set_global();
   info->set_unicode_cache(&source_->unicode_cache);
-
-  bool disable_lazy = isolate->debug()->RequiresEagerCompilation();
-  if (disable_lazy && options == ScriptCompiler::kProduceParserCache) {
-    // Producing cached data while parsing eagerly is not supported.
-    options = ScriptCompiler::kNoCompileOptions;
-  }
-
   info->set_compile_options(options);
-  info->set_allow_lazy_parsing(!disable_lazy);
+  info->set_allow_lazy_parsing(true);
 }
 
 

diff --git a/src/compiler.cc b/src/compiler.cc
index fc6760d..b71e72b 100644 (file)
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -1072,6 +1072,8 @@ static Handle<SharedFunctionInfo> CompileToplevel(CompilationInfo* info) {
       }
     }
 
+    DCHECK(!info->is_debug() || !parse_info->allow_lazy_parsing());
+
     info->MarkAsFirstCompile();
 
     FunctionLiteral* lit = info->function();
@@ -1333,8 +1335,10 @@ Handle<SharedFunctionInfo> Compiler::CompileStreamedScript(
       static_cast<LanguageMode>(parse_info->language_mode() | language_mode));
 
   CompilationInfo compile_info(parse_info);
-  // TODO(marja): FLAG_serialize_toplevel is not honoured and won't be; when the
-  // real code caching lands, streaming needs to be adapted to use it.
+
+  // If compiling for debugging, parse eagerly from scratch.
+  if (compile_info.is_debug()) parse_info->set_literal(NULL);
+
   return CompileToplevel(&compile_info);
 }
 

diff --git a/src/debug.h b/src/debug.h
index 715410b..3fa736e 100644 (file)
--- a/src/debug.h
+++ b/src/debug.h
@@ -493,11 +493,6 @@ class Debug {
            break_id() == id;
   }
 
-  bool RequiresEagerCompilation(bool allows_lazy_without_ctx = false) {
-    return LiveEditFunctionTracker::IsActive(isolate_) ||
-           (is_active() && !allows_lazy_without_ctx);
-  }
-
   // Flags and states.
   DebugScope* debugger_entry() {
     return reinterpret_cast<DebugScope*>(

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 80db551..44d29d1 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -20989,43 +20989,50 @@ TEST(StreamingProducesParserCache) {
 }
 
 
-TEST(StreamingWithDebuggingDoesNotProduceParserCache) {
-  // If the debugger is active, we should just not produce parser cache at
-  // all. This is a regeression test: We used to produce a parser cache without
-  // any data in it (just headers).
+TEST(StreamingWithDebuggingEnabledLate) {
+  // The streaming parser can only parse lazily, i.e. inner functions are not
+  // fully parsed. However, we may compile inner functions eagerly when
+  // debugging. Make sure that we can deal with this when turning on debugging
+  // after streaming parser has already finished parsing.
   i::FLAG_min_preparse_length = 0;
-  const char* chunks[] = {"function foo() { ret", "urn 13; } f", "oo(); ",
+  const char* chunks[] = {"with({x:1}) {",
+                          "  var foo = function foo(y) {",
+                          "    return x + y;",
+                          "  };",
+                          "  foo(2);",
+                          "}",
                           NULL};
 
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
   v8::HandleScope scope(isolate);
-
-  // Make the debugger active by setting a breakpoint.
-  CompileRun("function break_here() { }");
-  i::Handle<i::JSFunction> func = i::Handle<i::JSFunction>::cast(
-      v8::Utils::OpenHandle(*env->Global()->Get(v8_str("break_here"))));
-  EnableDebugger();
-  v8::internal::Debug* debug = CcTest::i_isolate()->debug();
-  int position = 0;
-  debug->SetBreakPoint(func, i::Handle<i::Object>(v8::internal::Smi::FromInt(1),
-                                                  CcTest::i_isolate()),
-                       &position);
+  v8::TryCatch try_catch(isolate);
 
   v8::ScriptCompiler::StreamedSource source(
       new TestSourceStream(chunks),
       v8::ScriptCompiler::StreamedSource::ONE_BYTE);
   v8::ScriptCompiler::ScriptStreamingTask* task =
-      v8::ScriptCompiler::StartStreamingScript(
-          isolate, &source, v8::ScriptCompiler::kProduceParserCache);
+      v8::ScriptCompiler::StartStreamingScript(isolate, &source);
 
-  // TestSourceStream::GetMoreData won't block, so it's OK to just run the
-  // task here in the main thread.
   task->Run();
   delete task;
 
-  // Check that we got no cached data.
-  CHECK(source.GetCachedData() == NULL);
+  CHECK(!try_catch.HasCaught());
+
+  v8::ScriptOrigin origin(v8_str("http://foo.com"));
+  char* full_source = TestSourceStream::FullSourceString(chunks);
+
+  EnableDebugger();
+
+  v8::Handle<Script> script = v8::ScriptCompiler::Compile(
+      isolate, &source, v8_str(full_source), origin);
+
+  Maybe<uint32_t> result =
+      script->Run(env.local()).ToLocalChecked()->Uint32Value(env.local());
+  CHECK_EQ(3, result.FromMaybe(0));
+
+  delete[] full_source;
+
   DisableDebugger();
 }