Handle invalid parser cache gracefully (move invalid cache handling to embedder).

Blink already has code for handling invalid cached data. The attached test
ensures that cached data is gracefully rejected if it cannot be used.

This also unifies parser cache and code cache handling.

R=yangguo@chromium.org
BUG=439889
LOG=N

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

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

diff --git a/src/parser.cc b/src/parser.cc
index 057b2e9..9cb92b3 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3831,63 +3831,63 @@ void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
   if (produce_cached_parse_data()) CHECK(log_);
 
   int function_block_pos = position();
-  if (consume_cached_parse_data()) {
+  if (consume_cached_parse_data() && !cached_parse_data_->rejected()) {
     // If we have cached data, we use it to skip parsing the function body. The
     // data contains the information we need to construct the lazy function.
     FunctionEntry entry =
         cached_parse_data_->GetFunctionEntry(function_block_pos);
-    // Check that cached data is valid.
-    CHECK(entry.is_valid());
-    // End position greater than end of stream is safe, and hard to check.
-    CHECK(entry.end_pos() > function_block_pos);
-    scanner()->SeekForward(entry.end_pos() - 1);
-
-    scope_->set_end_position(entry.end_pos());
-    Expect(Token::RBRACE, ok);
-    if (!*ok) {
-      return;
-    }
-    total_preparse_skipped_ += scope_->end_position() - function_block_pos;
-    *materialized_literal_count = entry.literal_count();
-    *expected_property_count = entry.property_count();
-    scope_->SetStrictMode(entry.strict_mode());
-  } else {
-    // With no cached data, we partially parse the function, without building an
-    // AST. This gathers the data needed to build a lazy function.
-    SingletonLogger logger;
-    PreParser::PreParseResult result =
-        ParseLazyFunctionBodyWithPreParser(&logger);
-    if (result == PreParser::kPreParseStackOverflow) {
-      // Propagate stack overflow.
-      set_stack_overflow();
-      *ok = false;
-      return;
-    }
-    if (logger.has_error()) {
-      ParserTraits::ReportMessageAt(
-          Scanner::Location(logger.start(), logger.end()),
-          logger.message(), logger.argument_opt(), logger.is_reference_error());
-      *ok = false;
-      return;
-    }
-    scope_->set_end_position(logger.end());
-    Expect(Token::RBRACE, ok);
-    if (!*ok) {
+    // Check that cached data is valid. If not, mark it as invalid (the embedder
+    // handles it). Note that end position greater than end of stream is safe,
+    // and hard to check.
+    if (entry.is_valid() && entry.end_pos() > function_block_pos) {
+      scanner()->SeekForward(entry.end_pos() - 1);
+
+      scope_->set_end_position(entry.end_pos());
+      Expect(Token::RBRACE, ok);
+      if (!*ok) {
+        return;
+      }
+      total_preparse_skipped_ += scope_->end_position() - function_block_pos;
+      *materialized_literal_count = entry.literal_count();
+      *expected_property_count = entry.property_count();
+      scope_->SetStrictMode(entry.strict_mode());
       return;
     }
-    total_preparse_skipped_ += scope_->end_position() - function_block_pos;
-    *materialized_literal_count = logger.literals();
-    *expected_property_count = logger.properties();
-    scope_->SetStrictMode(logger.strict_mode());
-    if (produce_cached_parse_data()) {
-      DCHECK(log_);
-      // Position right after terminal '}'.
-      int body_end = scanner()->location().end_pos;
-      log_->LogFunction(function_block_pos, body_end,
-                        *materialized_literal_count,
-                        *expected_property_count,
-                        scope_->strict_mode());
-    }
+    cached_parse_data_->Reject();
+  }
+  // With no cached data, we partially parse the function, without building an
+  // AST. This gathers the data needed to build a lazy function.
+  SingletonLogger logger;
+  PreParser::PreParseResult result =
+      ParseLazyFunctionBodyWithPreParser(&logger);
+  if (result == PreParser::kPreParseStackOverflow) {
+    // Propagate stack overflow.
+    set_stack_overflow();
+    *ok = false;
+    return;
+  }
+  if (logger.has_error()) {
+    ParserTraits::ReportMessageAt(
+        Scanner::Location(logger.start(), logger.end()), logger.message(),
+        logger.argument_opt(), logger.is_reference_error());
+    *ok = false;
+    return;
+  }
+  scope_->set_end_position(logger.end());
+  Expect(Token::RBRACE, ok);
+  if (!*ok) {
+    return;
+  }
+  total_preparse_skipped_ += scope_->end_position() - function_block_pos;
+  *materialized_literal_count = logger.literals();
+  *expected_property_count = logger.properties();
+  scope_->SetStrictMode(logger.strict_mode());
+  if (produce_cached_parse_data()) {
+    DCHECK(log_);
+    // Position right after terminal '}'.
+    int body_end = scanner()->location().end_pos;
+    log_->LogFunction(function_block_pos, body_end, *materialized_literal_count,
+                      *expected_property_count, scope_->strict_mode());
   }
 }
 

diff --git a/src/parser.h b/src/parser.h
index 58ef47a..b812c98 100644 (file)
--- a/src/parser.h
+++ b/src/parser.h
@@ -78,6 +78,10 @@ class ParseData {
     return reinterpret_cast<unsigned*>(const_cast<byte*>(script_data_->data()));
   }
 
+  void Reject() { script_data_->Reject(); }
+
+  bool rejected() const { return script_data_->rejected(); }
+
  private:
   explicit ParseData(ScriptData* script_data) : script_data_(script_data) {}
 

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index bf14b88..e8c3450 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -24379,6 +24379,7 @@ TEST(StreamingProducesParserCache) {
   const v8::ScriptCompiler::CachedData* cached_data = source.GetCachedData();
   CHECK(cached_data != NULL);
   CHECK(cached_data->data != NULL);
+  CHECK(!cached_data->rejected);
   CHECK_GT(cached_data->length, 0);
 }
 
@@ -24470,6 +24471,61 @@ TEST(InvalidCacheData) {
 }
 
 
+TEST(ParserCacheRejectedGracefully) {
+  i::FLAG_min_preparse_length = 0;
+  v8::V8::Initialize();
+  v8::HandleScope scope(CcTest::isolate());
+  LocalContext context;
+  // Produce valid cached data.
+  v8::ScriptOrigin origin(v8_str("origin"));
+  v8::Local<v8::String> source_str = v8_str("function foo() {}");
+  v8::ScriptCompiler::Source source(source_str, origin);
+  v8::Handle<v8::Script> script = v8::ScriptCompiler::Compile(
+      CcTest::isolate(), &source, v8::ScriptCompiler::kProduceParserCache);
+  CHECK(!script.IsEmpty());
+  const v8::ScriptCompiler::CachedData* original_cached_data =
+      source.GetCachedData();
+  CHECK(original_cached_data != NULL);
+  CHECK(original_cached_data->data != NULL);
+  CHECK(!original_cached_data->rejected);
+  CHECK_GT(original_cached_data->length, 0);
+  // Recompiling the same script with it won't reject the data.
+  {
+    v8::ScriptCompiler::Source source_with_cached_data(
+        source_str, origin,
+        new v8::ScriptCompiler::CachedData(original_cached_data->data,
+                                           original_cached_data->length));
+    v8::Handle<v8::Script> script =
+        v8::ScriptCompiler::Compile(CcTest::isolate(), &source_with_cached_data,
+                                    v8::ScriptCompiler::kConsumeParserCache);
+    CHECK(!script.IsEmpty());
+    const v8::ScriptCompiler::CachedData* new_cached_data =
+        source_with_cached_data.GetCachedData();
+    CHECK(new_cached_data != NULL);
+    CHECK(!new_cached_data->rejected);
+  }
+  // Compile an incompatible script with the cached data. The new script doesn't
+  // have the same starting position for the function as the old one, so the old
+  // cached data will be incompatible with it and will be rejected.
+  {
+    v8::Local<v8::String> incompatible_source_str =
+        v8_str("   function foo() {}");
+    v8::ScriptCompiler::Source source_with_cached_data(
+        incompatible_source_str, origin,
+        new v8::ScriptCompiler::CachedData(original_cached_data->data,
+                                           original_cached_data->length));
+    v8::Handle<v8::Script> script =
+        v8::ScriptCompiler::Compile(CcTest::isolate(), &source_with_cached_data,
+                                    v8::ScriptCompiler::kConsumeParserCache);
+    CHECK(!script.IsEmpty());
+    const v8::ScriptCompiler::CachedData* new_cached_data =
+        source_with_cached_data.GetCachedData();
+    CHECK(new_cached_data != NULL);
+    CHECK(new_cached_data->rejected);
+  }
+}
+
+
 TEST(StringConcatOverflow) {
   v8::V8::Initialize();
   v8::HandleScope scope(CcTest::isolate());