Commit b7a124e3 authored by Ross McIlroy's avatar Ross McIlroy Committed by Commit Bot

[asm-js] Use existing character-stream to re-parse asm.js modules.

Instead of creating a new character stream to re-parse the asm.js module,
use the existing stream which was used by the parser.  By doing this, we
avoid accessing the heap if the original character stream is a streaming
source or an external string, which will enable asm.js verification to run
off-thread in those situations.

BUG=v8:5203

Change-Id: I5dbf83c993512eb2f3dd709120e152e3f9900bdf
Reviewed-on: https://chromium-review.googlesource.com/616723Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarMarja Hölttä <marja@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47500}
parent e2eb208a
......@@ -8,6 +8,7 @@
#include "src/asmjs/asm-parser.h"
#include "src/assert-scope.h"
#include "src/ast/ast.h"
#include "src/base/optional.h"
#include "src/base/platform/elapsed-timer.h"
#include "src/compilation-info.h"
#include "src/compiler.h"
......@@ -16,6 +17,7 @@
#include "src/handles.h"
#include "src/isolate.h"
#include "src/objects-inl.h"
#include "src/parsing/parse-info.h"
#include "src/parsing/scanner-character-streams.h"
#include "src/parsing/scanner.h"
......@@ -175,7 +177,7 @@ void ReportInstantiationFailure(Handle<Script> script, int position,
} // namespace
// The compilation of asm.js modules is split into two distinct steps:
// [1] PrepareJobImpl: The asm.js module source is parsed, validated, and
// [1] ExecuteJobImpl: The asm.js module source is parsed, validated, and
// translated to a valid WebAssembly module. The result are two vectors
// representing the encoded module as well as encoded source position
// information and a StdlibSet bit set.
......@@ -213,14 +215,10 @@ class AsmJsCompilationJob final : public CompilationJob {
};
CompilationJob::Status AsmJsCompilationJob::PrepareJobImpl() {
// TODO(rmcilroy): Temporarily allow heap access here until we use a
// off-heap ScannerStream.
DCHECK(
ThreadId::Current().Equals(compilation_info()->isolate()->thread_id()));
AllowHeapAllocation allow_allocation;
AllowHandleAllocation allow_handles;
AllowHandleDereference allow_deref;
return SUCCEEDED;
}
CompilationJob::Status AsmJsCompilationJob::ExecuteJobImpl() {
// Step 1: Translate asm.js module to WebAssembly module.
HistogramTimerScope translate_time_scope(
compilation_info()->isolate()->counters()->asm_wasm_translation_time());
......@@ -231,14 +229,24 @@ CompilationJob::Status AsmJsCompilationJob::PrepareJobImpl() {
Zone* compile_zone = compilation_info()->zone();
Zone translate_zone(compilation_info()->isolate()->allocator(), ZONE_NAME);
// TODO(mstarzinger): In order to move translation to the non-main thread
// ExecuteJob phase, the scanner stream needs to be off-heap.
std::unique_ptr<Utf16CharacterStream> stream(ScannerStream::For(
handle(String::cast(compilation_info()->script()->source())),
compilation_info()->literal()->start_position(),
compilation_info()->literal()->end_position()));
wasm::AsmJsParser parser(&translate_zone, stack_limit(), std::move(stream));
Utf16CharacterStream* stream = parse_info()->character_stream();
base::Optional<AllowHandleDereference> allow_deref;
if (stream->can_access_heap()) {
DCHECK(
ThreadId::Current().Equals(compilation_info()->isolate()->thread_id()));
allow_deref.emplace();
}
stream->Seek(compilation_info()->literal()->start_position());
wasm::AsmJsParser parser(&translate_zone, stack_limit(), stream);
if (!parser.Run()) {
// TODO(rmcilroy): Temporarily allow heap access here until we have a
// mechanism for delaying pending messages.
DCHECK(
ThreadId::Current().Equals(compilation_info()->isolate()->thread_id()));
AllowHeapAllocation allow_allocation;
AllowHandleAllocation allow_handles;
allow_deref.emplace();
DCHECK(!compilation_info()->isolate()->has_pending_exception());
ReportCompilationFailure(compilation_info()->script(),
parser.failure_location(),
......@@ -269,12 +277,6 @@ CompilationJob::Status AsmJsCompilationJob::PrepareJobImpl() {
return SUCCEEDED;
}
CompilationJob::Status AsmJsCompilationJob::ExecuteJobImpl() {
// TODO(mstarzinger): Move translation to the Execute phase (see comment about
// scanner stream above.
return SUCCEEDED;
}
CompilationJob::Status AsmJsCompilationJob::FinalizeJobImpl() {
// Step 2: Compile and decode the WebAssembly module.
base::ElapsedTimer compile_timer;
......
......@@ -68,8 +68,9 @@ namespace wasm {
#define TOK(name) AsmJsScanner::kToken_##name
AsmJsParser::AsmJsParser(Zone* zone, uintptr_t stack_limit,
std::unique_ptr<Utf16CharacterStream> stream)
Utf16CharacterStream* stream)
: zone_(zone),
scanner_(stream),
module_builder_(new (zone) WasmModuleBuilder(zone)),
return_type_(nullptr),
stack_limit_(stack_limit),
......@@ -88,7 +89,6 @@ AsmJsParser::AsmJsParser(Zone* zone, uintptr_t stack_limit,
pending_label_(0),
global_imports_(zone) {
InitializeStdlibTypes();
scanner_.SetStream(std::move(stream));
}
void AsmJsParser::InitializeStdlibTypes() {
......
......@@ -50,7 +50,7 @@ class AsmJsParser {
typedef EnumSet<StandardMember, uint64_t> StdlibSet;
explicit AsmJsParser(Zone* zone, uintptr_t stack_limit,
std::unique_ptr<Utf16CharacterStream> stream);
Utf16CharacterStream* stream);
bool Run();
const char* failure_message() const { return failure_message_; }
int failure_location() const { return failure_location_; }
......
......@@ -18,8 +18,9 @@ namespace {
static const int kMaxIdentifierCount = 0xf000000;
};
AsmJsScanner::AsmJsScanner()
: token_(kUninitialized),
AsmJsScanner::AsmJsScanner(Utf16CharacterStream* stream)
: stream_(stream),
token_(kUninitialized),
preceding_token_(kUninitialized),
next_token_(kUninitialized),
position_(0),
......@@ -44,14 +45,6 @@ AsmJsScanner::AsmJsScanner()
#define V(name) global_names_[#name] = kToken_##name;
KEYWORD_NAME_LIST(V)
#undef V
}
// Destructor of unique_ptr<T> requires complete declaration of T, we only want
// to include the necessary declaration here instead of the header file.
AsmJsScanner::~AsmJsScanner() {}
void AsmJsScanner::SetStream(std::unique_ptr<Utf16CharacterStream> stream) {
stream_ = std::move(stream);
Next();
}
......
......@@ -31,11 +31,7 @@ class V8_EXPORT_PRIVATE AsmJsScanner {
public:
typedef int32_t token_t;
AsmJsScanner();
~AsmJsScanner();
// Pick the stream to parse (must be called before anything else).
void SetStream(std::unique_ptr<Utf16CharacterStream> stream);
explicit AsmJsScanner(Utf16CharacterStream* stream);
// Get current token.
token_t Token() const { return token_; }
......@@ -140,7 +136,7 @@ class V8_EXPORT_PRIVATE AsmJsScanner {
// clang-format on
private:
std::unique_ptr<Utf16CharacterStream> stream_;
Utf16CharacterStream* stream_;
token_t token_;
token_t preceding_token_;
token_t next_token_; // Only set when in {rewind} state.
......
......@@ -376,6 +376,21 @@ bool Scope::IsAsmModule() const {
return is_function_scope() && AsDeclarationScope()->asm_module();
}
bool Scope::ContainsAsmModule() const {
if (IsAsmModule()) return true;
// Check inner scopes recursively
for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
// Don't check inner functions which won't be eagerly compiled.
if (!scope->is_function_scope() ||
scope->AsDeclarationScope()->ShouldEagerCompile()) {
if (scope->ContainsAsmModule()) return true;
}
}
return false;
}
Scope* Scope::DeserializeScopeChain(Zone* zone, ScopeInfo* scope_info,
DeclarationScope* script_scope,
AstValueFactory* ast_value_factory,
......
......@@ -364,6 +364,9 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
bool inner_scope_calls_eval() const { return inner_scope_calls_eval_; }
bool IsAsmModule() const;
// Returns true if this scope or any inner scopes that might be eagerly
// compiled are asm modules.
bool ContainsAsmModule() const;
// Does this scope have the potential to execute declarations non-linearly?
bool is_nonlinear() const { return scope_nonlinear_; }
......
......@@ -267,7 +267,7 @@ bool UseAsmWasm(FunctionLiteral* literal, bool asm_wasm_broken) {
if (FLAG_stress_validate_asm) return true;
// In general, we respect the "use asm" directive.
return literal->scope()->asm_module();
return literal->scope()->IsAsmModule();
}
void InstallUnoptimizedCode(CompilationInfo* compilation_info) {
......@@ -431,6 +431,9 @@ std::unique_ptr<CompilationJob> GenerateUnoptimizedCode(
inner_function_jobs->emplace_front(std::move(inner_job));
}
// Character stream shouldn't be used again.
parse_info->ResetCharacterStream();
return outer_function_job;
}
......
......@@ -548,6 +548,19 @@ void Parser::DeserializeScopeChain(
original_scope_ = scope;
}
namespace {
void MaybeResetCharacterStream(ParseInfo* info, FunctionLiteral* literal) {
// Don't reset the character stream if there is an asm.js module since it will
// be used again by the asm-parser.
if (!FLAG_stress_validate_asm &&
(literal == nullptr || !literal->scope()->ContainsAsmModule())) {
info->ResetCharacterStream();
}
}
} // namespace
FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
// TODO(bmeurer): We temporarily need to pass allow_nesting = true here,
// see comment for HistogramTimerScope class.
......@@ -582,7 +595,7 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
scanner_.Initialize(info->character_stream(), info->is_module());
FunctionLiteral* result = DoParseProgram(info);
info->ResetCharacterStream(); // Character stream no longer used.
MaybeResetCharacterStream(info, result);
HandleSourceURLComments(isolate, info->script());
......@@ -742,7 +755,7 @@ FunctionLiteral* Parser::ParseFunction(Isolate* isolate, ParseInfo* info,
scanner_.Initialize(info->character_stream(), info->is_module());
FunctionLiteral* result = DoParseFunction(info, info->function_name());
info->ResetCharacterStream(); // Character stream no longer used.
MaybeResetCharacterStream(info, result);
if (result != nullptr) {
Handle<String> inferred_name(shared_info->inferred_name());
result->set_inferred_name(inferred_name);
......@@ -3389,7 +3402,7 @@ void Parser::ParseOnBackground(ParseInfo* info) {
} else {
result = DoParseFunction(info, info->function_name());
}
info->ResetCharacterStream(); // Character stream no longer used.
MaybeResetCharacterStream(info, result);
info->set_literal(result);
......
......@@ -72,6 +72,8 @@ class GenericStringUtf16CharacterStream : public BufferedUtf16CharacterStream {
GenericStringUtf16CharacterStream(Handle<String> data, size_t start_position,
size_t end_position);
bool can_access_heap() override { return true; }
protected:
size_t FillBuffer(size_t position) override;
......@@ -109,6 +111,8 @@ class ExternalTwoByteStringUtf16CharacterStream : public Utf16CharacterStream {
size_t start_position,
size_t end_position);
bool can_access_heap() override { return false; }
private:
bool ReadBlock() override;
......@@ -160,6 +164,8 @@ class ExternalOneByteStringUtf16CharacterStream
// For testing:
ExternalOneByteStringUtf16CharacterStream(const char* data, size_t length);
bool can_access_heap() override { return false; }
protected:
size_t FillBuffer(size_t position) override;
......@@ -603,6 +609,8 @@ class ExternalStreamingStream : public BufferedUtf16CharacterStream {
RuntimeCallStats* stats)
: chunks_(new Chunks(std::move(source), stats)) {}
bool can_access_heap() override { return false; }
protected:
bool ReadBlock() override;
size_t FillBuffer(size_t position) override;
......
......@@ -92,6 +92,9 @@ class Utf16CharacterStream {
}
}
// Returns true if the stream could access the V8 heap after construction.
virtual bool can_access_heap() = 0;
protected:
Utf16CharacterStream(const uint16_t* buffer_start,
const uint16_t* buffer_cursor,
......
......@@ -15,38 +15,40 @@ namespace internal {
class AsmJsScannerTest : public ::testing::Test {
protected:
void SetupSource(const char* source) {
scanner.SetStream(ScannerStream::ForTesting(source));
void SetupScanner(const char* source) {
stream = ScannerStream::ForTesting(source);
scanner.reset(new AsmJsScanner(stream.get()));
}
void Skip(AsmJsScanner::token_t t) {
CHECK_EQ(t, scanner.Token());
scanner.Next();
CHECK_EQ(t, scanner->Token());
scanner->Next();
}
void SkipGlobal() {
CHECK(scanner.IsGlobal());
scanner.Next();
CHECK(scanner->IsGlobal());
scanner->Next();
}
void SkipLocal() {
CHECK(scanner.IsLocal());
scanner.Next();
CHECK(scanner->IsLocal());
scanner->Next();
}
void CheckForEnd() { CHECK(scanner.Token() == AsmJsScanner::kEndOfInput); }
void CheckForEnd() { CHECK(scanner->Token() == AsmJsScanner::kEndOfInput); }
void CheckForParseError() {
CHECK(scanner.Token() == AsmJsScanner::kParseError);
CHECK(scanner->Token() == AsmJsScanner::kParseError);
}
AsmJsScanner scanner;
std::unique_ptr<Utf16CharacterStream> stream;
std::unique_ptr<AsmJsScanner> scanner;
};
TEST_F(AsmJsScannerTest, SimpleFunction) {
SetupSource("function foo() { return; }");
SetupScanner("function foo() { return; }");
Skip(TOK(function));
DCHECK_EQ("foo", scanner.GetIdentifierString());
DCHECK_EQ("foo", scanner->GetIdentifierString());
SkipGlobal();
Skip('(');
Skip(')');
......@@ -60,7 +62,7 @@ TEST_F(AsmJsScannerTest, SimpleFunction) {
}
TEST_F(AsmJsScannerTest, JSKeywords) {
SetupSource(
SetupScanner(
"arguments break case const continue\n"
"default do else eval for function\n"
"if new return switch var while\n");
......@@ -87,7 +89,7 @@ TEST_F(AsmJsScannerTest, JSKeywords) {
}
TEST_F(AsmJsScannerTest, JSOperatorsSpread) {
SetupSource(
SetupScanner(
"+ - * / % & | ^ ~ << >> >>>\n"
"< > <= >= == !=\n");
Skip('+');
......@@ -112,7 +114,7 @@ TEST_F(AsmJsScannerTest, JSOperatorsSpread) {
}
TEST_F(AsmJsScannerTest, JSOperatorsTight) {
SetupSource(
SetupScanner(
"+-*/%&|^~<<>> >>>\n"
"<><=>= ==!=\n");
Skip('+');
......@@ -137,17 +139,17 @@ TEST_F(AsmJsScannerTest, JSOperatorsTight) {
}
TEST_F(AsmJsScannerTest, UsesOfAsm) {
SetupSource("'use asm' \"use asm\"\n");
SetupScanner("'use asm' \"use asm\"\n");
Skip(TOK(UseAsm));
Skip(TOK(UseAsm));
CheckForEnd();
}
TEST_F(AsmJsScannerTest, DefaultGlobalScope) {
SetupSource("var x = x + x;");
SetupScanner("var x = x + x;");
Skip(TOK(var));
CHECK_EQ("x", scanner.GetIdentifierString());
AsmJsScanner::token_t x = scanner.Token();
CHECK_EQ("x", scanner->GetIdentifierString());
AsmJsScanner::token_t x = scanner->Token();
SkipGlobal();
Skip('=');
Skip(x);
......@@ -158,11 +160,11 @@ TEST_F(AsmJsScannerTest, DefaultGlobalScope) {
}
TEST_F(AsmJsScannerTest, GlobalScope) {
SetupSource("var x = x + x;");
scanner.EnterGlobalScope();
SetupScanner("var x = x + x;");
scanner->EnterGlobalScope();
Skip(TOK(var));
CHECK_EQ("x", scanner.GetIdentifierString());
AsmJsScanner::token_t x = scanner.Token();
CHECK_EQ("x", scanner->GetIdentifierString());
AsmJsScanner::token_t x = scanner->Token();
SkipGlobal();
Skip('=');
Skip(x);
......@@ -173,11 +175,11 @@ TEST_F(AsmJsScannerTest, GlobalScope) {
}
TEST_F(AsmJsScannerTest, LocalScope) {
SetupSource("var x = x + x;");
scanner.EnterLocalScope();
SetupScanner("var x = x + x;");
scanner->EnterLocalScope();
Skip(TOK(var));
CHECK_EQ("x", scanner.GetIdentifierString());
AsmJsScanner::token_t x = scanner.Token();
CHECK_EQ("x", scanner->GetIdentifierString());
AsmJsScanner::token_t x = scanner->Token();
SkipLocal();
Skip('=');
Skip(x);
......@@ -188,41 +190,41 @@ TEST_F(AsmJsScannerTest, LocalScope) {
}
TEST_F(AsmJsScannerTest, Numbers) {
SetupSource("1 1.2 0x1f 1.e3");
SetupScanner("1 1.2 0x1f 1.e3");
CHECK(scanner.IsUnsigned());
CHECK_EQ(1, scanner.AsUnsigned());
scanner.Next();
CHECK(scanner->IsUnsigned());
CHECK_EQ(1, scanner->AsUnsigned());
scanner->Next();
CHECK(scanner.IsDouble());
CHECK_EQ(1.2, scanner.AsDouble());
scanner.Next();
CHECK(scanner->IsDouble());
CHECK_EQ(1.2, scanner->AsDouble());
scanner->Next();
CHECK(scanner.IsUnsigned());
CHECK_EQ(31, scanner.AsUnsigned());
scanner.Next();
CHECK(scanner->IsUnsigned());
CHECK_EQ(31, scanner->AsUnsigned());
scanner->Next();
CHECK(scanner.IsDouble());
CHECK_EQ(1.0e3, scanner.AsDouble());
scanner.Next();
CHECK(scanner->IsDouble());
CHECK_EQ(1.0e3, scanner->AsDouble());
scanner->Next();
CheckForEnd();
}
TEST_F(AsmJsScannerTest, UnsignedNumbers) {
SetupSource("0x7fffffff 0x80000000 0xffffffff 0x100000000");
SetupScanner("0x7fffffff 0x80000000 0xffffffff 0x100000000");
CHECK(scanner.IsUnsigned());
CHECK_EQ(0x7fffffff, scanner.AsUnsigned());
scanner.Next();
CHECK(scanner->IsUnsigned());
CHECK_EQ(0x7fffffff, scanner->AsUnsigned());
scanner->Next();
CHECK(scanner.IsUnsigned());
CHECK_EQ(0x80000000, scanner.AsUnsigned());
scanner.Next();
CHECK(scanner->IsUnsigned());
CHECK_EQ(0x80000000, scanner->AsUnsigned());
scanner->Next();
CHECK(scanner.IsUnsigned());
CHECK_EQ(0xffffffff, scanner.AsUnsigned());
scanner.Next();
CHECK(scanner->IsUnsigned());
CHECK_EQ(0xffffffff, scanner->AsUnsigned());
scanner->Next();
// Numeric "unsigned" literals with a payload of more than 32-bit are rejected
// by asm.js in all contexts, we hence consider `0x100000000` to be an error.
......@@ -230,30 +232,30 @@ TEST_F(AsmJsScannerTest, UnsignedNumbers) {
}
TEST_F(AsmJsScannerTest, BadNumber) {
SetupSource(".123fe");
SetupScanner(".123fe");
Skip('.');
CheckForParseError();
}
TEST_F(AsmJsScannerTest, Rewind1) {
SetupSource("+ - * /");
SetupScanner("+ - * /");
Skip('+');
scanner.Rewind();
scanner->Rewind();
Skip('+');
Skip('-');
scanner.Rewind();
scanner->Rewind();
Skip('-');
Skip('*');
scanner.Rewind();
scanner->Rewind();
Skip('*');
Skip('/');
scanner.Rewind();
scanner->Rewind();
Skip('/');
CheckForEnd();
}
TEST_F(AsmJsScannerTest, Comments) {
SetupSource(
SetupScanner(
"var // This is a test /* */ eval\n"
"var /* test *** test */ eval\n"
"function /* this */ ^");
......@@ -266,22 +268,22 @@ TEST_F(AsmJsScannerTest, Comments) {
}
TEST_F(AsmJsScannerTest, TrailingCComment) {
SetupSource("var /* test\n");
SetupScanner("var /* test\n");
Skip(TOK(var));
CheckForParseError();
}
TEST_F(AsmJsScannerTest, Seeking) {
SetupSource("var eval do arguments function break\n");
SetupScanner("var eval do arguments function break\n");
Skip(TOK(var));
size_t old_pos = scanner.Position();
size_t old_pos = scanner->Position();
Skip(TOK(eval));
Skip(TOK(do));
Skip(TOK(arguments));
scanner.Rewind();
scanner->Rewind();
Skip(TOK(arguments));
scanner.Rewind();
scanner.Seek(old_pos);
scanner->Rewind();
scanner->Seek(old_pos);
Skip(TOK(eval));
Skip(TOK(do));
Skip(TOK(arguments));
......@@ -291,19 +293,19 @@ TEST_F(AsmJsScannerTest, Seeking) {
}
TEST_F(AsmJsScannerTest, Newlines) {
SetupSource(
SetupScanner(
"var x = 1\n"
"var y = 2\n");
Skip(TOK(var));
scanner.Next();
scanner->Next();
Skip('=');
scanner.Next();
CHECK(scanner.IsPrecededByNewline());
scanner->Next();
CHECK(scanner->IsPrecededByNewline());
Skip(TOK(var));
scanner.Next();
scanner->Next();
Skip('=');
scanner.Next();
CHECK(scanner.IsPrecededByNewline());
scanner->Next();
CHECK(scanner->IsPrecededByNewline());
CheckForEnd();
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment