From 8d37463fac2bea9e49cda1c19aa137f673a38251 Mon Sep 17 00:00:00 2001 From: dcodeIO Date: Sat, 28 Oct 2017 14:59:09 +0200 Subject: [PATCH 1/4] Do not print the entire and possibly very large module when validation fails --- src/wasm/wasm-validator.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 4359b4e13bd..4564191c29d 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -1036,8 +1036,6 @@ bool WasmValidator::validate(Module& module, FeatureSet features, Flags flags) { std::cerr << info.getStream(func.get()).str(); } std::cerr << info.getStream(nullptr).str(); - // also print the module - WasmPrinter::printModule(&module, std::cerr); } return info.valid.load(); } From 999a227f5aeb36f68446a92337a08c291f35cd90 Mon Sep 17 00:00:00 2001 From: dcodeIO Date: Tue, 31 Oct 2017 04:08:14 +0100 Subject: [PATCH 2/4] Explicitly print module contents in tools when validation fails --- src/passes/pass.cpp | 2 ++ src/tools/asm2wasm.cpp | 1 + src/tools/s2wasm.cpp | 4 +++- src/tools/wasm-as.cpp | 1 + src/tools/wasm-merge.cpp | 1 + src/tools/wasm-opt.cpp | 14 ++++++++++++-- src/tools/wasm-shell.cpp | 9 ++++++++- 7 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index d92a4a80c44..f2463691a7a 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -232,6 +232,7 @@ void PassRunner::run() { // validate, ignoring the time std::cerr << "[PassRunner] (validating)\n"; if (!WasmValidator().validate(*wasm, options.features, validationFlags)) { + WasmPrinter::printModule(wasm); if (passDebug >= 2) { std::cerr << "Last pass (" << pass->name << ") broke validation. Here is the module before: \n" << moduleBefore.str() << "\n"; } else { @@ -247,6 +248,7 @@ void PassRunner::run() { // validate std::cerr << "[PassRunner] (final validation)\n"; if (!WasmValidator().validate(*wasm, options.features, validationFlags)) { + WasmPrinter::printModule(wasm); std::cerr << "final module does not validate\n"; abort(); } diff --git a/src/tools/asm2wasm.cpp b/src/tools/asm2wasm.cpp index 4ec68283fef..3f70e0388e6 100644 --- a/src/tools/asm2wasm.cpp +++ b/src/tools/asm2wasm.cpp @@ -207,6 +207,7 @@ int main(int argc, const char *argv[]) { } if (!WasmValidator().validate(wasm, options.passOptions.features)) { + WasmPrinter::printModule(&wasm); Fatal() << "error in validating output"; } diff --git a/src/tools/s2wasm.cpp b/src/tools/s2wasm.cpp index 37604e74ba8..76146d6f1e7 100644 --- a/src/tools/s2wasm.cpp +++ b/src/tools/s2wasm.cpp @@ -194,8 +194,10 @@ int main(int argc, const char *argv[]) { if (options.extra["validate"] != "none") { if (options.debug) std::cerr << "Validating..." << std::endl; - if (!wasm::WasmValidator().validate(linker.getOutput().wasm, + Module* output = &linker.getOutput.wasm; + if (!wasm::WasmValidator().validate(*output, WasmValidator::Globally | (options.extra["validate"] == "web" ? WasmValidator::Web : 0))) { + WasmPrinter::printModule(output); Fatal() << "Error: linked module is not valid.\n"; } } diff --git a/src/tools/wasm-as.cpp b/src/tools/wasm-as.cpp index d456f43fdcf..0e053e05a18 100644 --- a/src/tools/wasm-as.cpp +++ b/src/tools/wasm-as.cpp @@ -94,6 +94,7 @@ int main(int argc, const char *argv[]) { if (options.debug) std::cerr << "Validating..." << std::endl; if (!wasm::WasmValidator().validate(wasm, Feature::All, WasmValidator::Globally | (options.extra["validate"] == "web" ? WasmValidator::Web : 0))) { + WasmPrinter::printModule(&wasm); Fatal() << "Error: input module is not valid.\n"; } } diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index 441cdc0f7a0..7a713675f8a 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -627,6 +627,7 @@ int main(int argc, const char* argv[]) { } if (!WasmValidator().validate(output)) { + WasmPrinter::printModule(&output); Fatal() << "error in validating output"; } diff --git a/src/tools/wasm-opt.cpp b/src/tools/wasm-opt.cpp index 7c2a3a35a1d..91e1633f0af 100644 --- a/src/tools/wasm-opt.cpp +++ b/src/tools/wasm-opt.cpp @@ -129,6 +129,7 @@ int main(int argc, const char* argv[]) { } if (!WasmValidator().validate(wasm, features)) { + WasmPrinter::printModule(&wasm); Fatal() << "error in validating input"; } } else { @@ -136,6 +137,7 @@ int main(int argc, const char* argv[]) { TranslateToFuzzReader reader(wasm); reader.read(options.extra["infile"]); if (!WasmValidator().validate(wasm, features)) { + WasmPrinter::printModule(&wasm); std::cerr << "translate-to-fuzz must always generate a valid module"; abort(); } @@ -176,7 +178,11 @@ int main(int argc, const char* argv[]) { if (options.runningPasses()) { if (options.debug) std::cerr << "running passes...\n"; options.runPasses(wasm); - assert(WasmValidator().validate(wasm, features)); + bool valid = WasmValidator().validate(wasm, features); + if (!valid) { + WasmPrinter::printModule(&wasm); + } + assert(valid); } if (fuzzExec) { @@ -192,7 +198,11 @@ int main(int argc, const char* argv[]) { auto input = buffer.getAsChars(); WasmBinaryBuilder parser(second, input, false); parser.read(); - assert(WasmValidator().validate(second, features)); + bool valid = WasmValidator().validate(second, features); + if (!valid) { + WasmPrinter::printModule(&second); + } + assert(valid); } results.check(*compare); } diff --git a/src/tools/wasm-shell.cpp b/src/tools/wasm-shell.cpp index cd8c2743717..b4f81a13958 100644 --- a/src/tools/wasm-shell.cpp +++ b/src/tools/wasm-shell.cpp @@ -143,6 +143,9 @@ static void run_asserts(Name moduleName, size_t* i, bool* checked, Module* wasm, if (!invalid) { // maybe parsed ok, but otherwise incorrect invalid = !WasmValidator().validate(wasm); + if (invalid) { + WasmPrinter::printModule(&wasm); + } } if (!invalid && id == ASSERT_UNLINKABLE) { // validate "instantiating" the mdoule @@ -287,7 +290,11 @@ int main(int argc, const char* argv[]) { builders[moduleName].swap(builder); modules[moduleName].swap(module); i++; - assert(WasmValidator().validate(*modules[moduleName])); + bool valid = WasmValidator().validate(*modules[moduleName]); + if (!valid) { + WasmPrinter::printModule(modules[moduleName].get()); + } + assert(valid); run_asserts(moduleName, &i, &checked, modules[moduleName].get(), &root, builders[moduleName].get(), entry); } else { run_asserts(Name(), &i, &checked, nullptr, &root, nullptr, entry); From 2e0a58e74e06713ab090e830a36e29671b061acf Mon Sep 17 00:00:00 2001 From: dcodeIO Date: Tue, 31 Oct 2017 14:11:41 +0100 Subject: [PATCH 3/4] fix --- src/tools/s2wasm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/s2wasm.cpp b/src/tools/s2wasm.cpp index 76146d6f1e7..96d9ae5ce10 100644 --- a/src/tools/s2wasm.cpp +++ b/src/tools/s2wasm.cpp @@ -194,7 +194,7 @@ int main(int argc, const char *argv[]) { if (options.extra["validate"] != "none") { if (options.debug) std::cerr << "Validating..." << std::endl; - Module* output = &linker.getOutput.wasm; + Module* output = &linker.getOutput().wasm; if (!wasm::WasmValidator().validate(*output, WasmValidator::Globally | (options.extra["validate"] == "web" ? WasmValidator::Web : 0))) { WasmPrinter::printModule(output); From 1e85f41f0dc3fd47b4b8d8f114ff9a157ceef79a Mon Sep 17 00:00:00 2001 From: dcodeIO Date: Wed, 1 Nov 2017 11:15:46 +0100 Subject: [PATCH 4/4] fix --- src/tools/wasm-shell.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tools/wasm-shell.cpp b/src/tools/wasm-shell.cpp index b4f81a13958..965887cf900 100644 --- a/src/tools/wasm-shell.cpp +++ b/src/tools/wasm-shell.cpp @@ -143,9 +143,6 @@ static void run_asserts(Name moduleName, size_t* i, bool* checked, Module* wasm, if (!invalid) { // maybe parsed ok, but otherwise incorrect invalid = !WasmValidator().validate(wasm); - if (invalid) { - WasmPrinter::printModule(&wasm); - } } if (!invalid && id == ASSERT_UNLINKABLE) { // validate "instantiating" the mdoule