Validator: Pass the module along when printing errors, so type names are used (#3727)

For example, on this invalid wat:

(module
  (type $vec (struct (field i64)))
  (func $test
    (drop
      (struct.new_with_rtt $vec (i32.const 1) (rtt.canon $vec))
    )
  )
)

We used to print:

[wasm-validator error in function test] struct.new operand must have proper type, on 
(struct.new_with_rtt ${i64}
 (i32.const 1)
 (rtt.canon ${i64})
)

We will now print:

[wasm-validator error in function test] struct.new operand must have proper type, on 
(struct.new_with_rtt $vec
 (i32.const 1)
 (rtt.canon $vec)
)

Note that $vec is used. In real-world examples the autogenerated structural name
can be huge, which this avoids.

3 files changed, 22 insertions, 9 deletions

diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 6dfd495c0..31a6ccaae 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -31,7 +31,8 @@ namespace wasm {
 static std::ostream& printExpression(Expression* expression,
                                      std::ostream& o,
                                      bool minify = false,
-                                     bool full = false);
+                                     bool full = false,
+                                     Module* wasm = nullptr);
 
 static std::ostream&
 printStackInst(StackInst* inst, std::ostream& o, Function* func = nullptr);
@@ -3039,13 +3040,15 @@ Pass* createPrintStackIRPass() { return new PrintStackIR(); }
 static std::ostream& printExpression(Expression* expression,
                                      std::ostream& o,
                                      bool minify,
-                                     bool full) {
+                                     bool full,
+                                     Module* wasm) {
   if (!expression) {
     o << "(null expression)";
     return o;
   }
   PrintSExpression print(o);
   print.setMinify(minify);
+  print.currModule = wasm;
   if (full || isFullForced()) {
     print.setFull(true);
     o << "[" << expression->type << "] ";
@@ -3215,6 +3218,10 @@ std::ostream& operator<<(std::ostream& o, wasm::Expression* expression) {
   return wasm::printExpression(expression, o);
 }
 
+std::ostream& operator<<(std::ostream& o, wasm::ModuleExpression pair) {
+  return wasm::printExpression(pair.second, o, false, false, &pair.first);
+}
+
 std::ostream& operator<<(std::ostream& o, wasm::StackInst& inst) {
   return wasm::printStackInst(&inst, o);
 }
diff --git a/src/wasm.h b/src/wasm.h
index 4747b48a3..101f45c81 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -1945,6 +1945,8 @@ public:
   void clearDebugInfo();
 };
 
+using ModuleExpression = std::pair<Module&, Expression*>;
+
 } // namespace wasm
 
 namespace std {
@@ -1956,6 +1958,7 @@ template<> struct hash<wasm::Address> {
 
 std::ostream& operator<<(std::ostream& o, wasm::Module& module);
 std::ostream& operator<<(std::ostream& o, wasm::Expression& expression);
+std::ostream& operator<<(std::ostream& o, wasm::ModuleExpression pair);
 std::ostream& operator<<(std::ostream& o, wasm::StackInst& inst);
 std::ostream& operator<<(std::ostream& o, wasm::StackIR& ir);
 
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index b49b4bb0f..14955ecae 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -35,22 +35,25 @@ template<typename T,
          typename std::enable_if<!std::is_base_of<
            Expression,
            typename std::remove_pointer<T>::type>::value>::type* = nullptr>
-inline std::ostream& printModuleComponent(T curr, std::ostream& stream) {
+inline std::ostream&
+printModuleComponent(T curr, std::ostream& stream, Module& wasm) {
   stream << curr << std::endl;
   return stream;
 }
 
 // Extra overload for Expressions, to print their contents.
-inline std::ostream& printModuleComponent(Expression* curr,
-                                          std::ostream& stream) {
+inline std::ostream&
+printModuleComponent(Expression* curr, std::ostream& stream, Module& wasm) {
   if (curr) {
-    stream << *curr << '\n';
+    stream << ModuleExpression(wasm, curr) << '\n';
   }
   return stream;
 }
 
 // For parallel validation, we have a helper struct for coordination
 struct ValidationInfo {
+  Module& wasm;
+
   bool validateWeb;
   bool validateGlobally;
   bool quiet;
@@ -63,7 +66,7 @@ struct ValidationInfo {
   std::mutex mutex;
   std::unordered_map<Function*, std::unique_ptr<std::ostringstream>> outputs;
 
-  ValidationInfo() { valid.store(true); }
+  ValidationInfo(Module& wasm) : wasm(wasm) { valid.store(true); }
 
   std::ostringstream& getStream(Function* func) {
     std::unique_lock<std::mutex> lock(mutex);
@@ -86,7 +89,7 @@ struct ValidationInfo {
     }
     auto& ret = printFailureHeader(func);
     ret << text << ", on \n";
-    return printModuleComponent(curr, ret);
+    return printModuleComponent(curr, ret, wasm);
   }
 
   std::ostream& printFailureHeader(Function* func) {
@@ -2919,7 +2922,7 @@ static void validateFeatures(Module& module, ValidationInfo& info) {
 // then Using PassRunner::getPassDebug causes a circular dependence. We should
 // fix that, perhaps by moving some of the pass infrastructure into libsupport.
 bool WasmValidator::validate(Module& module, Flags flags) {
-  ValidationInfo info;
+  ValidationInfo info(module);
   info.validateWeb = (flags & Web) != 0;
   info.validateGlobally = (flags & Globally) != 0;
   info.quiet = (flags & Quiet) != 0;