diff --git a/check.py b/check.py index 128cdae8b0f..5057933fbd6 100755 --- a/check.py +++ b/check.py @@ -221,8 +221,6 @@ def check_expected(actual, expected): check_expected(actual, expected) - run_spec_test(wast) - # check binary format. here we can verify execution of the final # result, no need for an output verification actual = '' diff --git a/scripts/test/shared.py b/scripts/test/shared.py index b1764d30f36..c96257e743f 100644 --- a/scripts/test/shared.py +++ b/scripts/test/shared.py @@ -443,13 +443,14 @@ def get_tests(test_dir, extensions=[], recursive=False): 'imports.wast', # Missing validation of missing function on instantiation 'proposals/threads/imports.wast', # Missing memory type validation on instantiation 'linking.wast', # Missing function type validation on instantiation - 'memory.wast', # Requires wast `module definition` support 'proposals/threads/memory.wast', # Missing memory type validation on instantiation 'memory64-imports.wast', # Missing validation on instantiation 'annotations.wast', # String annotations IDs should be allowed 'id.wast', # Empty IDs should be disallowed - 'instance.wast', # Requires wast `module definition` support - 'table64.wast', # Requires wast `module definition` support + # Requires correct handling of tag imports from different instances of the same module, + # ref.null wast constants, and splitting for module instances + 'instance.wast', + 'table64.wast', # Requires validations for table size 'table_grow.wast', # Incorrect table linking semantics in interpreter 'tag.wast', # Non-empty tag results allowed by stack switching 'try_table.wast', # Requires try_table interpretation @@ -473,7 +474,7 @@ def get_tests(test_dir, extensions=[], recursive=False): 'type-rec.wast', # Missing function type validation on instantiation 'type-subtyping.wast', # ShellExternalInterface::callTable does not handle subtyping 'call_indirect.wast', # Bug with 64-bit inline element segment parsing - 'memory64.wast', # Requires wast `module definition` support + 'memory64.wast', # Requires validations for memory size 'imports0.wast', # Missing memory type validation on instantiation 'imports2.wast', # Missing memory type validation on instantiation 'imports3.wast', # Missing memory type validation on instantiation diff --git a/scripts/test/support.py b/scripts/test/support.py index c109c98b654..c69c53fd42c 100644 --- a/scripts/test/support.py +++ b/scripts/test/support.py @@ -90,8 +90,14 @@ def untar(tarfile, outdir): QUOTED = re.compile(r'\(module\s*(\$\S*)?\s+(quote|binary)') +MODULE_DEFINITION_OR_INSTANCE = re.compile(r'(?m)\(module\s+(instance|definition)') + def split_wast(wastFile): + ''' + Returns a list of pairs of module definitions and assertions. + Module invalidity tests, as well as (module definition ...) and (module instance ...) are skipped. + ''' # if it's a binary, leave it as is, we can't split it wast = None if not wastFile.endswith('.wasm'): @@ -128,7 +134,7 @@ def to_end(j): return j i = 0 - ignoring_quoted = False + ignoring_assertions = False while i >= 0: start = wast.find('(', i) if start >= 0 and wast[start + 1] == ';': @@ -146,17 +152,17 @@ def to_end(j): break i = to_end(start + 1) chunk = wast[start:i] - if QUOTED.match(chunk): + if QUOTED.match(chunk) or MODULE_DEFINITION_OR_INSTANCE.match(chunk): # There may be assertions after this quoted module, but we aren't # returning the module, so we need to skip the assertions as well. - ignoring_quoted = True + ignoring_assertions = True continue if chunk.startswith('(module'): - ignoring_quoted = False + ignoring_assertions = False ret += [(chunk, [])] elif chunk.startswith('(assert_invalid'): continue - elif chunk.startswith(('(assert', '(invoke', '(register')) and not ignoring_quoted: + elif chunk.startswith(('(assert', '(invoke', '(register')) and not ignoring_assertions: # ret may be empty if there are some asserts before the first # module. in that case these are asserts *without* a module, which # are valid (they may check something that doesn't refer to a module @@ -190,14 +196,13 @@ def run_command(cmd, expected_status=0, stderr=None, out, err = proc.communicate() code = proc.returncode if expected_status is not None and code != expected_status: - raise Exception(('run_command failed (%s)' % code, out + str(err or ''))) + raise Exception(f"run_command `{' '.join(cmd)}` failed ({code}) {err or ''}") if expected_err is not None: if err_ignore is not None: err = "\n".join([line for line in err.split('\n') if err_ignore not in line]) err_correct = expected_err in err if err_contains else expected_err == err if not err_correct: - raise Exception(('run_command unexpected stderr', - "expected '%s', actual '%s'" % (expected_err, err))) + raise Exception(f"run_command unexpected stderr. Expected '{expected_err}', actual '{err}'") return out diff --git a/src/parser/wast-parser.cpp b/src/parser/wast-parser.cpp index 6aa9d6a200b..52045bf347e 100644 --- a/src/parser/wast-parser.cpp +++ b/src/parser/wast-parser.cpp @@ -88,16 +88,21 @@ Result action(Lexer& in) { return in.err("expected action"); } -// (module id? binary string*) -// (module id? quote string*) -// (module ...) +// (module id binary string*) +// (module id quote string*) +// (module definition id? ...) +// (module definition id? binary ...) Result wastModule(Lexer& in, bool maybeInvalid = false) { Lexer reset = in; if (!in.takeSExprStart("module"sv)) { return in.err("expected module"); } - // TODO: use ID? - [[maybe_unused]] auto id = in.takeID(); + + bool isDefinition = in.takeKeyword("definition"sv); + std::optional id = in.takeID(); + + Lexer moduleBody = in; + QuotedModuleType type; if (in.takeKeyword("quote"sv)) { type = QuotedModuleType::Text; @@ -118,15 +123,24 @@ Result wastModule(Lexer& in, bool maybeInvalid = false) { } } std::string mod(reset.next().substr(0, in.getPos() - reset.getPos())); - return QuotedModule{QuotedModuleType::Text, mod}; + return WASTModule{isDefinition, QuotedModule{QuotedModuleType::Text, mod}}; } else { - // This is a normal inline module that should be parseable. Reset to the - // start and parse it normally. - in = std::move(reset); + // In this case the module is mostly valid WAT, unless it is a module + // definition in which case it will begin with (module definition ...) + in = std::move(moduleBody); + auto wasm = std::make_shared(); + if (id) { + wasm->name = *id; + } + wasm->features = FeatureSet::All; - CHECK_ERR(parseModule(*wasm, in)); - return wasm; + CHECK_ERR(parseModuleBody(*wasm, in)); + if (!in.takeRParen()) { + return in.err("expected end of module"); + } + + return WASTModule{isDefinition, wasm}; } // We have a quote or binary module. Collect its contents. @@ -139,7 +153,7 @@ Result wastModule(Lexer& in, bool maybeInvalid = false) { return in.err("expected end of module"); } - return QuotedModule{type, ss.str()}; + return WASTModule{isDefinition, QuotedModule{type, ss.str()}}; } Result nan(Lexer& in) { @@ -440,17 +454,42 @@ MaybeResult register_(Lexer& in) { return in.err("expected name"); } - // TODO: Do we need to use this optional id? - in.takeID(); + auto instanceName = in.takeID(); if (!in.takeRParen()) { - // TODO: handle optional module id. return in.err("expected end of register command"); } - return Register{*name}; + + return Register{*name, instanceName}; } -// module | register | action | assertion +// (module instance instance_name? module_name?) +MaybeResult instantiation(Lexer& in) { + Lexer reset = in; + if (!in.takeSExprStart("module"sv)) { + std::optional actual = in.peekKeyword(); + return in.err((std::stringstream() << "expected `module` keyword but got " + << actual.value_or("")) + .str()); + } + + if (!in.takeKeyword("instance"sv)) { + // This is not a module instance and probably a module instead. + in = reset; + return {}; + } + + auto instanceId = in.takeID(); + auto moduleId = in.takeID(); + + if (!in.takeRParen()) { + return in.err("expected end of module instantiation"); + } + + return ModuleInstantiation{moduleId, instanceId}; +} + +// instantiate | module | register | action | assertion Result command(Lexer& in) { if (auto cmd = register_(in)) { CHECK_ERR(cmd); @@ -464,9 +503,14 @@ Result command(Lexer& in) { CHECK_ERR(cmd); return *cmd; } - auto mod = wastModule(in); - CHECK_ERR(mod); - return *mod; + if (auto cmd = instantiation(in)) { + CHECK_ERR(cmd); + return *cmd; + } + + auto module = wastModule(in); + CHECK_ERR(module); + return *module; } #pragma GCC diagnostic push @@ -487,7 +531,8 @@ Result wast(Lexer& in) { // No, that wasn't the problem. Return the original error. return Err{err->msg}; } - cmds.push_back({WASTModule{std::move(wasm)}, line}); + cmds.push_back( + {WASTModule{/*isDefinition=*/false, std::move(wasm)}, line}); return cmds; } CHECK_ERR(cmd); diff --git a/src/parser/wat-parser.h b/src/parser/wat-parser.h index 6bbe8601a4c..e448c7d35da 100644 --- a/src/parser/wat-parser.h +++ b/src/parser/wat-parser.h @@ -97,7 +97,10 @@ struct QuotedModule { std::string module; }; -using WASTModule = std::variant>; +struct WASTModule { + bool isDefinition = false; + std::variant> module; +}; enum class ModuleAssertionType { Trap, Malformed, Invalid, Unlinkable }; @@ -109,10 +112,21 @@ struct AssertModule { using Assertion = std::variant; struct Register { + // TODO: Rename this to distinguish it from instanceName. Name name; + std::optional instanceName = std::nullopt; +}; + +struct ModuleInstantiation { + // If not specified, instantiate the most recent module definition. + std::optional moduleName; + + // If not specified, use the moduleName + std::optional instanceName; }; -using WASTCommand = std::variant; +using WASTCommand = + std::variant; struct ScriptEntry { WASTCommand cmd; diff --git a/src/tools/wasm-shell.cpp b/src/tools/wasm-shell.cpp index 1ee9a5a4c5f..6a94cb8db70 100644 --- a/src/tools/wasm-shell.cpp +++ b/src/tools/wasm-shell.cpp @@ -39,13 +39,17 @@ using namespace wasm; using namespace wasm::WATParser; struct Shell { + // Keyed by module name. std::map> modules; + + // Keyed by instance name. std::map> interfaces; std::map> instances; - // used for imports + // Used for imports, keyed by instance name. std::map> linkedInstances; - Name lastModule; + Name lastInstance; + std::optional lastModuleDefinition; Options& options; @@ -86,6 +90,9 @@ struct Shell { return Ok{}; } else if (auto* assn = std::get_if(&cmd)) { return doAssertion(*assn); + } else if (auto* instantiateModule = + std::get_if(&cmd)) { + return doInstantiate(*instantiateModule); } else { WASM_UNREACHABLE("unexpected command"); } @@ -93,7 +100,7 @@ struct Shell { Result> makeModule(WASTModule& mod) { std::shared_ptr wasm; - if (auto* quoted = std::get_if(&mod)) { + if (auto* quoted = std::get_if(&mod.module)) { wasm = std::make_shared(); switch (quoted->type) { case QuotedModuleType::Text: { @@ -114,7 +121,7 @@ struct Shell { break; } } - } else if (auto* ptr = std::get_if>(&mod)) { + } else if (auto* ptr = std::get_if>(&mod.module)) { wasm = *ptr; } else { WASM_UNREACHABLE("unexpected module kind"); @@ -130,23 +137,47 @@ struct Shell { return Ok{}; } - using InstanceInfo = std::pair, - std::shared_ptr>; + Result<> doInstantiate(ModuleInstantiation& instantiateModule) { + auto moduleDefinitionName = instantiateModule.moduleName + ? instantiateModule.moduleName + : lastModuleDefinition; + if (!moduleDefinitionName) { + return Err{"No module definition found in module instantiation, and no " + "previous module definition was found."}; + } + + auto instanceName = instantiateModule.instanceName + ? instantiateModule.instanceName + : lastModuleDefinition; + if (!instanceName) { + return Err{"No instance name found in module instantiation, and no " + "previous module definition was found."}; + } - Result instantiate(Module& wasm) { + return instantiate(*modules[*moduleDefinitionName], *instanceName); + } + + Result<> instantiate(Module& wasm, Name instanceName) { + std::shared_ptr interface; + std::shared_ptr instance; try { - auto interface = - std::make_shared(linkedInstances); - auto instance = + interface = std::make_shared(linkedInstances); + instance = std::make_shared(wasm, interface.get(), linkedInstances); + // This is not an optimization: we want to execute anything, even relaxed // SIMD instructions. instance->setRelaxedBehavior(ModuleRunner::RelaxedBehavior::Execute); instance->instantiate(); - return {{std::move(interface), std::move(instance)}}; } catch (...) { return Err{"failed to instantiate module"}; } + + lastInstance = instanceName; + + interfaces[instanceName] = std::move(interface); + instances[instanceName] = std::move(instance); + return Ok{}; } Result<> addModule(WASTModule& mod) { @@ -156,20 +187,20 @@ struct Shell { auto wasm = *module; CHECK_ERR(validateModule(*wasm)); - auto instanceInfo = instantiate(*wasm); - CHECK_ERR(instanceInfo); - - auto& [interface, instance] = *instanceInfo; - lastModule = wasm->name; - modules[lastModule] = std::move(wasm); - interfaces[lastModule] = std::move(interface); - instances[lastModule] = std::move(instance); + modules[wasm->name] = wasm; + if (!mod.isDefinition) { + CHECK_ERR(instantiate(*wasm, wasm->name)); + } else { + lastModuleDefinition = wasm->name; + } return Ok{}; } Result<> addRegistration(Register& reg) { - auto instance = instances[lastModule]; + Name instanceName = reg.instanceName ? *reg.instanceName : lastInstance; + + auto instance = instances[instanceName]; if (!instance) { return Err{"register called without a module"}; } @@ -177,9 +208,8 @@ struct Shell { // We copy pointers as a registered module's name might still be used // in an assertion or invoke command. - modules[reg.name] = modules[lastModule]; - interfaces[reg.name] = interfaces[lastModule]; - instances[reg.name] = instances[lastModule]; + interfaces[reg.name] = interfaces[instanceName]; + instances[reg.name] = instances[instanceName]; return Ok{}; } @@ -212,9 +242,9 @@ struct Shell { } ActionResult doAction(Action& act) { - assert(instances[lastModule].get()); + assert(instances[lastInstance].get()); if (auto* invoke = std::get_if(&act)) { - auto it = instances.find(invoke->base ? *invoke->base : lastModule); + auto it = instances.find(invoke->base ? *invoke->base : lastInstance); if (it == instances.end()) { return TrapResult{}; } @@ -239,7 +269,7 @@ struct Shell { } return flow.values; } else if (auto* get = std::get_if(&act)) { - auto it = instances.find(get->base ? *get->base : lastModule); + auto it = instances.find(get->base ? *get->base : lastInstance); if (it == instances.end()) { return TrapResult{}; } @@ -441,7 +471,7 @@ struct Shell { return Err{"expected invalid module"}; } - auto instance = instantiate(**wasm); + auto instance = instantiate(**wasm, (*wasm)->name); if (auto* err = instance.getErr()) { if (assn.type == ModuleAssertionType::Unlinkable || assn.type == ModuleAssertionType::Trap) { @@ -513,15 +543,18 @@ struct Shell { // print_* functions are handled separately, no need to define here. - WASTModule mod = std::move(spectest); + WASTModule mod = {/*isDefinition=*/false, spectest}; auto added = addModule(mod); if (added.getErr()) { WASM_UNREACHABLE("error building spectest module"); } - Register registration{"spectest"}; + Register registration{/*name=*/"spectest"}; + modules["spectest"] = spectest; auto registered = addRegistration(registration); if (registered.getErr()) { - WASM_UNREACHABLE("error registering spectest module"); + WASM_UNREACHABLE((std::string("error registering spectest module: ") + + registered.getErr()->msg) + .c_str()); } } }; diff --git a/src/tools/wasm2js.cpp b/src/tools/wasm2js.cpp index e0f61a21766..c6277c6c30f 100644 --- a/src/tools/wasm2js.cpp +++ b/src/tools/wasm2js.cpp @@ -788,7 +788,11 @@ void AssertionEmitter::emit() { Name testFuncName("check" + std::to_string(i)); auto& cmd = script[i].cmd; if (auto* mod = std::get_if(&cmd)) { - if (auto* w = std::get_if>(mod)) { + if (mod->isDefinition) { + Fatal() << "Module definition is not supported on line " + << script[i].line; + } + if (auto* w = std::get_if>(&mod->module)) { wasm = *w; // We have already done the parse, but we still do this to apply the // features from the command line. @@ -951,7 +955,10 @@ int main(int argc, const char* argv[]) { Fatal() << "expected module"; } if (auto* mod = std::get_if(&(*script)[0].cmd)) { - if (auto* w = std::get_if>(mod)) { + if (mod->isDefinition) { + Fatal() << "module definition is not supported"; + } + if (auto* w = std::get_if>(&mod->module)) { wasm = *w; // This isn't actually before the parse, but we can't apply the // feature options any earlier. FIXME.