[WebAssembly] Handle block-like structures consistently in type checker

[aheejin]

Summary

We disable type check in unreachable code, but when the unreachable code
is enclosed within a block-like structure, the block as a whole has a
valid type and we should continue type checking after the block. But it
looks we currently only do that for blocks and not other block-like
structures (loops, trys, and ifs). Also unreachable code within
if's true body shouldn't disable type checking in else body, and
that in try body shouldn't disable type checking in catch/catch_all
body.

This also causes the values/types on the stack to be correctly checked
when encounterint catch, catch_all, and delegate.

[aheejin]

Add one more test

[aheejin]

Test name change

[aheejin]

More

[aheejin]

Fix

[aheejin]

delete newline

[aheejin]

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

309–322

This code is just moved from below

[aheejin]

More

[aheejin]

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

284

This (and another FIXME end_if_type_mismatch_2) is not done even with this patch due to another bug, which I'm planning to fix in another CL.

I renamed these to tests to else_*** to end_if_*** so that else_*** means checking types when we encounter else, and end_if_*** means checking types when we encounter end_if.

[aheejin]

Handle delegate too

[aheejin]

Add one more test

[dschuff]

These are nice improvements!

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

if (Name == "local.get") {
     if (popType(ErrorLoc, wasm::ValType::I32))
       return true;
     if (popType(ErrorLoc, Type))
       return true;
   } else if (Name == "drop") {
     if (popType(ErrorLoc, {}))
       return true;
   } else if (Name == "end_block" || Name == "end_loop" || Name == "end_if" ||
-             Name == "else" || Name == "end_try") {
-    if (checkEnd(ErrorLoc, Name == "else"))
+             Name == "else" || Name == "end_try" || Name == "catch" ||
+             Name == "catch_all" || Name == "delegate") {
+    if (checkEnd(ErrorLoc,
+                 Name == "else" || Name == "catch" || Name == "catch_all"))
       return true;
-    if (Name == "end_block")
-      Unreachable = false;
+    if (Name == "catch") {
+      const MCSymbolRefExpr *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(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 == "return") {
     if (endOfFunction(ErrorLoc))
       return true;
   } 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(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(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(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(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 if (Name == "ref.is_null") {
     if (popRefType(ErrorLoc))
       return true;
     Stack.push_back(wasm::ValType::I32);
   } else {
     // The current instruction is a stack instruction which doesn't have

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

 end_loop_type_mismatch:
   .functype end_loop_type_mismatch () -> ()
   loop f32
   i32.const 1
 # CHECK: :[[@LINE+1]]:3: error: end got i32, expected f32
   end_loop
   end_function
 
-end_if_insufficient_values_on_stack:
-  .functype end_if_insufficient_values_on_stack () -> ()
+end_if_insufficient_values_on_stack_1:
+  .functype end_if_insufficient_values_on_stack_1 () -> ()
   i32.const 1
   if i32
 # CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
   end_if
   end_function
 
-end_if_type_mismatch:
-  .functype end_if_type_mismatch () -> ()
+end_if_type_mismatch_1:
+  .functype end_if_type_mismatch_1 () -> ()
   i32.const 1
   if f32
   i32.const 1
 # CHECK: :[[@LINE+1]]:3: error: end got i32, expected f32
   end_if
   end_function
 
-else_insufficient_values_on_stack:
-  .functype else_insufficient_values_on_stack () -> ()
+end_if_insufficient_values_on_stack_2:
+  .functype end_if_insufficient_values_on_stack_2 () -> ()
   i32.const 1
   if i32
   i32.const 2
   else
 # FIXME: Should complain about insufficient values on the stack.
   end_if
-# FIXME: Should complain about superflous value on the stack.
+  drop
   end_function
 
-else_type_mismatch:
-  .functype else_type_mismatch () -> ()
+end_if_type_mismatch_2:
+  .functype end_if_type_mismatch_2 () -> ()
   i32.const 1
   if i32
   i32.const 2
   else
 # FIXME: Should complain about a type mismatch.
   f32.const 3.0
   end_if
   drop
   end_function
 
+else_insufficient_values_on_stack:
+  .functype else_insufficient_values_on_stack () -> ()
+  i32.const 1
+  if i32
+# CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
+  else
+  i32.const 0
+  end_if
+  drop
+  end_function
+
+else_type_mismatch:
+  .functype else_type_mismatch () -> ()
+  i32.const 1
+  if i32
+  f32.const 0.0
+# CHECK: :[[@LINE+1]]:3: error: popped f32, expected i32
+  else
+  i32.const 0
+  end_if
+  drop
+  end_function
+
+.tagtype tag_i32 i32
+
 end_try_insufficient_values_on_stack:
   .functype end_try_insufficient_values_on_stack () -> ()
   try i32
 # CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
   end_try
   end_function
 
 end_try_type_mismatch:
   .functype end_try_type_mismatch () -> ()
   try i32
   f32.const 1.0
 # CHECK: :[[@LINE+1]]:3: error: end got f32, expected i32
   end_try
   end_function
 
+catch_insufficient_values_on_stack:
+  .functype catch_insufficient_values_on_stack () -> ()
+  try i32
+# CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
+  catch tag_i32
+  end_try
+  drop
+  end_function
+
+catch_type_mismatch:
+  .functype catch_type_mismatch () -> ()
+  try i32
+  f32.const 1.0
+# CHECK: :[[@LINE+1]]:3: error: popped f32, expected i32
+  catch tag_i32
+  end_try
+  drop
+  end_function
+
+catch_all_insufficient_values_on_stack:
+  .functype catch_all_insufficient_values_on_stack () -> ()
+  try i32
+# CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
+  catch_all
+  i32.const 0
+  end_try
+  drop
+  end_function
+
+catch_all_type_mismatch:
+  .functype catch_all_type_mismatch () -> ()
+  try i32
+  f32.const 1.0
+# CHECK: :[[@LINE+1]]:3: error: popped f32, expected i32
+  catch_all
+  i32.const 0
+  end_try
+  drop
+  end_function
+
+delegate_insufficient_values_on_stack:
+  .functype delegate_insufficient_values_on_stack () -> ()
+  try i32
+# CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
+  delegate 0
+  drop
+  end_function
+
+delegate_type_mismatch:
+  .functype delegate_type_mismatch () -> ()
+  try i32
+  f32.const 1.0
+# CHECK: :[[@LINE+1]]:3: error: end got f32, expected i32
+  delegate 0
+  drop
+  end_function
+
 end_function_empty_stack_while_popping:
   .functype end_function_empty_stack_while_popping () -> (i32)
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping i32
   end_function
 
 end_function_type_mismatch:
   .functype end_function_type_mismatch () -> (f32)
   i32.const 1
catch_missing_tagtype:
   try
 # CHECK: :[[@LINE+1]]:9: 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
   catch tag_i32
   end_try
 # FIXME: Superfluous value should be caught at end_try?
 # CHECK: :[[@LINE+1]]:3: error: 1 superfluous return values
   end_function
 
 ref_is_null_empty_stack_while_popping:

 
 other_insn_test_3:
   .functype other_insn_test_3 () -> ()
   f32.const 1.0
   f32.const 2.0
   f32.add
 # CHECK: :[[@LINE+1]]:3: error: 1 superfluous return values
   end_function
+
+# Unreachable code within 'block' does not affect type checking after
+# 'end_block'
+check_after_unreachable_within_block:
+  .functype check_after_unreachable_within_block () -> ()
+  block
+  unreachable
+  end_block
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_function
+
+# Unreachable code within 'loop' does not affect type checking after 'end_loop'
+check_after_unreachable_within_loop:
+  .functype check_after_unreachable_within_loop () -> ()
+  loop
+  unreachable
+  end_loop
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_function
+
+# Unreachable code within 'if' does not affect type checking after 'end_if'
+check_after_unreachable_within_if_1:
+  .functype check_after_unreachable_within_if_1 () -> ()
+  i32.const 0
+  if
+  unreachable
+  else
+  unreachable
+  end_if
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_function
+
+# Unreachable code within 'if' does not affect type checking after 'else'
+check_after_unreachable_within_if_2:
+  .functype check_after_unreachable_within_if_2 () -> ()
+  i32.const 0
+  if
+  unreachable
+  else
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_if
+  end_function
+
+# Unreachable code within 'try' does not affect type checking after 'end_try'
+check_after_unreachable_within_try_1:
+  .functype check_after_unreachable_within_try_1 () -> ()
+  try
+  unreachable
+  catch_all
+  unreachable
+  end_try
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_function
+
+# Unreachable code within 'try' does not affect type checking after 'catch'
+check_after_unreachable_within_try_2:
+  .functype check_after_unreachable_within_try_2 () -> ()
+  try
+  unreachable
+  catch tag_i32
+  drop
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_try
+  end_function
+
+# Unreachable code within 'try' does not affect type checking after 'catch_all'
+check_after_unreachable_within_try_3:
+  .functype check_after_unreachable_within_try_3 () -> ()
+  try
+  unreachable
+  catch_all
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_try
+  end_function
+
+# Unreachable code within 'try' does not affect type checking after 'delegate'
+check_after_unreachable_within_try_4:
+  .functype check_after_unreachable_within_try_4 () -> ()
+  try
+  unreachable
+  delegate 0
+# CHECK: :[[@LINE+1]]:3: error: empty stack while popping value
+  drop
+  end_function

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

 .functype fn_void_to_void () -> ()
 
 return_call_superfluous_return_values:
   .functype return_call_superfluous_return_values () -> ()
   f32.const 1.0
   i32.const 2
   return_call fn_void_to_void
   end_function
+
+# Unreachable code is stack-polymorphic, meaning its input and return types can
+# be anything. So the 'drop' after it doesn't cause an error.
+no_check_after_unreachable:
+  .functype no_check_after_unreachable () -> ()
+  unreachable
+  drop
+  end_function