Reviewers: jochen,

Message:
Please take a look.


https://codereview.chromium.org/1247743002/diff/1/src/compiler.cc
File src/compiler.cc (left):

https://codereview.chromium.org/1247743002/diff/1/src/compiler.cc#oldcode1337
src/compiler.cc:1337: // real code caching lands, streaming needs to be
adapted to use it.
For the current strategy used in Chrome this is not necessary. We never
produce code cache on first parse/compile anyways. Instead of set a time
stamp to support code caching decision at a later time.

Description:
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.

[email protected]

Please review this at https://codereview.chromium.org/1247743002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+34, -37 lines):
  M src/background-parsing-task.cc
  M src/compiler.cc
  M src/debug.h
  M test/cctest/test-api.cc


Index: src/background-parsing-task.cc
diff --git a/src/background-parsing-task.cc b/src/background-parsing-task.cc
index a79ce52788aeeef7284c1c3d13a2b93d69812744..c0dba93a533663a51f319e42bda75fb769ecbf54 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);
 }


Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index fc6760d3f26386b8a23cb09316dbf78a5b17de30..c76072691b2cfeac9a1578ff7fde13428d2e8453 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -1333,8 +1333,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);
 }

Index: src/debug.h
diff --git a/src/debug.h b/src/debug.h
index 715410b3a8ea468578b1282d204825dea4f9a9a8..3fa736e0942f2bb5bb8943ddeef9ef6352471455 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<DebugScope*>(
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 5de04907085bca722611dfc21a244e2a0b864c23..d3a7309008f400109e50ebb1239210234e90fdd3 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -20987,43 +20987,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();
 }



--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to