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

aheejin · aheejin · commit 3e9881b92b09 · 2023-04-11T02:07:00.000-07:00
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 (`loop`s, `try`s, and `if`s). 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`. Reviewed By: dschuff Differential Revision: https://reviews.llvm.org/D147852
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
@@ -300,11 +300,26 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
     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"))
-      return true;
-    if (Name == "end_block")
-      Unreachable = false;
+             Name == "else" || Name == "end_try" || Name == "catch" ||
+             Name == "catch_all" || Name == "delegate") {
+    if (checkEnd(ErrorLoc,
+                 Name == "else" || Name == "catch" || Name == "catch_all"))
+      return true;
+    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;
@@ -327,19 +342,6 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
     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") {
diff --git a/llvm/test/MC/WebAssembly/type-checker-errors.s b/llvm/test/MC/WebAssembly/type-checker-errors.s
@@ -258,36 +258,36 @@ end_loop_type_mismatch:
   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
@@ -298,6 +298,31 @@ else_type_mismatch:
   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
@@ -313,6 +338,63 @@ end_try_type_mismatch:
   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
@@ -467,7 +549,6 @@ catch_missing_tagtype:
 
 catch_superfluous_value_at_end:
   .functype catch_superfluous_value_at_end () -> ()
-  .tagtype tag_i32 i32
   try
   catch tag_i32
   end_try
@@ -520,3 +601,94 @@ other_insn_test_3:
   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
diff --git a/llvm/test/MC/WebAssembly/type-checker-return.s b/llvm/test/MC/WebAssembly/type-checker-return.s
@@ -27,3 +27,11 @@ return_call_superfluous_return_values:
   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
