[llvm] 8c0798f - [WebAssembly] Fix type index block type handling in type checker

[Heejin Ahn via llvm-commits]

Author: Heejin Ahn
Date: 2023-04-11T02:04:56-07:00
New Revision: 8c0798f368356e50f59a2598b28e7cac8770ea4c

URL: https://github.com/llvm/llvm-project/commit/8c0798f368356e50f59a2598b28e7cac8770ea4c
DIFF: https://github.com/llvm/llvm-project/commit/8c0798f368356e50f59a2598b28e7cac8770ea4c.diff

LOG: [WebAssembly] Fix type index block type handling in type checker

The current code is
```
      ExpectBlockType = false;
      TC.setLastSig(*Signature.get());
      if (ExpectBlockType)
        NestingStack.back().Sig = *Signature.get();
```

Because of the first line, the third line's `if (ExpectBlockType)` is
always false and we don't get to update `NestingStack.back().Sig`. This
results in not correctly erroring out when the types of remaining values
on the stack do not match the block type if the block type is written in
the form of a function type. We should set `ExpectBlockType` to false
after the `if`.

Reviewed By: sbc100

Differential Revision: https://reviews.llvm.org/D147837

Added: 
    

Modified: 
    llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
    llvm/test/MC/WebAssembly/type-checker-errors.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index ded5abd3fe3c..eeffc5ac2c69 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -637,10 +637,10 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       if (parseSignature(Signature.get()))
         return true;
       // Got signature as block type, don't need more
-      ExpectBlockType = false;
       TC.setLastSig(*Signature.get());
       if (ExpectBlockType)
         NestingStack.back().Sig = *Signature.get();
+      ExpectBlockType = false;
       auto &Ctx = getContext();
       // The "true" here will cause this to be a nameless symbol.
       MCSymbol *Sym = Ctx.createTempSymbol("typeindex", true);

diff  --git a/llvm/test/MC/WebAssembly/type-checker-errors.s b/llvm/test/MC/WebAssembly/type-checker-errors.s
index dcb9dc16133b..4b75cd501281 100644
--- a/llvm/test/MC/WebAssembly/type-checker-errors.s
+++ b/llvm/test/MC/WebAssembly/type-checker-errors.s
@@ -221,13 +221,20 @@ drop_empty_stack_while_popping:
   drop
   end_function
 
-end_block_insufficient_values_on_stack:
-  .functype end_block_insufficient_values_on_stack () -> ()
+end_block_insufficient_values_on_stack_1:
+  .functype end_block_insufficient_values_on_stack_1 () -> ()
   block i32
 # CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
   end_block
   end_function
 
+end_block_insufficient_values_on_stack_2:
+  .functype end_block_insufficient_values_on_stack_2 () -> ()
+  block () -> (i32)
+# CHECK: :[[@LINE+1]]:3: error: end: insufficient values on the type stack
+  end_block
+  end_function
+
 end_block_type_mismatch:
   .functype end_block_type_mismatch () -> ()
   block i32