diff --git a/src/asmjs/asm-parser.cc b/src/asmjs/asm-parser.cc index f0ef125a49..c172765612 100644 --- a/src/asmjs/asm-parser.cc +++ b/src/asmjs/asm-parser.cc @@ -235,21 +235,16 @@ uint32_t AsmJsParser::VarIndex(VarInfo* info) { return info->index + static_cast(global_imports_.size()); } -void AsmJsParser::AddGlobalImport(std::string name, AsmType* type, +void AsmJsParser::AddGlobalImport(Vector name, AsmType* type, ValueType vtype, bool mutable_variable, VarInfo* info) { - // TODO(bradnelson): Refactor memory management here. - // AsmModuleBuilder should really own import names. - char* name_data = zone()->NewArray(name.size()); - memcpy(name_data, name.data(), name.size()); - // Allocate a separate variable for the import. // TODO(mstarzinger): Consider using the imported global directly instead of // allocating a separate global variable for immutable (i.e. const) imports. DeclareGlobal(info, mutable_variable, type, vtype); // Record the need to initialize the global from the import. - global_imports_.push_back({name_data, name.size(), vtype, info}); + global_imports_.push_back({name, vtype, info}); } void AsmJsParser::DeclareGlobal(VarInfo* info, bool mutable_variable, @@ -276,6 +271,13 @@ uint32_t AsmJsParser::TempVariable(int index) { return function_temp_locals_offset_ + index; } +Vector AsmJsParser::CopyCurrentIdentifierString() { + const std::string& str = scanner_.GetIdentifierString(); + char* buffer = zone()->NewArray(str.size()); + str.copy(buffer, str.size()); + return Vector(buffer, static_cast(str.size())); +} + void AsmJsParser::SkipSemicolon() { if (Check(';')) { // Had a semicolon. @@ -363,13 +365,9 @@ void AsmJsParser::ValidateModule() { // Add start function to init things. WasmFunctionBuilder* start = module_builder_->AddFunction(); module_builder_->MarkStartFunction(start); - for (auto global_import : global_imports_) { - // TODO(bradnelson): Reuse parse buffer memory / make wasm-module-builder - // managed the memory for the import name (currently have to keep our - // own memory for it). + for (auto& global_import : global_imports_) { uint32_t import_index = module_builder_->AddGlobalImport( - global_import.import_name, - static_cast(global_import.import_name_size), + global_import.import_name.start(), global_import.import_name.length(), global_import.value_type); start->EmitWithVarInt(kExprGetGlobal, import_index); start->EmitWithVarInt(kExprSetGlobal, VarIndex(global_import.var_info)); @@ -533,31 +531,24 @@ bool AsmJsParser::ValidateModuleVarImport(VarInfo* info, if (Check('+')) { EXPECT_TOKENf(foreign_name_); EXPECT_TOKENf('.'); - AddGlobalImport(scanner_.GetIdentifierString(), AsmType::Double(), kWasmF64, - mutable_variable, info); + Vector name = CopyCurrentIdentifierString(); + AddGlobalImport(name, AsmType::Double(), kWasmF64, mutable_variable, info); scanner_.Next(); return true; } else if (Check(foreign_name_)) { EXPECT_TOKENf('.'); - std::string import_name = scanner_.GetIdentifierString(); + Vector name = CopyCurrentIdentifierString(); scanner_.Next(); if (Check('|')) { if (!CheckForZero()) { FAILf("Expected |0 type annotation for foreign integer import"); } - AddGlobalImport(import_name, AsmType::Int(), kWasmI32, mutable_variable, - info); + AddGlobalImport(name, AsmType::Int(), kWasmI32, mutable_variable, info); return true; } info->kind = VarKind::kImportedFunction; - info->import = - new (zone()->New(sizeof(FunctionImportInfo))) FunctionImportInfo( - {nullptr, 0, WasmModuleBuilder::SignatureMap(zone())}); - // TODO(bradnelson): Refactor memory management here. - // AsmModuleBuilder should really own import names. - info->import->function_name = zone()->NewArray(import_name.size()); - memcpy(info->import->function_name, import_name.data(), import_name.size()); - info->import->function_name_size = import_name.size(); + info->import = new (zone()->New(sizeof(FunctionImportInfo))) + FunctionImportInfo({name, WasmModuleBuilder::SignatureMap(zone())}); return true; } return false; @@ -629,7 +620,7 @@ void AsmJsParser::ValidateExport() { // clang format on if (Check('{')) { for (;;) { - std::string name = scanner_.GetIdentifierString(); + Vector name = CopyCurrentIdentifierString(); if (!scanner_.IsGlobal() && !scanner_.IsLocal()) { FAIL("Illegal export name"); } @@ -642,8 +633,7 @@ void AsmJsParser::ValidateExport() { if (info->kind != VarKind::kFunction) { FAIL("Expected function"); } - info->function_builder->ExportAs( - {name.c_str(), static_cast(name.size())}); + info->function_builder->ExportAs(name); if (Check(',')) { if (!Peek('}')) { continue; @@ -727,7 +717,7 @@ void AsmJsParser::ValidateFunction() { FAIL("Expected function name"); } - std::string function_name_raw = scanner_.GetIdentifierString(); + Vector function_name_str = CopyCurrentIdentifierString(); AsmJsScanner::token_t function_name = Consume(); VarInfo* function_info = GetVarInfo(function_name); if (function_info->kind == VarKind::kUnused) { @@ -741,12 +731,7 @@ void AsmJsParser::ValidateFunction() { } function_info->function_defined = true; - // TODO(bradnelson): Cleanup memory management here. - // WasmModuleBuilder should own these. - char* function_name_chr = zone()->NewArray(function_name_raw.size()); - memcpy(function_name_chr, function_name_raw.data(), function_name_raw.size()); - function_info->function_builder->SetName( - {function_name_chr, static_cast(function_name_raw.size())}); + function_info->function_builder->SetName(function_name_str); current_function_builder_ = function_info->function_builder; return_type_ = nullptr; @@ -2099,7 +2084,6 @@ AsmType* AsmJsParser::ValidateCall() { } else if (t->IsA(AsmType::Double())) { param_types.push_back(AsmType::Double()); } else { - std::string a = t->Name(); FAILn("Bad function argument type"); } if (!Peek(')')) { @@ -2170,9 +2154,8 @@ AsmType* AsmJsParser::ValidateCall() { index = it->second; } else { index = module_builder_->AddImport( - function_info->import->function_name, - static_cast(function_info->import->function_name_size), - sig); + function_info->import->function_name.start(), + function_info->import->function_name.length(), sig); function_info->import->cache[sig] = index; } current_function_builder_->AddAsmWasmOffset(call_pos, to_number_pos); diff --git a/src/asmjs/asm-parser.h b/src/asmjs/asm-parser.h index 05ca7a7336..a657d2549c 100644 --- a/src/asmjs/asm-parser.h +++ b/src/asmjs/asm-parser.h @@ -57,8 +57,7 @@ class AsmJsParser { // clang-format on struct FunctionImportInfo { - char* function_name; - size_t function_name_size; + Vector function_name; WasmModuleBuilder::SignatureMap cache; }; @@ -76,8 +75,7 @@ class AsmJsParser { }; struct GlobalImport { - char* import_name; - size_t import_name_size; + Vector import_name; ValueType value_type; VarInfo* var_info; }; @@ -156,8 +154,8 @@ class AsmJsParser { // statements it's attached to. AsmJsScanner::token_t pending_label_; - // Global imports. - // NOTE: Holds the strings referenced in wasm-module-builder for imports. + // Global imports. The list of imported variables that are copied during + // module instantiation into a corresponding global variable. ZoneLinkedList global_imports_; Zone* zone() { return zone_; } @@ -228,13 +226,15 @@ class AsmJsParser { ValueType vtype, const WasmInitExpr& init = WasmInitExpr()); void DeclareStdlibFunc(VarInfo* info, VarKind kind, AsmType* type); + void AddGlobalImport(Vector name, AsmType* type, ValueType vtype, + bool mutable_variable, VarInfo* info); // Allocates a temporary local variable. The given {index} is absolute within // the function body, consider using {TemporaryVariableScope} when nesting. uint32_t TempVariable(int index); - void AddGlobalImport(std::string name, AsmType* type, ValueType vtype, - bool mutable_variable, VarInfo* info); + // Preserves a copy of the scanner's current identifier string in the zone. + Vector CopyCurrentIdentifierString(); // Use to set up block stack layers (including synthetic ones for if-else). // Begin/Loop/End below are implemented with these plus code generation. diff --git a/src/asmjs/asm-scanner.h b/src/asmjs/asm-scanner.h index 6f9dfbe821..3cc5170055 100644 --- a/src/asmjs/asm-scanner.h +++ b/src/asmjs/asm-scanner.h @@ -43,12 +43,15 @@ class V8_EXPORT_PRIVATE AsmJsScanner { void Next(); // Back up by one token. void Rewind(); - // Get raw string for current identifier. + + // Get raw string for current identifier. Note that the returned string will + // become invalid when the scanner advances, create a copy to preserve it. const std::string& GetIdentifierString() const { // Identifier strings don't work after a rewind. DCHECK(!rewind_); return identifier_string_; } + // Check if we just passed a newline. bool IsPrecededByNewline() const { // Newline tracking doesn't work if you back up. diff --git a/src/wasm/wasm-module-builder.h b/src/wasm/wasm-module-builder.h index 567d13b567..d4e995dc05 100644 --- a/src/wasm/wasm-module-builder.h +++ b/src/wasm/wasm-module-builder.h @@ -225,6 +225,7 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject { explicit WasmModuleBuilder(Zone* zone); // Building methods. + // TODO(mstarzinger): Refactor to take Vector instead. uint32_t AddImport(const char* name, int name_length, FunctionSig* sig); void SetImportName(uint32_t index, const char* name, int name_length) { function_imports_[index].name = name; @@ -233,6 +234,7 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject { WasmFunctionBuilder* AddFunction(FunctionSig* sig = nullptr); uint32_t AddGlobal(ValueType type, bool exported, bool mutability = true, const WasmInitExpr& init = WasmInitExpr()); + // TODO(mstarzinger): Refactor to take Vector instead. uint32_t AddGlobalImport(const char* name, int name_length, ValueType type); void AddDataSegment(const byte* data, uint32_t size, uint32_t dest); uint32_t AddSignature(FunctionSig* sig);