[WebAssembly] Use location of operand for operand-based type check errors

[asb]

Summary

This addresses a series of FIXMEs introduced in D122020.

A follow-up patch addresses the bug that is exposed by this change (an issue with source location information when lexing identifiers).

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

case Match_Success: {
         // an offset64 arg instead of offset32, but to the assembler matcher
         // they're both immediates so don't get selected for.
         auto Opc64 = WebAssembly::getWasm64Opcode(
             static_cast<uint16_t>(Inst.getOpcode()));
         if (Opc64 >= 0) {
           Inst.setOpcode(Opc64);
         }
       }
-      if (!SkipTypeCheck && TC.typeCheck(IDLoc, Inst))
+      if (!SkipTypeCheck && TC.typeCheck(IDLoc, Inst, Operands))
         return true;
       Out.emitInstruction(Inst, getSTI());
       if (CurrentState == EndFunction) {
         onEndOfFunction(IDLoc);
       } else {
         CurrentState = Instructions;
       }
       return false;

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h

 ///
 /// It contains code to translate a parsed .s file into MCInsts.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_WEBASSEMBLY_ASMPARSER_TYPECHECK_H
 #define LLVM_LIB_TARGET_WEBASSEMBLY_ASMPARSER_TYPECHECK_H
 
-#include "llvm/MC/MCParser/MCAsmParser.h"
-#include "llvm/MC/MCInstrInfo.h"
 #include "llvm/BinaryFormat/Wasm.h"
+#include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCParser/MCAsmParser.h"
+#include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCSymbol.h"
 
 namespace llvm {
 
 class WebAssemblyAsmTypeCheck final {
   MCAsmParser &Parser;
   const MCInstrInfo &MII;
 

 
 public:
   WebAssemblyAsmTypeCheck(MCAsmParser &Parser, const MCInstrInfo &MII, bool is64);
 
   void funcDecl(const wasm::WasmSignature &Sig);
   void localDecl(const SmallVector<wasm::ValType, 4> &Locals);
   void setLastSig(const wasm::WasmSignature &Sig) { LastSig = Sig; }
   bool endOfFunction(SMLoc ErrorLoc);
-  bool typeCheck(SMLoc ErrorLoc, const MCInst &Inst);
+  bool typeCheck(SMLoc ErrorLoc, const MCInst &Inst, OperandVector &Operands);
 
   void Clear() {
     Stack.clear();
     LocalTypes.clear();
     ReturnTypes.clear();
     TypeErrorThisFunction = false;
     Unreachable = false;
   }
 };
 
 } // end namespace llvm
 
 #endif  // LLVM_LIB_TARGET_WEBASSEMBLY_ASMPARSER_TYPECHECK_H

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp

bool WebAssemblyAsmTypeCheck::endOfFunction(SMLoc ErrorLoc) {
   if (!Stack.empty()) {
     return typeError(ErrorLoc, std::to_string(Stack.size()) +
                                    " superfluous return values");
   }
   Unreachable = true;
   return false;
 }
 
-bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst) {
+bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
+                                        OperandVector &Operands) {
   auto Opc = Inst.getOpcode();
   auto Name = GetMnemonic(Opc);
   dumpTypeStack("typechecking " + Name + ": ");
   wasm::ValType Type;
   if (Name == "local.get") {
-    if (getLocal(ErrorLoc, Inst, Type))
+    if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     Stack.push_back(Type);
   } else if (Name == "local.set") {
-    if (getLocal(ErrorLoc, Inst, Type))
+    if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     if (popType(ErrorLoc, Type))
       return true;
   } else if (Name == "local.tee") {
-    if (getLocal(ErrorLoc, Inst, Type))
+    if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     if (popType(ErrorLoc, Type))
       return true;
     Stack.push_back(Type);
   } else if (Name == "global.get") {
-    if (getGlobal(ErrorLoc, Inst, Type))
+    if (getGlobal(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     Stack.push_back(Type);
   } else if (Name == "global.set") {
-    if (getGlobal(ErrorLoc, Inst, Type))
+    if (getGlobal(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     if (popType(ErrorLoc, Type))
       return true;
   } else if (Name == "table.get") {
-    if (getTable(ErrorLoc, Inst, Type))
+    if (getTable(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     if (popType(ErrorLoc, wasm::ValType::I32))
       return true;
     Stack.push_back(Type);
   } else if (Name == "table.set") {
-    if (getTable(ErrorLoc, Inst, Type))
+    if (getTable(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     if (popType(ErrorLoc, Type))
       return true;
     if (popType(ErrorLoc, wasm::ValType::I32))
       return true;
   } else if (Name == "table.fill") {
-    if (getTable(ErrorLoc, Inst, Type))
+    if (getTable(Operands[1]->getStartLoc(), Inst, Type))
       return true;
     if (popType(ErrorLoc, wasm::ValType::I32))
       return true;
     if (popType(ErrorLoc, Type))
       return true;
     if (popType(ErrorLoc, wasm::ValType::I32))
       return true;
   } else if (Name == "drop") {
bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
   } else if (Name == "call_indirect" || Name == "return_call_indirect") {
     // Function value.
     if (popType(ErrorLoc, wasm::ValType::I32)) return true;
     if (checkSig(ErrorLoc, LastSig)) return true;
     if (Name == "return_call_indirect" && endOfFunction(ErrorLoc))
       return true;
   } else if (Name == "call" || Name == "return_call") {
     const MCSymbolRefExpr *SymRef;
-    if (getSymRef(ErrorLoc, Inst, SymRef))
+    if (getSymRef(Operands[1]->getStartLoc(), Inst, SymRef))
       return true;
     auto WasmSym = cast<MCSymbolWasm>(&SymRef->getSymbol());
     auto Sig = WasmSym->getSignature();
     if (!Sig || WasmSym->getType() != wasm::WASM_SYMBOL_TYPE_FUNCTION)
-      return typeError(ErrorLoc, StringRef("symbol ") + WasmSym->getName() +
+      return typeError(Operands[1]->getStartLoc(), StringRef("symbol ") +
+                                                       WasmSym->getName() +
                                                        " missing .functype");
     if (checkSig(ErrorLoc, *Sig)) return true;
     if (Name == "return_call" && endOfFunction(ErrorLoc))
       return true;
   } else if (Name == "catch") {
     const MCSymbolRefExpr *SymRef;
-    if (getSymRef(ErrorLoc, Inst, SymRef))
+    if (getSymRef(Operands[1]->getStartLoc(), Inst, SymRef))
       return true;
     const auto *WasmSym = cast<MCSymbolWasm>(&SymRef->getSymbol());
     const auto *Sig = WasmSym->getSignature();
     if (!Sig || WasmSym->getType() != wasm::WASM_SYMBOL_TYPE_TAG)
-      return typeError(ErrorLoc, StringRef("symbol ") + WasmSym->getName() +
+      return typeError(Operands[1]->getStartLoc(), StringRef("symbol ") +
+                                                       WasmSym->getName() +
                                                        " missing .tagtype");
     // catch instruction pushes values whose types are specified in the tag's
     // "params" part
     Stack.insert(Stack.end(), Sig->Params.begin(), Sig->Params.end());
   } else if (Name == "unreachable") {
     Unreachable = true;
   } else {
     // The current instruction is a stack instruction which doesn't have
     // explicit operands that indicate push/pop types, so we get those from

llvm/test/MC/WebAssembly/type-checker-errors.s

 # RUN: not llvm-mc -triple=wasm32 -mattr=+exception-handling,+reference-types,+tail-call %s 2>&1 | FileCheck %s
 
 # These tests are intended to act as a litmus test for the WebAssembly ASM
 # type-checker - both in terms of errors it can catch and in terms of the
 # location information used in the error messages.
 
 local_get_no_local_type:
   .functype local_get_no_local_type () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: no local type specified for index 0
+# CHECK: :[[@LINE+1]]:13: error: no local type specified for index 0
   local.get 0
   end_function
 
 local_set_no_local_type:
   .functype local_set_no_local_type () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: no local type specified for index 0
+# CHECK: :[[@LINE+1]]:13: error: no local type specified for index 0
   local.set 0
   end_function
 
 local_set_empty_stack_while_popping:
   .functype local_set_empty_stack_while_popping () -> ()
   .local i32
 # CHECK: [[@LINE+1]]:3: error: empty stack while popping i32
   local.set 0
   end_function
 
 local_set_type_mismatch:
   .functype local_set_type_mismatch () -> ()
   .local i32
   f32.const 1.0
 # CHECK: [[@LINE+1]]:3: error: popped f32, expected i32
   local.set 0
   end_function
 
 local_tee_no_local_type:
   .functype local_tee_no_local_type () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: no local type specified for index 0
+# CHECK: :[[@LINE+1]]:13: error: no local type specified for index 0
   local.tee 0
   end_function
 
 local_tee_empty_stack_while_popping:
   .functype local_tee_empty_stack_while_popping () -> ()
   .local f32
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping f32
   local.tee 0
   end_function
 
 local_tee_type_mismatch:
   .functype local_tee_type_mismatch () -> ()
   .local f32
   i32.const 1
 # CHECK: :[[@LINE+1]]:3: error: popped i32, expected f32
   local.tee 0
   end_function
 
 global_get_missing_globaltype:
   .functype global_get_missing_globaltype () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .globaltype
+# FIXME: Error location should be at beginning of operand.
+# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .globaltype
   global.get foo
   end_function
 
 global_get_expected_expression_operand:
   .functype global_get_expected_expression_operand () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:14: error: expected expression operand
   global.get 1
   end_function
 
 global_set_missing_globaltype:
   .functype global_set_missing_globaltype () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .globaltype
+# FIXME: Error location should be at beginning of operand.
+# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .globaltype
   global.set foo
   end_function
 
 global_set_expected_expression_operand:
   .functype global_set_expected_expression_operand () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:14: error: expected expression operand
   global.set 1
   end_function
 
 global_set_empty_stack_while_popping:
   .functype global_set_empty_stack_while_popping () -> ()
   .globaltype valid_global, i64
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping i64
   global.set valid_global
   end_function
 
 global_set_type_mismatch:
   .functype global_set_type_mismatch () -> ()
   .globaltype valid_global, i64
   i32.const 1
 # CHECK: :[[@LINE+1]]:3: error: popped i32, expected i64
   global.set valid_global
   end_function
 
 table_get_expected_expression_operand:
   .functype table_get_expected_expression_operand () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:13: error: expected expression operand
   table.get 1
   end_function
 
 table_get_missing_tabletype:
   .functype table_get_missing_tabletype () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
+# FIXME: Error location should be at beginning of operand.
+# CHECK: :[[@LINE+1]]:16: error: symbol foo missing .tabletype
   table.get foo
   end_function
 
 .tabletype valid_table, externref
 
 table_get_empty_stack_while_popping:
   .functype table_get_empty_stack_while_popping () -> ()
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping i32
   table.get valid_table
   end_function
 
 table_get_type_mismatch:
   .functype table_get_type_mismatch () -> ()
   f32.const 1.0
 # CHECK: :[[@LINE+1]]:3: error: popped f32, expected i32
   table.get valid_table
   end_function
 
 table_set_expected_expression_operand:
   .functype table_set_expected_expression_operand () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:13: error: expected expression operand
   table.set 1
   end_function
 
 table_set_missing_tabletype:
   .functype table_set_missing_tabletype () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
+# FIXME: Error location should be at beginning ofoperand.
+# CHECK: :[[@LINE+1]]:16: error: symbol foo missing .tabletype
   table.set foo
   end_function
 
 table_set_empty_stack_while_popping_1:
   .functype table_set_empty_stack_while_popping_1 () -> ()
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping externref
   table.set valid_table
   end_function
table_set_type_mismatch_2:
   f32.const 1.0
   ref.null_extern
 # CHECK: :[[@LINE+1]]:3: error: popped f32, expected i32
   table.set valid_table
   end_function
 
 table_fill_expected_expression_operand:
   .functype table_fill_expected_expression_operand () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:14: error: expected expression operand
   table.fill 1
   end_function
 
 table_fill_missing_tabletype:
   .functype table_fill_missing_tabletype () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
+# FIXME: Error location should be at beginning of operand.
+# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .tabletype
   table.fill foo
   end_function
 
 table_fill_empty_stack_while_popping_1:
   .functype table_fill_empty_stack_while_popping_1 () -> ()
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping i32
   table.fill valid_table
   end_function
return_call_indirect_empty_stack_while_popping_2:
   .functype return_call_indirect_empty_stack_while_popping_2 () -> ()
   i32.const 1
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping f32
   return_call_indirect (f32) -> ()
   end_function
 
 call_expected_expression_operand:
   .functype call_expected_expression_operand () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:8: error: expected expression operand
   call 1
   end_function
 
 .functype fn_i32_to_void (i32) -> ()
 
 call_empty_stack_while_popping:
   .functype call_empty_stack_while_popping () -> ()
 # CHECK: [[@LINE+1]]:3: error: empty stack while popping i32

 call_superfluous_value_at_end:
   .functype call_superfluous_value_at_end () -> ()
   call fn_void_to_i32
 # CHECK: :[[@LINE+1]]:3: error: 1 superfluous return values
   end_function
 
 call_missing_functype:
   .functype call_missing_functype () -> ()
-# CHECK: :[[@LINE+1]]:3: error: symbol no_functype missing .functype
+# FIXME: Error location should be at beginning of operand.
+# CHECK: :[[@LINE+1]]:19: error: symbol no_functype missing .functype
   call no_functype
   end_function
 
 return_call_expected_expression_operand:
   .functype return_call_expected_expression_operand () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:15: error: expected expression operand
   return_call 1
   end_function
 
 return_call_empty_stack_while_popping:
   .functype return_call_empty_stack_while_popping () -> ()
 # CHECK: [[@LINE+1]]:3: error: empty stack while popping i32
   return_call fn_i32_to_void
   end_function
 
 return_call_type_mismatch:
   .functype return_call_type_mismatch () -> ()
   f32.const 1.0
 # CHECK: :[[@LINE+1]]:3: error: popped f32, expected i32
   return_call fn_i32_to_void
   end_function
 
 return_call_missing_functype:
   .functype return_call_missing_functype () -> ()
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: symbol no_functype missing .functype
+# FIXME: Error location should be at beginning of operand.
+# CHECK: :[[@LINE+1]]:26: error: symbol no_functype missing .functype
   return_call no_functype
   end_function
 
 catch_expected_expression_operand:
   .functype catch_expected_expression_operand () -> ()
   try
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: expected expression operand
+# CHECK: :[[@LINE+1]]:9: error: expected expression operand
   catch 1
   end_try
   end_function
 
 catch_missing_tagtype:
   .functype catch_missing_tagtype () -> ()
   try
-# FIXME: Error location should be at operand.
-# CHECK: :[[@LINE+1]]:3: error: symbol no_tagtype missing .tagtype
+# FIXME: Error location should be at beginning of operand.
+# CHECK: :[[@LINE+1]]:19: error: symbol no_tagtype missing .tagtype
   catch no_tagtype
   end_try
   end_function
 
 catch_superfluous_value_at_end:
   .functype catch_superfluous_value_at_end () -> ()
   .tagtype tag_i32 i32
   try