[llvm] [WebAssembly] Fix br type checking (PR #108746)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-webassembly

Author: Heejin Ahn (aheejin)

<details>
<summary>Changes</summary>

Currently the type checker does not pop values from the stack when a `br` takes a value to the end of a typed `block`. For example:
```wat
block i32
  ...
  block
    i32.const 3
    br 1
  end_block
  ;; At this point, 'i32.const 3' is still on the stack, because 'br'
  ;; did not pop 'i32.const 3'
  ...
end_block
```

`checkStackTop` only checks the top of the stack and does not actually pop them. This makes `br` actually pop the stack.

Test changes in the `if` part were necessary because those `if`s were not valid but somehow did not error out due to the (incorrectly) remaining values from above. Also the checker does not correctly handle block input values, but that's a separate bug.

A couple more lines of test changes were just drive-by fixes to make `-filetype=obj` work. Currently the produced `.o` files are not valid without them.

---
Full diff: https://github.com/llvm/llvm-project/pull/108746.diff


3 Files Affected:

- (modified) llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp (+4-3) 
- (modified) llvm/test/MC/WebAssembly/basic-assembly.s (+3-1) 
- (modified) llvm/test/MC/WebAssembly/eh-assembly.s (+1-1) 


``````````diff
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
index 9f8f78a5b6adb7..93dc4e786f7670 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
@@ -143,9 +143,10 @@ bool WebAssemblyAsmTypeCheck::checkBr(SMLoc ErrorLoc, size_t Level) {
       BrStack[BrStack.size() - Level - 1];
   if (Expected.size() > Stack.size())
     return typeError(ErrorLoc, "br: insufficient values on the type stack");
-  auto IsStackTopInvalid = checkStackTop(Expected, Stack);
-  if (IsStackTopInvalid)
-    return typeError(ErrorLoc, "br " + IsStackTopInvalid.value());
+  for (auto VT : llvm::reverse(Expected)) {
+    if (popType(ErrorLoc, VT))
+      return true;
+  }
   return false;
 }
 
diff --git a/llvm/test/MC/WebAssembly/basic-assembly.s b/llvm/test/MC/WebAssembly/basic-assembly.s
index db7ccc9759beca..63d28bb41722da 100644
--- a/llvm/test/MC/WebAssembly/basic-assembly.s
+++ b/llvm/test/MC/WebAssembly/basic-assembly.s
@@ -82,11 +82,13 @@ test0:
     i32.const   3
     end_block            # "switch" exit.
     if                   # void
+    i32.const   0
     if          i32
+    i32.const   5
     end_if
+    drop
     else
     end_if
-    drop
     block       void
     i32.const   2
     return
diff --git a/llvm/test/MC/WebAssembly/eh-assembly.s b/llvm/test/MC/WebAssembly/eh-assembly.s
index cd33d198f8d9f5..b5d0b84344aa76 100644
--- a/llvm/test/MC/WebAssembly/eh-assembly.s
+++ b/llvm/test/MC/WebAssembly/eh-assembly.s
@@ -4,10 +4,10 @@
 
   .tagtype  __cpp_exception i32
   .tagtype  __c_longjmp i32
-  .functype  eh_legacy_test () -> ()
   .functype  foo () -> ()
 
 eh_legacy_test:
+  .functype  eh_legacy_test () -> ()
   # try-catch with catch, catch_all, throw, and rethrow
   try
     i32.const 3

``````````

</details>


https://github.com/llvm/llvm-project/pull/108746