From 3d1a17c2ac87313ea37f788ed0e2a604d7f0d235 Mon Sep 17 00:00:00 2001 From: "marja@chromium.org" Date: Fri, 4 Apr 2014 12:36:23 +0000 Subject: [PATCH] Update tests to use the new compilation API + related fixes. Esp. get rid of PreCompile in tests, as it's going to be removed. Notes: - The new compilation API doesn't have a separate precompilation phase, so there is no separate way to check for errors except checking the compilation errors. Removed some tests which don't make sense any more. - test-api/Regress31661 didn't make sense as a regression test even before the compilation API changes, because Blink doesn't precompile this short scripts. So detecting this kind of errors (see crbug.com/31661 for more information) cannot rely on precompilation errors. - test-parsing/PreParserStrictOctal has nothing to do with PreParser, and the comment about "forcing preparsing" was just wrong. - test-api/PreCompile was supposed to test that "pre-compilation (aka preparsing) can be called without initializing the whole VM"; that's no longer true, since there's no separate precompilation step in the new compile API. There are other tests (test-parsing/DontRegressPreParserDataSizes) which ensure that we produce cached data. - Updated tests which test preparsing to use PreParser directly (not via the preparsing API). - In the new compilation API, the user doesn't need to deal with ScriptData ever. It's only used internally, and needed in tests that test internal aspects (e.g., modify the cached data before passing it back). - Some tests which used to test preparse + parse now test first time parse + second time parse, and had to be modified to ensure we don't hit the compilation cache. BUG= R=ulan@chromium.org Review URL: https://codereview.chromium.org/225743002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20511 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/parser.h | 7 ++ test/cctest/cctest.h | 18 ++-- test/cctest/test-api.cc | 113 ++++++++------------- test/cctest/test-parsing.cc | 195 ++++++++++++++++-------------------- tools/parser-shell.cc | 85 +++++++--------- 5 files changed, 175 insertions(+), 243 deletions(-) diff --git a/src/parser.h b/src/parser.h index cf7d2a5905..cb13f716e9 100644 --- a/src/parser.h +++ b/src/parser.h @@ -114,6 +114,13 @@ class ScriptDataImpl : public ScriptData { ? store_[PreparseDataConstants::kSymbolCountOffset] : 0; } + int function_count() { + int functions_size = + static_cast(store_[PreparseDataConstants::kFunctionsSizeOffset]); + if (functions_size < 0) return 0; + if (functions_size % FunctionEntry::kSize != 0) return 0; + return functions_size / FunctionEntry::kSize; + } // The following functions should only be called if SanityCheck has // returned true. bool has_error() { return store_[PreparseDataConstants::kHasErrorOffset]; } diff --git a/test/cctest/cctest.h b/test/cctest/cctest.h index 6359835235..b4e31f26df 100644 --- a/test/cctest/cctest.h +++ b/test/cctest/cctest.h @@ -346,19 +346,13 @@ static inline v8::Local CompileRun(v8::Local source) { static inline v8::Local PreCompileCompileRun(const char* source) { + // Compile once just to get the preparse data, then compile the second time + // using the data. v8::Isolate* isolate = v8::Isolate::GetCurrent(); - v8::Local source_string = - v8::String::NewFromUtf8(isolate, source); - v8::ScriptData* preparse = v8::ScriptData::PreCompile(source_string); - v8::ScriptCompiler::Source script_source( - source_string, new v8::ScriptCompiler::CachedData( - reinterpret_cast(preparse->Data()), - preparse->Length())); - v8::Local script = - v8::ScriptCompiler::Compile(isolate, &script_source); - v8::Local result = script->Run(); - delete preparse; - return result; + v8::ScriptCompiler::Source script_source(v8_str(source)); + v8::ScriptCompiler::Compile(isolate, &script_source, + v8::ScriptCompiler::kProduceDataToCache); + return v8::ScriptCompiler::Compile(isolate, &script_source)->Run(); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 52a71ac1b2..3c46185322 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -14823,74 +14823,32 @@ THREADED_TEST(TurnOnAccessCheckAndRecompile) { } -// This test verifies that pre-compilation (aka preparsing) can be called -// without initializing the whole VM. Thus we cannot run this test in a -// multi-threaded setup. -TEST(PreCompile) { - // TODO(155): This test would break without the initialization of V8. This is - // a workaround for now to make this test not fail. - v8::V8::Initialize(); - v8::Isolate* isolate = CcTest::isolate(); - HandleScope handle_scope(isolate); - const char* script = "function foo(a) { return a+1; }"; - v8::ScriptData* sd = v8::ScriptData::PreCompile(v8::String::NewFromUtf8( - isolate, script, v8::String::kNormalString, i::StrLength(script))); - CHECK_NE(sd->Length(), 0); - CHECK_NE(sd->Data(), NULL); - CHECK(!sd->HasError()); - delete sd; -} - - -TEST(PreCompileWithError) { - v8::V8::Initialize(); - v8::Isolate* isolate = CcTest::isolate(); - HandleScope handle_scope(isolate); - const char* script = "function foo(a) { return 1 * * 2; }"; - v8::ScriptData* sd = v8::ScriptData::PreCompile(v8::String::NewFromUtf8( - isolate, script, v8::String::kNormalString, i::StrLength(script))); - CHECK(sd->HasError()); - delete sd; -} - - -TEST(Regress31661) { - v8::V8::Initialize(); - v8::Isolate* isolate = CcTest::isolate(); - HandleScope handle_scope(isolate); - const char* script = " The Definintive Guide"; - v8::ScriptData* sd = v8::ScriptData::PreCompile(v8::String::NewFromUtf8( - isolate, script, v8::String::kNormalString, i::StrLength(script))); - CHECK(sd->HasError()); - delete sd; -} - - // Tests that ScriptData can be serialized and deserialized. TEST(PreCompileSerialization) { v8::V8::Initialize(); - v8::Isolate* isolate = CcTest::isolate(); + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); HandleScope handle_scope(isolate); - const char* script = "function foo(a) { return a+1; }"; - v8::ScriptData* sd = v8::ScriptData::PreCompile(v8::String::NewFromUtf8( - isolate, script, v8::String::kNormalString, i::StrLength(script))); + i::FLAG_min_preparse_length = 0; + const char* script = "function foo(a) { return a+1; }"; + v8::ScriptCompiler::Source source(v8_str(script)); + v8::ScriptCompiler::Compile(isolate, &source, + v8::ScriptCompiler::kProduceDataToCache); // Serialize. - int serialized_data_length = sd->Length(); - char* serialized_data = i::NewArray(serialized_data_length); - i::OS::MemCopy(serialized_data, sd->Data(), serialized_data_length); + const v8::ScriptCompiler::CachedData* cd = source.GetCachedData(); + char* serialized_data = i::NewArray(cd->length); + i::OS::MemCopy(serialized_data, cd->data, cd->length); // Deserialize. - v8::ScriptData* deserialized_sd = - v8::ScriptData::New(serialized_data, serialized_data_length); + v8::ScriptData* deserialized = + v8::ScriptData::New(serialized_data, cd->length); // Verify that the original is the same as the deserialized. - CHECK_EQ(sd->Length(), deserialized_sd->Length()); - CHECK_EQ(0, memcmp(sd->Data(), deserialized_sd->Data(), sd->Length())); - CHECK_EQ(sd->HasError(), deserialized_sd->HasError()); + CHECK_EQ(cd->length, deserialized->Length()); + CHECK_EQ(0, memcmp(cd->data, deserialized->Data(), cd->length)); - delete sd; - delete deserialized_sd; + delete deserialized; i::DeleteArray(serialized_data); } @@ -14908,17 +14866,25 @@ TEST(PreCompileDeserializationError) { } -// Attempts to deserialize bad data. -TEST(PreCompileInvalidPreparseDataError) { +TEST(CompileWithInvalidCachedData) { v8::V8::Initialize(); v8::Isolate* isolate = CcTest::isolate(); LocalContext context; v8::HandleScope scope(context->GetIsolate()); + i::FLAG_min_preparse_length = 0; const char* script = "function foo(){ return 5;}\n" "function bar(){ return 6 + 7;} foo();"; - v8::ScriptData* sd = v8::ScriptData::PreCompile(v8::String::NewFromUtf8( - isolate, script, v8::String::kNormalString, i::StrLength(script))); + v8::ScriptCompiler::Source source(v8_str(script)); + v8::ScriptCompiler::Compile(isolate, &source, + v8::ScriptCompiler::kProduceDataToCache); + // source owns its cached data. Create a ScriptData based on it. The user + // never needs to create ScriptDatas any more; we only need it here because we + // want to modify the data before passing it back. + const v8::ScriptCompiler::CachedData* cd = source.GetCachedData(); + // ScriptData does not take ownership of the buffers passed to it. + v8::ScriptData* sd = + v8::ScriptData::New(reinterpret_cast(cd->data), cd->length); CHECK(!sd->HasError()); // ScriptDataImpl private implementation details const int kHeaderSize = i::PreparseDataConstants::kHeaderSize; @@ -14932,12 +14898,18 @@ TEST(PreCompileInvalidPreparseDataError) { sd_data[kHeaderSize + 1 * kFunctionEntrySize + kFunctionEntryEndOffset] = 0; v8::TryCatch try_catch; - v8::ScriptCompiler::Source script_source( - String::NewFromUtf8(isolate, script), + // Make the script slightly different so that we don't hit the compilation + // cache. Don't change the lenghts of tokens. + const char* script2 = "function foo(){ return 6;}\n" + "function bar(){ return 6 + 7;} foo();"; + v8::ScriptCompiler::Source source2( + v8_str(script2), + // CachedData doesn't take ownership of the buffers, Source takes + // ownership of CachedData. new v8::ScriptCompiler::CachedData( reinterpret_cast(sd->Data()), sd->Length())); Local compiled_script = - v8::ScriptCompiler::CompileUnbound(isolate, &script_source); + v8::ScriptCompiler::CompileUnbound(isolate, &source2); CHECK(try_catch.HasCaught()); String::Utf8Value exception_value(try_catch.Message()->Get()); @@ -14950,19 +14922,20 @@ TEST(PreCompileInvalidPreparseDataError) { // 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. - sd = v8::ScriptData::PreCompile(v8::String::NewFromUtf8( - isolate, script, v8::String::kNormalString, i::StrLength(script))); + // ScriptData does not take ownership of the buffers passed to it. + sd = v8::ScriptData::New(reinterpret_cast(cd->data), cd->length); sd_data = reinterpret_cast(const_cast(sd->Data())); sd_data[kHeaderSize + 1 * kFunctionEntrySize + kFunctionEntryStartOffset] = 200; - v8::ScriptCompiler::Source script_source2( - String::NewFromUtf8(isolate, script), + const char* script3 = "function foo(){ return 7;}\n" + "function bar(){ return 6 + 7;} foo();"; + v8::ScriptCompiler::Source source3( + v8_str(script3), new v8::ScriptCompiler::CachedData( reinterpret_cast(sd->Data()), sd->Length())); compiled_script = - v8::ScriptCompiler::CompileUnbound(isolate, &script_source2); + v8::ScriptCompiler::CompileUnbound(isolate, &source3); CHECK(!try_catch.HasCaught()); - delete sd; } diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 56d20f0b34..7c07ee22eb 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -144,24 +144,36 @@ TEST(ScanHTMLEndComments) { int marker; CcTest::i_isolate()->stack_guard()->SetStackLimit( reinterpret_cast(&marker) - 128 * 1024); - + uintptr_t stack_limit = CcTest::i_isolate()->stack_guard()->real_climit(); for (int i = 0; tests[i]; i++) { - v8::Handle source = v8::String::NewFromUtf8( - isolate, tests[i], v8::String::kNormalString, i::StrLength(tests[i])); - v8::ScriptData* data = v8::ScriptData::PreCompile(source); - CHECK(data != NULL && !data->HasError()); - delete data; + const i::byte* source = + reinterpret_cast(tests[i]); + i::Utf8ToUtf16CharacterStream stream(source, i::StrLength(tests[i])); + i::CompleteParserRecorder log; + i::Scanner scanner(CcTest::i_isolate()->unicode_cache()); + scanner.Initialize(&stream); + i::PreParser preparser(&scanner, &log, stack_limit); + preparser.set_allow_lazy(true); + i::PreParser::PreParseResult result = preparser.PreParseProgram(); + CHECK_EQ(i::PreParser::kPreParseSuccess, result); + i::ScriptDataImpl data(log.ExtractData()); + CHECK(!data.has_error()); } for (int i = 0; fail_tests[i]; i++) { - v8::Handle source = - v8::String::NewFromUtf8(isolate, - fail_tests[i], - v8::String::kNormalString, - i::StrLength(fail_tests[i])); - v8::ScriptData* data = v8::ScriptData::PreCompile(source); - CHECK(data == NULL || data->HasError()); - delete data; + const i::byte* source = + reinterpret_cast(fail_tests[i]); + i::Utf8ToUtf16CharacterStream stream(source, i::StrLength(fail_tests[i])); + i::CompleteParserRecorder log; + i::Scanner scanner(CcTest::i_isolate()->unicode_cache()); + scanner.Initialize(&stream); + i::PreParser preparser(&scanner, &log, stack_limit); + preparser.set_allow_lazy(true); + i::PreParser::PreParseResult result = preparser.PreParseProgram(); + // Even in the case of a syntax error, kPreParseSuccess is returned. + CHECK_EQ(i::PreParser::kPreParseSuccess, result); + i::ScriptDataImpl data(log.ExtractData()); + CHECK(data.has_error()); } } @@ -180,7 +192,7 @@ class ScriptResource : public v8::String::ExternalAsciiStringResource { }; -TEST(Preparsing) { +TEST(UsingCachedData) { v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope handles(isolate); v8::Local context = v8::Context::New(isolate); @@ -203,60 +215,22 @@ TEST(Preparsing) { "var y = { get getter() { return 42; }, " " set setter(v) { this.value = v; }};"; int source_length = i::StrLength(source); - const char* error_source = "var x = y z;"; - int error_source_length = i::StrLength(error_source); - v8::ScriptData* preparse = v8::ScriptData::PreCompile(v8::String::NewFromUtf8( - isolate, source, v8::String::kNormalString, source_length)); - CHECK(!preparse->HasError()); + // ScriptResource will be deleted when the corresponding String is GCd. + v8::ScriptCompiler::Source script_source(v8::String::NewExternal( + isolate, new ScriptResource(source, source_length))); + i::FLAG_min_preparse_length = 0; + v8::ScriptCompiler::Compile(isolate, &script_source, + v8::ScriptCompiler::kProduceDataToCache); + CHECK(script_source.GetCachedData()); + + // Compile the script again, using the cached data. bool lazy_flag = i::FLAG_lazy; - { - i::FLAG_lazy = true; - ScriptResource* resource = new ScriptResource(source, source_length); - v8::ScriptCompiler::Source script_source( - v8::String::NewExternal(isolate, resource), - new v8::ScriptCompiler::CachedData( - reinterpret_cast(preparse->Data()), - preparse->Length())); - v8::ScriptCompiler::Compile(isolate, - &script_source); - } - - { - i::FLAG_lazy = false; - - ScriptResource* resource = new ScriptResource(source, source_length); - v8::ScriptCompiler::Source script_source( - v8::String::NewExternal(isolate, resource), - new v8::ScriptCompiler::CachedData( - reinterpret_cast(preparse->Data()), - preparse->Length())); - v8::ScriptCompiler::CompileUnbound(isolate, &script_source); - } - delete preparse; + i::FLAG_lazy = true; + v8::ScriptCompiler::Compile(isolate, &script_source); + i::FLAG_lazy = false; + v8::ScriptCompiler::CompileUnbound(isolate, &script_source); i::FLAG_lazy = lazy_flag; - - // Syntax error. - v8::ScriptData* error_preparse = v8::ScriptData::PreCompile( - v8::String::NewFromUtf8(isolate, - error_source, - v8::String::kNormalString, - error_source_length)); - CHECK(error_preparse->HasError()); - i::ScriptDataImpl *pre_impl = - reinterpret_cast(error_preparse); - i::Scanner::Location error_location = - pre_impl->MessageLocation(); - // Error is at "z" in source, location 10..11. - CHECK_EQ(10, error_location.beg_pos); - CHECK_EQ(11, error_location.end_pos); - // Should not crash. - const char* message = pre_impl->BuildMessage(); - i::Vector args = pre_impl->BuildArgs(); - CHECK_GT(strlen(message), 0); - args.Dispose(); - i::DeleteArray(message); - delete error_preparse; } @@ -472,15 +446,6 @@ struct CompleteParserRecorderFriend { log->symbol_id_ = number + 1; } } - static int symbol_position(CompleteParserRecorder* log) { - return log->symbol_store_.size(); - } - static int symbol_ids(CompleteParserRecorder* log) { - return log->symbol_id_; - } - static int function_position(CompleteParserRecorder* log) { - return log->function_store_.size(); - } }; } @@ -536,9 +501,17 @@ TEST(RegressChromium62639) { i::Utf8ToUtf16CharacterStream stream( reinterpret_cast(program), static_cast(strlen(program))); - i::ScriptDataImpl* data = i::PreParserApi::PreParse(isolate, &stream); - CHECK(data->HasError()); - delete data; + i::CompleteParserRecorder log; + i::Scanner scanner(CcTest::i_isolate()->unicode_cache()); + scanner.Initialize(&stream); + i::PreParser preparser(&scanner, &log, + CcTest::i_isolate()->stack_guard()->real_climit()); + preparser.set_allow_lazy(true); + i::PreParser::PreParseResult result = preparser.PreParseProgram(); + // Even in the case of a syntax error, kPreParseSuccess is returned. + CHECK_EQ(i::PreParser::kPreParseSuccess, result); + i::ScriptDataImpl data(log.ExtractData()); + CHECK(data.has_error()); } @@ -563,16 +536,23 @@ TEST(Regress928) { i::Handle source( factory->NewStringFromAscii(i::CStrVector(program))); i::GenericStringUtf16CharacterStream stream(source, 0, source->length()); - i::ScriptDataImpl* data = i::PreParserApi::PreParse(isolate, &stream); - CHECK(!data->HasError()); - - data->Initialize(); + i::CompleteParserRecorder log; + i::Scanner scanner(CcTest::i_isolate()->unicode_cache()); + scanner.Initialize(&stream); + i::PreParser preparser(&scanner, &log, + CcTest::i_isolate()->stack_guard()->real_climit()); + preparser.set_allow_lazy(true); + i::PreParser::PreParseResult result = preparser.PreParseProgram(); + CHECK_EQ(i::PreParser::kPreParseSuccess, result); + i::ScriptDataImpl data(log.ExtractData()); + CHECK(!data.has_error()); + data.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 = data->GetFunctionEntry(first_lbrace); + i::FunctionEntry entry1 = data.GetFunctionEntry(first_lbrace); CHECK(!entry1.is_valid()); int second_function = @@ -580,10 +560,9 @@ TEST(Regress928) { int second_lbrace = second_function + i::StrLength("function () "); CHECK_EQ('{', program[second_lbrace]); - i::FunctionEntry entry2 = data->GetFunctionEntry(second_lbrace); + i::FunctionEntry entry2 = data.GetFunctionEntry(second_lbrace); CHECK(entry2.is_valid()); CHECK_EQ('}', program[entry2.end_pos() - 1]); - delete data; } @@ -1556,10 +1535,9 @@ TEST(ParserSync) { } -TEST(PreparserStrictOctal) { +TEST(StrictOctal) { // Test that syntax error caused by octal literal is reported correctly as // such (issue 2220). - v8::internal::FLAG_min_preparse_length = 1; // Force preparsing. v8::V8::Initialize(); v8::HandleScope scope(CcTest::isolate()); v8::Context::Scope context_scope( @@ -2086,9 +2064,12 @@ TEST(NoErrorsIdentifierNames) { TEST(DontRegressPreParserDataSizes) { - // These tests make sure that PreParser doesn't start producing less data. - + // These tests make sure that Parser doesn't start producing less "preparse + // data" (data which the embedder can cache). v8::V8::Initialize(); + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope handles(isolate); + int marker; CcTest::i_isolate()->stack_guard()->SetStackLimit( reinterpret_cast(&marker) - 128 * 1024); @@ -2118,45 +2099,37 @@ TEST(DontRegressPreParserDataSizes) { 1}, {NULL, 0, 0} }; - // Each function adds 5 elements to the preparse function data. - const int kDataPerFunction = 5; - typedef i::CompleteParserRecorderFriend F; - uintptr_t stack_limit = CcTest::i_isolate()->stack_guard()->real_climit(); for (int i = 0; test_cases[i].program; i++) { const char* program = test_cases[i].program; - i::Utf8ToUtf16CharacterStream stream( - reinterpret_cast(program), - static_cast(strlen(program))); - i::CompleteParserRecorder log; - i::Scanner scanner(CcTest::i_isolate()->unicode_cache()); - scanner.Initialize(&stream); + i::Factory* factory = CcTest::i_isolate()->factory(); + i::Handle source( + factory->NewStringFromUtf8(i::CStrVector(program))); + i::Handle script = factory->NewScript(source); + i::CompilationInfoWithZone info(script); + i::ScriptDataImpl* data = NULL; + info.SetCachedData(&data, i::PRODUCE_CACHED_DATA); + i::Parser::Parse(&info, true); + CHECK(data); + CHECK(!data->HasError()); - i::PreParser preparser(&scanner, &log, stack_limit); - preparser.set_allow_lazy(true); - preparser.set_allow_natives_syntax(true); - i::PreParser::PreParseResult result = preparser.PreParseProgram(); - CHECK_EQ(i::PreParser::kPreParseSuccess, result); - if (F::symbol_ids(&log) != test_cases[i].symbols) { + if (data->symbol_count() != test_cases[i].symbols) { i::OS::Print( "Expected preparse data for program:\n" "\t%s\n" "to contain %d symbols, however, received %d symbols.\n", - program, test_cases[i].symbols, F::symbol_ids(&log)); + program, test_cases[i].symbols, data->symbol_count()); CHECK(false); } - if (F::function_position(&log) != - test_cases[i].functions * kDataPerFunction) { + if (data->function_count() != test_cases[i].functions) { i::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, - F::function_position(&log) / kDataPerFunction); + data->function_count()); CHECK(false); } - i::ScriptDataImpl data(log.ExtractData()); - CHECK(!data.has_error()); } } diff --git a/tools/parser-shell.cc b/tools/parser-shell.cc index 4da15fc7e3..9b0de7fb9b 100644 --- a/tools/parser-shell.cc +++ b/tools/parser-shell.cc @@ -44,15 +44,9 @@ using namespace v8::internal; -enum TestMode { - PreParseAndParse, - PreParse, - Parse -}; - std::pair RunBaselineParser( const char* fname, Encoding encoding, int repeat, v8::Isolate* isolate, - v8::Handle context, TestMode test_mode) { + v8::Handle context) { int length = 0; const byte* source = ReadFileAndRepeat(fname, &length, repeat); v8::Handle source_handle; @@ -73,42 +67,41 @@ std::pair RunBaselineParser( break; } } - v8::ScriptData* cached_data = NULL; - TimeDelta preparse_time, parse_time; - if (test_mode == PreParseAndParse || test_mode == PreParse) { - ElapsedTimer timer; - timer.Start(); - cached_data = v8::ScriptData::PreCompile(source_handle); - preparse_time = timer.Elapsed(); - if (cached_data == NULL || cached_data->HasError()) { - fprintf(stderr, "Preparsing failed\n"); - return std::make_pair(TimeDelta(), TimeDelta()); - } - } - if (test_mode == PreParseAndParse || test_mode == Parse) { - Handle str = v8::Utils::OpenHandle(*source_handle); - i::Isolate* internal_isolate = str->GetIsolate(); - Handle