Soft fail for invalid cache data.

API=ScriptCompiler::CachedData::rejected
LOG=Y
R=vogelheim@google.com, vogelheim@chromium.org

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

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

diff --git a/include/v8.h b/include/v8.h
index e2dee5d..13e44f7 100644 (file)
--- a/include/v8.h
+++ b/include/v8.h
@@ -1045,7 +1045,11 @@ class V8_EXPORT ScriptCompiler {
       BufferOwned
     };
 
-    CachedData() : data(NULL), length(0), buffer_policy(BufferNotOwned) {}
+    CachedData()
+        : data(NULL),
+          length(0),
+          rejected(false),
+          buffer_policy(BufferNotOwned) {}
 
     // If buffer_policy is BufferNotOwned, the caller keeps the ownership of
     // data and guarantees that it stays alive until the CachedData object is
@@ -1058,6 +1062,7 @@ class V8_EXPORT ScriptCompiler {
     // which will be called when V8 no longer needs the data.
     const uint8_t* data;
     int length;
+    bool rejected;
     BufferPolicy buffer_policy;
 
    private:

diff --git a/src/api.cc b/src/api.cc
index c7ebc77..a8662d0 100644 (file)
--- a/src/api.cc
+++ b/src/api.cc
@@ -1536,7 +1536,10 @@ void ObjectTemplate::SetInternalFieldCount(int value) {
 
 ScriptCompiler::CachedData::CachedData(const uint8_t* data_, int length_,
                                        BufferPolicy buffer_policy_)
-    : data(data_), length(length_), buffer_policy(buffer_policy_) {}
+    : data(data_),
+      length(length_),
+      rejected(false),
+      buffer_policy(buffer_policy_) {}
 
 
 ScriptCompiler::CachedData::~CachedData() {
@@ -1752,6 +1755,8 @@ Local<UnboundScript> ScriptCompiler::CompileUnbound(
       source->cached_data = new CachedData(
           script_data->data(), script_data->length(), CachedData::BufferOwned);
       script_data->ReleaseDataOwnership();
+    } else if (options == kConsumeParserCache || options == kConsumeCodeCache) {
+      source->cached_data->rejected = script_data->rejected();
     }
     delete script_data;
   }

diff --git a/src/compiler.cc b/src/compiler.cc
index f52d6fc..b85a830 100644 (file)
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -34,7 +34,7 @@ namespace internal {
 
 
 ScriptData::ScriptData(const byte* data, int length)
-    : owns_data_(false), data_(data), length_(length) {
+    : owns_data_(false), rejected_(false), data_(data), length_(length) {
   if (!IsAligned(reinterpret_cast<intptr_t>(data), kPointerAlignment)) {
     byte* copy = NewArray<byte>(length);
     DCHECK(IsAligned(reinterpret_cast<intptr_t>(copy), kPointerAlignment));

diff --git a/src/compiler.h b/src/compiler.h
index 822151d..c8073fe 100644 (file)
--- a/src/compiler.h
+++ b/src/compiler.h
@@ -39,6 +39,9 @@ class ScriptData {
 
   const byte* data() const { return data_; }
   int length() const { return length_; }
+  bool rejected() const { return rejected_; }
+
+  void Reject() { rejected_ = true; }
 
   void AcquireDataOwnership() {
     DCHECK(!owns_data_);
@@ -51,7 +54,8 @@ class ScriptData {
   }
 
  private:
-  bool owns_data_;
+  bool owns_data_ : 1;
+  bool rejected_ : 1;
   const byte* data_;
   int length_;
 

diff --git a/src/parser.cc b/src/parser.cc
index 0301e40..63556cf 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -202,6 +202,7 @@ int ParseData::FunctionCount() {
 
 
 bool ParseData::IsSane() {
+  if (!IsAligned(script_data_->length(), sizeof(unsigned))) return false;
   // Check that the header data is valid and doesn't specify
   // point to positions outside the store.
   int data_length = Length();
@@ -256,7 +257,7 @@ void Parser::SetCachedData() {
   } else {
     DCHECK(info_->cached_data() != NULL);
     if (compile_options() == ScriptCompiler::kConsumeParserCache) {
-      cached_parse_data_ = new ParseData(*info_->cached_data());
+      cached_parse_data_ = ParseData::FromCachedData(*info_->cached_data());
     }
   }
 }
@@ -841,9 +842,9 @@ FunctionLiteral* Parser::ParseProgram() {
   CompleteParserRecorder recorder;
 
   debug_saved_compile_options_ = compile_options();
-  if (compile_options() == ScriptCompiler::kProduceParserCache) {
+  if (produce_cached_parse_data()) {
     log_ = &recorder;
-  } else if (compile_options() == ScriptCompiler::kConsumeParserCache) {
+  } else if (consume_cached_parse_data()) {
     cached_parse_data_->Initialize();
   }
 
@@ -884,7 +885,7 @@ FunctionLiteral* Parser::ParseProgram() {
     }
     PrintF(" - took %0.3f ms]\n", ms);
   }
-  if (compile_options() == ScriptCompiler::kProduceParserCache) {
+  if (produce_cached_parse_data()) {
     if (result != NULL) *info_->cached_data() = recorder.GetScriptData();
     log_ = NULL;
   }
@@ -3753,12 +3754,10 @@ void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
   CHECK(materialized_literal_count);
   CHECK(expected_property_count);
   CHECK(debug_saved_compile_options_ == compile_options());
-  if (compile_options() == ScriptCompiler::kProduceParserCache) {
-    CHECK(log_);
-  }
+  if (produce_cached_parse_data()) CHECK(log_);
 
   int function_block_pos = position();
-  if (compile_options() == ScriptCompiler::kConsumeParserCache) {
+  if (consume_cached_parse_data()) {
     // 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 =
@@ -3806,7 +3805,7 @@ void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
     *materialized_literal_count = logger.literals();
     *expected_property_count = logger.properties();
     scope_->SetStrictMode(logger.strict_mode());
-    if (compile_options() == ScriptCompiler::kProduceParserCache) {
+    if (produce_cached_parse_data()) {
       DCHECK(log_);
       // Position right after terminal '}'.
       int body_end = scanner()->location().end_pos;
@@ -4982,9 +4981,7 @@ void Parser::ParseOnBackground() {
 
   CompleteParserRecorder recorder;
   debug_saved_compile_options_ = compile_options();
-  if (compile_options() == ScriptCompiler::kProduceParserCache) {
-    log_ = &recorder;
-  }
+  if (produce_cached_parse_data()) log_ = &recorder;
 
   DCHECK(info()->source_stream() != NULL);
   ExternalStreamingStream stream(info()->source_stream(),
@@ -5012,7 +5009,7 @@ void Parser::ParseOnBackground() {
   // We cannot internalize on a background thread; a foreground task will take
   // care of calling Parser::Internalize just before compilation.
 
-  if (compile_options() == ScriptCompiler::kProduceParserCache) {
+  if (produce_cached_parse_data()) {
     if (result != NULL) *info_->cached_data() = recorder.GetScriptData();
     log_ = NULL;
   }

diff --git a/src/parser.h b/src/parser.h
index 1650cba..2feb8fd 100644 (file)
--- a/src/parser.h
+++ b/src/parser.h
@@ -62,10 +62,14 @@ class FunctionEntry BASE_EMBEDDED {
 // Wrapper around ScriptData to provide parser-specific functionality.
 class ParseData {
  public:
-  explicit ParseData(ScriptData* script_data) : script_data_(script_data) {
-    CHECK(IsAligned(script_data->length(), sizeof(unsigned)));
-    CHECK(IsSane());
+  static ParseData* FromCachedData(ScriptData* cached_data) {
+    ParseData* pd = new ParseData(cached_data);
+    if (pd->IsSane()) return pd;
+    cached_data->Reject();
+    delete pd;
+    return NULL;
   }
+
   void Initialize();
   FunctionEntry GetFunctionEntry(int start);
   int FunctionCount();
@@ -77,6 +81,8 @@ class ParseData {
   }
 
  private:
+  explicit ParseData(ScriptData* script_data) : script_data_(script_data) {}
+
   bool IsSane();
   unsigned Magic();
   unsigned Version();
@@ -688,6 +694,13 @@ class Parser : public ParserBase<ParserTraits> {
   ScriptCompiler::CompileOptions compile_options() const {
     return info_->compile_options();
   }
+  bool consume_cached_parse_data() const {
+    return compile_options() == ScriptCompiler::kConsumeParserCache &&
+           cached_parse_data_ != NULL;
+  }
+  bool produce_cached_parse_data() const {
+    return compile_options() == ScriptCompiler::kProduceParserCache;
+  }
   Scope* DeclarationScope(VariableMode mode) {
     return IsLexicalVariableMode(mode)
         ? scope_ : scope_->DeclarationScope();

diff --git a/src/serialize.cc b/src/serialize.cc
index 6347943..090eeb4 100644 (file)
--- a/src/serialize.cc
+++ b/src/serialize.cc
@@ -2185,7 +2185,7 @@ void CodeSerializer::SerializeSourceObject(HowToCode how_to_code,
 
 
 MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
-    Isolate* isolate, ScriptData* data, Handle<String> source) {
+    Isolate* isolate, ScriptData* cached_data, Handle<String> source) {
   base::ElapsedTimer timer;
   if (FLAG_profile_deserialization) timer.Start();
 
@@ -2194,18 +2194,24 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
   {
     HandleScope scope(isolate);
 
-    SerializedCodeData scd(data, *source);
-    SnapshotByteSource payload(scd.Payload(), scd.PayloadLength());
+    SerializedCodeData* scd =
+        SerializedCodeData::FromCachedData(cached_data, *source);
+    if (scd == NULL) {
+      if (FLAG_profile_deserialization) PrintF("[Cached code failed check]\n");
+      DCHECK(cached_data->rejected());
+      return MaybeHandle<SharedFunctionInfo>();
+    }
+    SnapshotByteSource payload(scd->Payload(), scd->PayloadLength());
     Deserializer deserializer(&payload);
 
     // Eagerly expand string table to avoid allocations during deserialization.
-    StringTable::EnsureCapacityForDeserialization(isolate,
-                                                  scd.NumInternalizedStrings());
+    StringTable::EnsureCapacityForDeserialization(
+        isolate, scd->NumInternalizedStrings());
 
     // Set reservations.
     STATIC_ASSERT(NEW_SPACE == 0);
     int current_space = NEW_SPACE;
-    Vector<const SerializedCodeData::Reservation> res = scd.Reservations();
+    Vector<const SerializedCodeData::Reservation> res = scd->Reservations();
     for (const auto& r : res) {
       deserializer.AddReservation(current_space, r.chunk_size());
       if (r.is_last_chunk()) current_space++;
@@ -2213,7 +2219,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
     DCHECK_EQ(kNumberOfSpaces, current_space);
 
     // Prepare and register list of attached objects.
-    Vector<const uint32_t> code_stub_keys = scd.CodeStubKeys();
+    Vector<const uint32_t> code_stub_keys = scd->CodeStubKeys();
     Vector<Handle<Object> > attached_objects = Vector<Handle<Object> >::New(
         code_stub_keys.length() + kCodeStubsBaseIndex);
     attached_objects[kSourceObjectIndex] = source;
@@ -2235,7 +2241,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
 
   if (FLAG_profile_deserialization) {
     double ms = timer.Elapsed().InMillisecondsF();
-    int length = data->length();
+    int length = cached_data->length();
     PrintF("[Deserializing from %d bytes took %0.3f ms]\n", length, ms);
   }
   Handle<SharedFunctionInfo> result(SharedFunctionInfo::cast(root), isolate);
@@ -2315,11 +2321,6 @@ bool SerializedCodeData::IsSane(String* source) {
 
 
 int SerializedCodeData::CheckSum(String* string) {
-  int checksum = Version::Hash();
-#ifdef DEBUG
-  uint32_t seed = static_cast<uint32_t>(checksum);
-  checksum = static_cast<int>(IteratingStringHasher::Hash(string, seed));
-#endif  // DEBUG
-  return checksum;
+  return Version::Hash() ^ string->length();
 }
 } }  // namespace v8::internal

diff --git a/src/serialize.h b/src/serialize.h
index d7a6c7f..1c78113 100644 (file)
--- a/src/serialize.h
+++ b/src/serialize.h
@@ -706,7 +706,7 @@ class CodeSerializer : public Serializer {
                                Handle<String> source);
 
   MUST_USE_RESULT static MaybeHandle<SharedFunctionInfo> Deserialize(
-      Isolate* isolate, ScriptData* data, Handle<String> source);
+      Isolate* isolate, ScriptData* cached_data, Handle<String> source);
 
   static const int kSourceObjectIndex = 0;
   static const int kCodeStubsBaseIndex = 1;
@@ -757,10 +757,14 @@ class CodeSerializer : public Serializer {
 class SerializedCodeData {
  public:
   // Used by when consuming.
-  explicit SerializedCodeData(ScriptData* data, String* source)
-      : script_data_(data), owns_script_data_(false) {
+  static SerializedCodeData* FromCachedData(ScriptData* cached_data,
+                                            String* source) {
     DisallowHeapAllocation no_gc;
-    CHECK(IsSane(source));
+    SerializedCodeData* scd = new SerializedCodeData(cached_data);
+    if (scd->IsSane(source)) return scd;
+    cached_data->Reject();
+    delete scd;
+    return NULL;
   }
 
   // Used when producing.
@@ -822,6 +826,9 @@ class SerializedCodeData {
   }
 
  private:
+  explicit SerializedCodeData(ScriptData* data)
+      : script_data_(data), owns_script_data_(false) {}
+
   void SetHeaderValue(int offset, int value) {
     reinterpret_cast<int*>(const_cast<byte*>(script_data_->data()))[offset] =
         value;

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 98c17de..8ef6a44 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -24207,3 +24207,28 @@ TEST(StreamingUtf8ScriptWithMultipleMultibyteCharactersSomeSplit2) {
   const char* chunks[] = {chunk1, chunk2, "foo();", NULL};
   RunStreamingTest(chunks, v8::ScriptCompiler::StreamedSource::UTF8);
 }
+
+
+void TestInvalidCacheData(v8::ScriptCompiler::CompileOptions option) {
+  const char* garbage = "garbage garbage garbage garbage.";
+  const uint8_t* data = reinterpret_cast<const uint8_t*>(garbage);
+  int length = 16;
+  v8::ScriptCompiler::CachedData* cached_data =
+      new v8::ScriptCompiler::CachedData(data, length);
+  DCHECK(!cached_data->rejected);
+  v8::ScriptOrigin origin(v8_str("origin"));
+  v8::ScriptCompiler::Source source(v8_str("42"), origin, cached_data);
+  v8::Handle<v8::Script> script =
+      v8::ScriptCompiler::Compile(CcTest::isolate(), &source, option);
+  CHECK(cached_data->rejected);
+  CHECK_EQ(42, script->Run()->Int32Value());
+}
+
+
+TEST(InvalidCacheData) {
+  v8::V8::Initialize();
+  v8::HandleScope scope(CcTest::isolate());
+  LocalContext context;
+  TestInvalidCacheData(v8::ScriptCompiler::kConsumeParserCache);
+  TestInvalidCacheData(v8::ScriptCompiler::kConsumeCodeCache);
+}

diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index 0ae46c3..9d53482 100644 (file)
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -454,14 +454,14 @@ TEST(Regress928) {
   i::PreParser::PreParseResult result = preparser.PreParseProgram();
   CHECK_EQ(i::PreParser::kPreParseSuccess, result);
   i::ScriptData* sd = log.GetScriptData();
-  i::ParseData pd(sd);
-  pd.Initialize();
+  i::ParseData* pd = i::ParseData::FromCachedData(sd);
+  pd->Initialize();
 
   int first_function =
       static_cast<int>(strstr(program, "function") - program);
   int first_lbrace = first_function + i::StrLength("function () ");
   CHECK_EQ('{', program[first_lbrace]);
-  i::FunctionEntry entry1 = pd.GetFunctionEntry(first_lbrace);
+  i::FunctionEntry entry1 = pd->GetFunctionEntry(first_lbrace);
   CHECK(!entry1.is_valid());
 
   int second_function =
@@ -469,10 +469,11 @@ TEST(Regress928) {
   int second_lbrace =
       second_function + i::StrLength("function () ");
   CHECK_EQ('{', program[second_lbrace]);
-  i::FunctionEntry entry2 = pd.GetFunctionEntry(second_lbrace);
+  i::FunctionEntry entry2 = pd->GetFunctionEntry(second_lbrace);
   CHECK(entry2.is_valid());
   CHECK_EQ('}', program[entry2.end_pos() - 1]);
   delete sd;
+  delete pd;
 }
 
 
@@ -2451,17 +2452,18 @@ TEST(DontRegressPreParserDataSizes) {
     i::ScriptData* sd = NULL;
     info.SetCachedData(&sd, v8::ScriptCompiler::kProduceParserCache);
     i::Parser::Parse(&info, true);
-    i::ParseData pd(sd);
+    i::ParseData* pd = i::ParseData::FromCachedData(sd);
 
-    if (pd.FunctionCount() != test_cases[i].functions) {
+    if (pd->FunctionCount() != test_cases[i].functions) {
       v8::base::OS::Print(
           "Expected preparse data for program:\n"
           "\t%s\n"
           "to contain %d functions, however, received %d functions.\n",
-          program, test_cases[i].functions, pd.FunctionCount());
+          program, test_cases[i].functions, pd->FunctionCount());
       CHECK(false);
     }
     delete sd;
+    delete pd;
   }
 }
 

diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
index fbd7fcf..2acaabc 100644 (file)
--- a/test/cctest/test-serialize.cc
+++ b/test/cctest/test-serialize.cc
@@ -1231,6 +1231,7 @@ TEST(SerializeToplevelIsolates) {
       script = v8::ScriptCompiler::CompileUnbound(
           isolate2, &source, v8::ScriptCompiler::kConsumeCodeCache);
     }
+    CHECK(!cache->rejected);
     v8::Local<v8::Value> result = script->BindToCurrentContext()->Run();
     CHECK(result->ToString()->Equals(v8_str("abcdef")));
   }