From 95570faf9178acb2482f77fba3e7b967d5a7bc5b Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 29 Nov 2018 14:58:58 -0800 Subject: [PATCH 1/2] Add support for a mutable globals as a Feature This picks up from #1644 and indeed borrows the test case from there. --- src/tools/wasm-opt.cpp | 2 ++ src/wasm.h | 1 + src/wasm/wasm-binary.cpp | 3 +-- src/wasm/wasm-s-parser.cpp | 6 ++++-- src/wasm/wasm-validator.cpp | 8 ++++++++ test/mutable-global.wasm | Bin 0 -> 52 bytes test/mutable-global.wasm.fromBinary | 13 +++++++++++++ test/mutable-global.wast | 12 ++++++++++++ test/mutable-global.wast.from-wast | 12 ++++++++++++ test/mutable-global.wast.fromBinary | 13 +++++++++++++ test/mutable-global.wast.fromBinary.noDebugInfo | 13 +++++++++++++ 11 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 test/mutable-global.wasm create mode 100644 test/mutable-global.wasm.fromBinary create mode 100644 test/mutable-global.wast create mode 100644 test/mutable-global.wast.from-wast create mode 100644 test/mutable-global.wast.fromBinary create mode 100644 test/mutable-global.wast.fromBinary.noDebugInfo diff --git a/src/tools/wasm-opt.cpp b/src/tools/wasm-opt.cpp index 7d35efeab13..421bcca5936 100644 --- a/src/tools/wasm-opt.cpp +++ b/src/tools/wasm-opt.cpp @@ -137,6 +137,8 @@ int main(int argc, const char* argv[]) { // It should be safe to just always enable atomics in wasm-opt, because we // don't expect any passes to accidentally generate atomic ops FeatureSet features = Feature::Atomics; + // Same for MutableGlobals + features |= Feature::MutableGlobals; if (options.debug) std::cerr << "reading...\n"; diff --git a/src/wasm.h b/src/wasm.h index a0f634d1735..2e09ef08881 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -40,6 +40,7 @@ namespace wasm { enum Feature : uint32_t { MVP = 0, Atomics = 1 << 0, + MutableGlobals = 1 << 1, All = 0xffffffff, }; typedef uint32_t FeatureSet; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 2dc89ee90bf..eebf0bd63d5 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -199,7 +199,7 @@ void WasmBinaryWriter::writeImports() { writeImportHeader(global); o << U32LEB(int32_t(ExternalKind::Global)); o << binaryType(global->type); - o << U32LEB(0); // Mutable globals can't be imported for now. + o << U32LEB(global->mutable_); }); if (wasm->memory.imported()) { if (debug) std::cerr << "write one memory" << std::endl; @@ -1009,7 +1009,6 @@ void WasmBinaryBuilder::readImports() { auto name = Name(std::string("gimport$") + std::to_string(i)); auto type = getConcreteType(); auto mutable_ = getU32LEB(); - assert(!mutable_); // for now, until mutable globals auto* curr = builder.makeGlobal(name, type, nullptr, mutable_ ? Builder::Mutable : Builder::Immutable); curr->module = module; curr->base = base; diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 34022b72b84..42dd9df0d8a 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1487,19 +1487,21 @@ void SExpressionWasmBuilder::parseImport(Element& s) { wasm.addFunction(func.release()); } else if (kind == ExternalKind::Global) { Type type; + bool mutable_ = false; if (inner[j]->isStr()) { type = stringToType(inner[j]->str()); } else { auto& inner2 = *inner[j]; if (inner2[0]->str() != MUT) throw ParseException("expected mut"); type = stringToType(inner2[1]->str()); - throw ParseException("cannot import a mutable global", s.line, s.col); + mutable_ = true; } auto global = make_unique(); global->name = name; global->module = module; global->base = base; global->type = type; + global->mutable_ = mutable_; wasm.addGlobal(global.release()); } else if (kind == ExternalKind::Table) { wasm.table.module = module; @@ -1571,12 +1573,12 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) { if (importModule.is()) { // this is an import, actually if (!preParseImport) throw ParseException("!preParseImport in global"); - if (mutable_) throw ParseException("cannot import a mutable global", s.line, s.col); auto im = make_unique(); im->name = global->name; im->module = importModule; im->base = importBase; im->type = type; + im->mutable_ = mutable_; if (wasm.getGlobalOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col); wasm.addGlobal(im.release()); return; diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index a08c9a12917..67bcd37fe0a 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -923,6 +923,11 @@ static void validateImports(Module& module, ValidationInfo& info) { } } }); + if (!(info.features & Feature::MutableGlobals)) { + ModuleUtils::iterImportedGlobals(module, [&](Global* curr) { + info.shouldBeFalse(curr->mutable_, curr->name, "Imported global cannot be mutable"); + }); + } } static void validateExports(Module& module, ValidationInfo& info) { @@ -935,6 +940,9 @@ static void validateExports(Module& module, ValidationInfo& info) { info.shouldBeUnequal(param, i64, f->name, "Exported function must not have i64 parameters"); } } + } else if (curr->kind == ExternalKind::Global && !(info.features & Feature::MutableGlobals)) { + Global* g = module.getGlobalOrNull(curr->value); + info.shouldBeFalse(g->mutable_, g->name, "Exported global cannot be mutable"); } } std::unordered_set exportNames; diff --git a/test/mutable-global.wasm b/test/mutable-global.wasm new file mode 100644 index 0000000000000000000000000000000000000000..537a596fd09019d83ffb506cfbe2a38d205bc50d GIT binary patch literal 52 zcmZQbEY4+QU|?WmVN76PU=n6zPR%RhO3%qpO3cyCEiGZLXJlq#WZ>dv literal 0 HcmV?d00001 diff --git a/test/mutable-global.wasm.fromBinary b/test/mutable-global.wasm.fromBinary new file mode 100644 index 00000000000..aba18cdc40d --- /dev/null +++ b/test/mutable-global.wasm.fromBinary @@ -0,0 +1,13 @@ +(module + (type $0 (func)) + (import "env" "global-mut" (global $gimport$0 (mut i32))) + (func $0 (; 0 ;) (type $0) + (set_global $gimport$0 + (i32.add + (get_global $gimport$0) + (i32.const 1) + ) + ) + ) +) + diff --git a/test/mutable-global.wast b/test/mutable-global.wast new file mode 100644 index 00000000000..71d333b1794 --- /dev/null +++ b/test/mutable-global.wast @@ -0,0 +1,12 @@ +(module + (type $0 (func)) + (import "env" "global-mut" (global $global-mut (mut i32))) + (func $foo (type $0) + (set_global $global-mut + (i32.add + (get_global $global-mut) + (i32.const 1) + ) + ) + ) +) diff --git a/test/mutable-global.wast.from-wast b/test/mutable-global.wast.from-wast new file mode 100644 index 00000000000..f12fb3fa427 --- /dev/null +++ b/test/mutable-global.wast.from-wast @@ -0,0 +1,12 @@ +(module + (type $0 (func)) + (import "env" "global-mut" (global $global-mut (mut i32))) + (func $foo (; 0 ;) (type $0) + (set_global $global-mut + (i32.add + (get_global $global-mut) + (i32.const 1) + ) + ) + ) +) diff --git a/test/mutable-global.wast.fromBinary b/test/mutable-global.wast.fromBinary new file mode 100644 index 00000000000..3273575ef0a --- /dev/null +++ b/test/mutable-global.wast.fromBinary @@ -0,0 +1,13 @@ +(module + (type $0 (func)) + (import "env" "global-mut" (global $gimport$0 (mut i32))) + (func $foo (; 0 ;) (type $0) + (set_global $gimport$0 + (i32.add + (get_global $gimport$0) + (i32.const 1) + ) + ) + ) +) + diff --git a/test/mutable-global.wast.fromBinary.noDebugInfo b/test/mutable-global.wast.fromBinary.noDebugInfo new file mode 100644 index 00000000000..aba18cdc40d --- /dev/null +++ b/test/mutable-global.wast.fromBinary.noDebugInfo @@ -0,0 +1,13 @@ +(module + (type $0 (func)) + (import "env" "global-mut" (global $gimport$0 (mut i32))) + (func $0 (; 0 ;) (type $0) + (set_global $gimport$0 + (i32.add + (get_global $gimport$0) + (i32.const 1) + ) + ) + ) +) + From d083222c4562af6ce3aeaef7506dad00b2b42570 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 29 Nov 2018 16:16:16 -0800 Subject: [PATCH 2/2] fix test --- src/wasm/wasm-validator.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 67bcd37fe0a..b88a94e6d02 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -941,8 +941,9 @@ static void validateExports(Module& module, ValidationInfo& info) { } } } else if (curr->kind == ExternalKind::Global && !(info.features & Feature::MutableGlobals)) { - Global* g = module.getGlobalOrNull(curr->value); - info.shouldBeFalse(g->mutable_, g->name, "Exported global cannot be mutable"); + if (Global* g = module.getGlobalOrNull(curr->value)) { + info.shouldBeFalse(g->mutable_, g->name, "Exported global cannot be mutable"); + } } } std::unordered_set exportNames;