Revert "Soft fail for invalid cache data."

This reverts commit eafce666f49f13011849b6c0c40b271676ec91cf.

Original commit failed some tests w/ memory leaks.

TBR=yangguo@chromium.org
BUG=

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

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

diff --git a/include/v8.h b/include/v8.h
index 13e44f7..e2dee5d 100644 (file)
--- a/include/v8.h
+++ b/include/v8.h
@@ -1045,11 +1045,7 @@ class V8_EXPORT ScriptCompiler {
       BufferOwned
     };
 
-    CachedData()
-        : data(NULL),
-          length(0),
-          rejected(false),
-          buffer_policy(BufferNotOwned) {}
+    CachedData() : data(NULL), length(0), 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
@@ -1062,7 +1058,6 @@ 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 a8662d0..c7ebc77 100644 (file)
--- a/src/api.cc
+++ b/src/api.cc
@@ -1536,10 +1536,7 @@ void ObjectTemplate::SetInternalFieldCount(int value) {
 
 ScriptCompiler::CachedData::CachedData(const uint8_t* data_, int length_,
                                        BufferPolicy buffer_policy_)
-    : data(data_),
-      length(length_),
-      rejected(false),
-      buffer_policy(buffer_policy_) {}
+    : data(data_), length(length_), buffer_policy(buffer_policy_) {}
 
 
 ScriptCompiler::CachedData::~CachedData() {
@@ -1755,8 +1752,6 @@ 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 b85a830..f52d6fc 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), rejected_(false), data_(data), length_(length) {
+    : owns_data_(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 c8073fe..822151d 100644 (file)
--- a/src/compiler.h
+++ b/src/compiler.h
@@ -39,9 +39,6 @@ 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_);
@@ -54,8 +51,7 @@ class ScriptData {
   }
 
  private:
-  bool owns_data_ : 1;
-  bool rejected_ : 1;
+  bool owns_data_;
   const byte* data_;
   int length_;
 

diff --git a/src/parser.cc b/src/parser.cc
index 63556cf..0301e40 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -202,7 +202,6 @@ 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();
@@ -257,7 +256,7 @@ void Parser::SetCachedData() {
   } else {
     DCHECK(info_->cached_data() != NULL);
     if (compile_options() == ScriptCompiler::kConsumeParserCache) {
-      cached_parse_data_ = ParseData::FromCachedData(*info_->cached_data());
+      cached_parse_data_ = new ParseData(*info_->cached_data());
     }
   }
 }
@@ -842,9 +841,9 @@ FunctionLiteral* Parser::ParseProgram() {
   CompleteParserRecorder recorder;
 
   debug_saved_compile_options_ = compile_options();
-  if (produce_cached_parse_data()) {
+  if (compile_options() == ScriptCompiler::kProduceParserCache) {
     log_ = &recorder;
-  } else if (consume_cached_parse_data()) {
+  } else if (compile_options() == ScriptCompiler::kConsumeParserCache) {
     cached_parse_data_->Initialize();
   }
 
@@ -885,7 +884,7 @@ FunctionLiteral* Parser::ParseProgram() {
     }
     PrintF(" - took %0.3f ms]\n", ms);
   }
-  if (produce_cached_parse_data()) {
+  if (compile_options() == ScriptCompiler::kProduceParserCache) {
     if (result != NULL) *info_->cached_data() = recorder.GetScriptData();
     log_ = NULL;
   }
@@ -3754,10 +3753,12 @@ void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
   CHECK(materialized_literal_count);
   CHECK(expected_property_count);
   CHECK(debug_saved_compile_options_ == compile_options());
-  if (produce_cached_parse_data()) CHECK(log_);
+  if (compile_options() == ScriptCompiler::kProduceParserCache) {
+    CHECK(log_);
+  }
 
   int function_block_pos = position();
-  if (consume_cached_parse_data()) {
+  if (compile_options() == ScriptCompiler::kConsumeParserCache) {
     // 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 =
@@ -3805,7 +3806,7 @@ void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
     *materialized_literal_count = logger.literals();
     *expected_property_count = logger.properties();
     scope_->SetStrictMode(logger.strict_mode());
-    if (produce_cached_parse_data()) {
+    if (compile_options() == ScriptCompiler::kProduceParserCache) {
       DCHECK(log_);
       // Position right after terminal '}'.
       int body_end = scanner()->location().end_pos;
@@ -4981,7 +4982,9 @@ void Parser::ParseOnBackground() {
 
   CompleteParserRecorder recorder;
   debug_saved_compile_options_ = compile_options();
-  if (produce_cached_parse_data()) log_ = &recorder;
+  if (compile_options() == ScriptCompiler::kProduceParserCache) {
+    log_ = &recorder;
+  }
 
   DCHECK(info()->source_stream() != NULL);
   ExternalStreamingStream stream(info()->source_stream(),
@@ -5009,7 +5012,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 (produce_cached_parse_data()) {
+  if (compile_options() == ScriptCompiler::kProduceParserCache) {
     if (result != NULL) *info_->cached_data() = recorder.GetScriptData();
     log_ = NULL;
   }

diff --git a/src/parser.h b/src/parser.h
index 2feb8fd..1650cba 100644 (file)
--- a/src/parser.h
+++ b/src/parser.h
@@ -62,14 +62,10 @@ class FunctionEntry BASE_EMBEDDED {
 // Wrapper around ScriptData to provide parser-specific functionality.
 class ParseData {
  public:
-  static ParseData* FromCachedData(ScriptData* cached_data) {
-    ParseData* pd = new ParseData(cached_data);
-    if (pd->IsSane()) return pd;
-    cached_data->Reject();
-    delete pd;
-    return NULL;
+  explicit ParseData(ScriptData* script_data) : script_data_(script_data) {
+    CHECK(IsAligned(script_data->length(), sizeof(unsigned)));
+    CHECK(IsSane());
   }
-
   void Initialize();
   FunctionEntry GetFunctionEntry(int start);
   int FunctionCount();
@@ -81,8 +77,6 @@ class ParseData {
   }
 
  private:
-  explicit ParseData(ScriptData* script_data) : script_data_(script_data) {}
-
   bool IsSane();
   unsigned Magic();
   unsigned Version();
@@ -694,13 +688,6 @@ 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 090eeb4..6347943 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* cached_data, Handle<String> source) {
+    Isolate* isolate, ScriptData* data, Handle<String> source) {
   base::ElapsedTimer timer;
   if (FLAG_profile_deserialization) timer.Start();
 
@@ -2194,24 +2194,18 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
   {
     HandleScope scope(isolate);
 
-    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());
+    SerializedCodeData scd(data, *source);
+    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++;
@@ -2219,7 +2213,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;
@@ -2241,7 +2235,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
 
   if (FLAG_profile_deserialization) {
     double ms = timer.Elapsed().InMillisecondsF();
-    int length = cached_data->length();
+    int length = data->length();
     PrintF("[Deserializing from %d bytes took %0.3f ms]\n", length, ms);
   }
   Handle<SharedFunctionInfo> result(SharedFunctionInfo::cast(root), isolate);
@@ -2321,6 +2315,11 @@ bool SerializedCodeData::IsSane(String* source) {
 
 
 int SerializedCodeData::CheckSum(String* string) {
-  return Version::Hash() ^ string->length();
+  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;
 }
 } }  // namespace v8::internal

diff --git a/src/serialize.h b/src/serialize.h
index 1c78113..d7a6c7f 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* cached_data, Handle<String> source);
+      Isolate* isolate, ScriptData* data, Handle<String> source);
 
   static const int kSourceObjectIndex = 0;
   static const int kCodeStubsBaseIndex = 1;
@@ -757,14 +757,10 @@ class CodeSerializer : public Serializer {
 class SerializedCodeData {
  public:
   // Used by when consuming.
-  static SerializedCodeData* FromCachedData(ScriptData* cached_data,
-                                            String* source) {
+  explicit SerializedCodeData(ScriptData* data, String* source)
+      : script_data_(data), owns_script_data_(false) {
     DisallowHeapAllocation no_gc;
-    SerializedCodeData* scd = new SerializedCodeData(cached_data);
-    if (scd->IsSane(source)) return scd;
-    cached_data->Reject();
-    delete scd;
-    return NULL;
+    CHECK(IsSane(source));
   }
 
   // Used when producing.
@@ -826,9 +822,6 @@ 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 8ef6a44..98c17de 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -24207,28 +24207,3 @@ 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 9d53482..0ae46c3 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 = i::ParseData::FromCachedData(sd);
-  pd->Initialize();
+  i::ParseData pd(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,11 +469,10 @@ 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;
 }
 
 
@@ -2452,18 +2451,17 @@ TEST(DontRegressPreParserDataSizes) {
     i::ScriptData* sd = NULL;
     info.SetCachedData(&sd, v8::ScriptCompiler::kProduceParserCache);
     i::Parser::Parse(&info, true);
-    i::ParseData* pd = i::ParseData::FromCachedData(sd);
+    i::ParseData pd(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 2acaabc..fbd7fcf 100644 (file)
--- a/test/cctest/test-serialize.cc
+++ b/test/cctest/test-serialize.cc
@@ -1231,7 +1231,6 @@ 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")));
   }