From e8752eb9cef68297ff34cf9d87d187d679677f62 Mon Sep 17 00:00:00 2001 From: yangguo Date: Wed, 22 Jul 2015 00:37:21 -0700 Subject: [PATCH] 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} --- src/background-parsing-task.cc | 9 +------- src/compiler.cc | 8 +++++-- src/debug.h | 5 ----- test/cctest/test-api.cc | 51 ++++++++++++++++++++++++------------------ 4 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/background-parsing-task.cc b/src/background-parsing-task.cc index a79ce52..c0dba93 100644 --- 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 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -1072,6 +1072,8 @@ static Handle CompileToplevel(CompilationInfo* info) { } } + DCHECK(!info->is_debug() || !parse_info->allow_lazy_parsing()); + info->MarkAsFirstCompile(); FunctionLiteral* lit = info->function(); @@ -1333,8 +1335,10 @@ Handle Compiler::CompileStreamedScript( static_cast(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 --- 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( diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 80db551..44d29d1 100644 --- 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 func = i::Handle::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(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