From eafce666f49f13011849b6c0c40b271676ec91cf Mon Sep 17 00:00:00 2001 From: Yang Guo Date: Thu, 13 Nov 2014 16:42:40 +0100 Subject: [PATCH] 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} --- include/v8.h | 7 ++++++- src/api.cc | 7 ++++++- src/compiler.cc | 2 +- src/compiler.h | 6 +++++- src/parser.cc | 23 ++++++++++------------- src/parser.h | 19 ++++++++++++++++--- src/serialize.cc | 29 +++++++++++++++-------------- src/serialize.h | 15 +++++++++++---- test/cctest/test-api.cc | 25 +++++++++++++++++++++++++ test/cctest/test-parsing.cc | 16 +++++++++------- test/cctest/test-serialize.cc | 1 + 11 files changed, 105 insertions(+), 45 deletions(-) diff --git a/include/v8.h b/include/v8.h index e2dee5d..13e44f7 100644 --- 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 --- 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 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 --- 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(data), kPointerAlignment)) { byte* copy = NewArray(length); DCHECK(IsAligned(reinterpret_cast(copy), kPointerAlignment)); diff --git a/src/compiler.h b/src/compiler.h index 822151d..c8073fe 100644 --- 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 --- 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 --- 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 { 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 --- a/src/serialize.cc +++ b/src/serialize.cc @@ -2185,7 +2185,7 @@ void CodeSerializer::SerializeSourceObject(HowToCode how_to_code, MaybeHandle CodeSerializer::Deserialize( - Isolate* isolate, ScriptData* data, Handle source) { + Isolate* isolate, ScriptData* cached_data, Handle source) { base::ElapsedTimer timer; if (FLAG_profile_deserialization) timer.Start(); @@ -2194,18 +2194,24 @@ MaybeHandle 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(); + } + 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 res = scd.Reservations(); + Vector 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 CodeSerializer::Deserialize( DCHECK_EQ(kNumberOfSpaces, current_space); // Prepare and register list of attached objects. - Vector code_stub_keys = scd.CodeStubKeys(); + Vector code_stub_keys = scd->CodeStubKeys(); Vector > attached_objects = Vector >::New( code_stub_keys.length() + kCodeStubsBaseIndex); attached_objects[kSourceObjectIndex] = source; @@ -2235,7 +2241,7 @@ MaybeHandle 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 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(checksum); - checksum = static_cast(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 --- a/src/serialize.h +++ b/src/serialize.h @@ -706,7 +706,7 @@ class CodeSerializer : public Serializer { Handle source); MUST_USE_RESULT static MaybeHandle Deserialize( - Isolate* isolate, ScriptData* data, Handle source); + Isolate* isolate, ScriptData* cached_data, Handle 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(const_cast(script_data_->data()))[offset] = value; diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 98c17de..8ef6a44 100644 --- 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(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 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 --- 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(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 --- 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 result = script->BindToCurrentContext()->Run(); CHECK(result->ToString()->Equals(v8_str("abcdef"))); } -- 2.7.4