Fail the compilation if the cached data is invalid.

R=svenpanne@chromium.org
BUG=

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20703 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -1694,40 +1694,44 @@ Local<UnboundScript> ScriptCompiler::CompileUnbound(
     CompileOptions options) {
   i::ScriptData* script_data_impl = NULL;
   i::CachedDataMode cached_data_mode = i::NO_CACHED_DATA;
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  ON_BAILOUT(isolate, "v8::ScriptCompiler::CompileUnbound()",
+             return Local<UnboundScript>());
   if (options & kProduceDataToCache) {
     cached_data_mode = i::PRODUCE_CACHED_DATA;
     ASSERT(source->cached_data == NULL);
     if (source->cached_data) {
       // Asked to produce cached data even though there is some already -> not
-      // good. In release mode, try to do the right thing: Just regenerate the
-      // data.
-      delete source->cached_data;
-      source->cached_data = NULL;
+      // good. Fail the compilation.
+      EXCEPTION_PREAMBLE(isolate);
+      i::Handle<i::Object> result = isolate->factory()->NewSyntaxError(
+          "invalid_cached_data", isolate->factory()->NewJSArray(0));
+      isolate->Throw(*result);
+      isolate->ReportPendingMessages();
+      has_pending_exception = true;
+      EXCEPTION_BAILOUT_CHECK(isolate, Local<UnboundScript>());
     }
   } else if (source->cached_data) {
+    cached_data_mode = i::CONSUME_CACHED_DATA;
     // ScriptData takes care of aligning, in case the data is not aligned
     // correctly.
     script_data_impl = i::ScriptData::New(
         reinterpret_cast<const char*>(source->cached_data->data),
         source->cached_data->length);
-    // We assert that the pre-data is sane, even though we can actually
-    // handle it if it turns out not to be in release mode.
-    ASSERT(script_data_impl->SanityCheck());
-    if (script_data_impl->SanityCheck()) {
-      cached_data_mode = i::CONSUME_CACHED_DATA;
-    } else {
-      // If the pre-data isn't sane we simply ignore it.
+    // If the cached data is not valid, fail the compilation.
+    if (script_data_impl == NULL || !script_data_impl->SanityCheck()) {
+      EXCEPTION_PREAMBLE(isolate);
+      i::Handle<i::Object> result = isolate->factory()->NewSyntaxError(
+          "invalid_cached_data", isolate->factory()->NewJSArray(0));
+      isolate->Throw(*result);
+      isolate->ReportPendingMessages();
       delete script_data_impl;
-      script_data_impl = NULL;
-      delete source->cached_data;
-      source->cached_data = NULL;
+      has_pending_exception = true;
+      EXCEPTION_BAILOUT_CHECK(isolate, Local<UnboundScript>());
     }
   }
 
   i::Handle<i::String> str = Utils::OpenHandle(*(source->source_string));
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
-  ON_BAILOUT(isolate, "v8::ScriptCompiler::CompileUnbound()",
-             return Local<UnboundScript>());
   LOG_API(isolate, "ScriptCompiler::CompileUnbound");
   ENTER_V8(isolate);
   i::SharedFunctionInfo* raw_result = NULL;

src/messages.js

@@ -154,7 +154,8 @@ var kMessages = {
   array_indexof_not_defined:     ["Array.getIndexOf: Argument undefined"],
   object_not_extensible:         ["Can't add property ", "%0", ", object is not extensible"],
   illegal_access:                ["Illegal access"],
-  invalid_preparser_data:        ["Invalid preparser data for function ", "%0"],
+  invalid_cached_data_function:  ["Invalid cached data for function ", "%0"],
+  invalid_cached_data:           ["Invalid cached data"],
   strict_mode_with:              ["Strict mode code may not include a with statement"],
   strict_eval_arguments:         ["Unexpected eval or arguments in strict mode"],
   too_many_arguments:            ["Too many arguments in function call (only 65535 allowed)"],

src/parser.cc

@@ -227,15 +227,13 @@ Handle<String> Parser::LookupCachedSymbol(int symbol_id) {
 ScriptData* ScriptData::New(const char* data, int length) {
   // The length is obviously invalid.
   if (length % sizeof(unsigned) != 0) {
-    return new ScriptData();
+    return NULL;
   }
 
   int deserialized_data_length = length / sizeof(unsigned);
   unsigned* deserialized_data;
-  ScriptData* script_data = new ScriptData();
-  script_data->owns_store_ =
-      reinterpret_cast<intptr_t>(data) % sizeof(unsigned) != 0;
-  if (script_data->owns_store_) {
+  bool owns_store = reinterpret_cast<intptr_t>(data) % sizeof(unsigned) != 0;
+  if (owns_store) {
     // Copy the data to align it.
     deserialized_data = i::NewArray<unsigned>(deserialized_data_length);
     i::CopyBytes(reinterpret_cast<char*>(deserialized_data),
@@ -244,9 +242,9 @@ ScriptData* ScriptData::New(const char* data, int length) {
     // If aligned, don't create a copy of the data.
     deserialized_data = reinterpret_cast<unsigned*>(const_cast<char*>(data));
   }
-  script_data->store_ =
-      Vector<unsigned>(deserialized_data, deserialized_data_length);
-  return script_data;
+  return new ScriptData(
+      Vector<unsigned>(deserialized_data, deserialized_data_length),
+      owns_store);
 }
 
 
@@ -3138,10 +3136,10 @@ DebuggerStatement* Parser::ParseDebuggerStatement(bool* ok) {
 }
 
 
-void Parser::ReportInvalidPreparseData(Handle<String> name, bool* ok) {
+void Parser::ReportInvalidCachedData(Handle<String> name, bool* ok) {
   SmartArrayPointer<char> name_string = name->ToCString(DISALLOW_NULLS);
   const char* element[1] = { name_string.get() };
-  ReportMessage("invalid_preparser_data",
+  ReportMessage("invalid_cached_data_function",
                 Vector<const char*>(element, 1));
   *ok = false;
 }
@@ -3402,7 +3400,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
           if (entry.end_pos() <= function_block_pos) {
             // End position greater than end of stream is safe, and hard
             // to check.
-            ReportInvalidPreparseData(function_name, CHECK_OK);
+            ReportInvalidCachedData(function_name, CHECK_OK);
           }
           scanner()->SeekForward(entry.end_pos() - 1);
 
@@ -3415,13 +3413,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
           scope_->SetStrictMode(entry.strict_mode());
         } else {
           // This case happens when we have preparse data but it doesn't contain
-          // an entry for the function. As a safety net, fall back to eager
-          // parsing. It is unclear whether PreParser's laziness analysis can
-          // produce different results than the Parser's laziness analysis (see
-          // https://codereview.chromium.org/7565003 ). In this case, we must
-          // discard all the preparse data, since the symbol data will be wrong.
-          is_lazily_parsed = false;
-          cached_data_mode_ = NO_CACHED_DATA;
+          // an entry for the function. Fail the compilation.
+          ReportInvalidCachedData(function_name, CHECK_OK);
         }
       } else {
         // With no cached data, we partially parse the function, without

src/parser.h

@@ -88,9 +88,9 @@ class ScriptData {
       : store_(store),
         owns_store_(true) { }
 
-  // Create an empty ScriptData that is guaranteed to not satisfy
-  // a SanityCheck.
-  ScriptData() : owns_store_(false) { }
+  ScriptData(Vector<unsigned> store, bool owns_store)
+      : store_(store),
+        owns_store_(owns_store) { }
 
   // The created ScriptData won't take ownership of the data. If the alignment
   // is not correct, this will copy the data (and the created ScriptData will
@@ -667,7 +667,7 @@ class Parser : public ParserBase<ParserTraits> {
                                   Handle<String> source);
 
   // Report syntax error
-  void ReportInvalidPreparseData(Handle<String> name, bool* ok);
+  void ReportInvalidCachedData(Handle<String> name, bool* ok);
 
   void SetCachedData(ScriptData** data,
                      CachedDataMode cached_data_mode) {

test/cctest/test-api.cc

@@ -14858,10 +14858,7 @@ TEST(PreCompileDeserializationError) {
   const char* data = "DONT CARE";
   int invalid_size = 3;
   i::ScriptData* sd = i::ScriptData::New(data, invalid_size);
-
-  CHECK_EQ(0, sd->Length());
-
-  delete sd;
+  CHECK_EQ(NULL, sd);
 }
 
 
@@ -14911,16 +14908,18 @@ TEST(CompileWithInvalidCachedData) {
       v8::ScriptCompiler::CompileUnbound(isolate, &source2);
 
   CHECK(try_catch.HasCaught());
-  String::Utf8Value exception_value(try_catch.Message()->Get());
-  CHECK_EQ("Uncaught SyntaxError: Invalid preparser data for function bar",
-           *exception_value);
+  {
+    String::Utf8Value exception_value(try_catch.Message()->Get());
+    CHECK_EQ("Uncaught SyntaxError: Invalid cached data for function bar",
+             *exception_value);
+  }
 
   try_catch.Reset();
   delete sd;
 
-  // Overwrite function bar's start position with 200.  The function entry
-  // will not be found when searching for it by position and we should fall
-  // back on eager compilation.
+  // Overwrite function bar's start position with 200. The function entry will
+  // not be found when searching for it by position, and the compilation fails.
+
   // ScriptData does not take ownership of the buffers passed to it.
   sd = i::ScriptData::New(reinterpret_cast<const char*>(cd->data), cd->length);
   sd_data = reinterpret_cast<unsigned*>(const_cast<char*>(sd->Data()));
@@ -14934,7 +14933,34 @@ TEST(CompileWithInvalidCachedData) {
           reinterpret_cast<const uint8_t*>(sd->Data()), sd->Length()));
   compiled_script =
       v8::ScriptCompiler::CompileUnbound(isolate, &source3);
-  CHECK(!try_catch.HasCaught());
+  CHECK(try_catch.HasCaught());
+  {
+    String::Utf8Value exception_value(try_catch.Message()->Get());
+    CHECK_EQ("Uncaught SyntaxError: Invalid cached data for function bar",
+             *exception_value);
+  }
+  CHECK(compiled_script.IsEmpty());
+  try_catch.Reset();
+  delete sd;
+
+  // Try passing in cached data which is obviously invalid (wrong length).
+  sd = i::ScriptData::New(reinterpret_cast<const char*>(cd->data), cd->length);
+  const char* script4 =
+      "function foo(){ return 8;}\n"
+      "function bar(){ return 6 + 7;}  foo();";
+  v8::ScriptCompiler::Source source4(
+      v8_str(script4),
+      new v8::ScriptCompiler::CachedData(
+          reinterpret_cast<const uint8_t*>(sd->Data()), sd->Length() - 1));
+  compiled_script =
+      v8::ScriptCompiler::CompileUnbound(isolate, &source4);
+  CHECK(try_catch.HasCaught());
+  {
+    String::Utf8Value exception_value(try_catch.Message()->Get());
+    CHECK_EQ("Uncaught SyntaxError: Invalid cached data",
+             *exception_value);
+  }
+  CHECK(compiled_script.IsEmpty());
   delete sd;
 }