Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebAssembly] validate table.grow correctly #80437

Merged

Conversation

HerrCai0907
Copy link
Contributor

@HerrCai0907 HerrCai0907 commented Feb 2, 2024

This PR add support in wasm asm type checker to implement checker of table.grow
Fixes: #79966.

This PR add support in wasm asm type checker to implement checker of `table.grow`
@llvmbot llvmbot added backend:WebAssembly mc Machine (object) code labels Feb 2, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 2, 2024

@llvm/pr-subscribers-backend-webassembly

@llvm/pr-subscribers-mc

Author: Congcong Cai (HerrCai0907)

Changes

This PR add support in wasm asm type checker to implement checker of table.grow


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

3 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp (+11-1)
  • (modified) llvm/test/MC/WebAssembly/tables.s (+4-11)
  • (modified) llvm/test/MC/WebAssembly/type-checker-errors.s (+20)
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
index 69466667e45a4..458972592f144 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
@@ -288,6 +288,16 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
       return true;
     if (popType(ErrorLoc, wasm::ValType::I32))
       return true;
+  } else if (Name == "table.size") {
+    if (getTable(Operands[1]->getStartLoc(), Inst, Type))
+      return true;
+    Stack.push_back(wasm::ValType::I32);
+  } else if (Name == "table.grow") {
+    if (getTable(Operands[1]->getStartLoc(), Inst, Type))
+      return true;
+    if (popType(ErrorLoc, wasm::ValType::I32))
+      return true;
+    Stack.push_back(wasm::ValType::I32);
   } else if (Name == "table.fill") {
     if (getTable(Operands[1]->getStartLoc(), Inst, Type))
       return true;
@@ -401,7 +411,7 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
     for (unsigned I = II.getNumOperands(); I > II.getNumDefs(); I--) {
       const auto &Op = II.operands()[I - 1];
       if (Op.OperandType == MCOI::OPERAND_REGISTER) {
-        auto VT = WebAssembly::regClassToValType(Op.RegClass);
+        wasm::ValType VT = WebAssembly::regClassToValType(Op.RegClass);
         if (popType(ErrorLoc, VT))
           return true;
       }
diff --git a/llvm/test/MC/WebAssembly/tables.s b/llvm/test/MC/WebAssembly/tables.s
index 8239432eed88d..0ead2a4366527 100644
--- a/llvm/test/MC/WebAssembly/tables.s
+++ b/llvm/test/MC/WebAssembly/tables.s
@@ -71,8 +71,6 @@ table_set:
 
 #      CHECK: table_grow:
 # CHECK-NEXT:	.functype	table_grow (i32) -> (i32)
-# CHECK-NEXT:	i32.const	0
-# CHECK-NEXT:	table.get	foo
 # CHECK-NEXT:	local.get	0
 #      CHECK:	table.grow	foo
 # CHECK-NEXT:	local.get	0
@@ -80,8 +78,6 @@ table_set:
 # CHECK-NEXT:	end_function
 table_grow:
     .functype table_grow (i32) -> (i32)
-    i32.const 0
-    table.get foo
     local.get 0
 
     # ENC: table.grow	foo                     # encoding: [0xfc,0x0f,0x80'A',0x80'A',0x80'A',0x80'A',A]
@@ -149,16 +145,13 @@ table_fill:
 # BIN-NEXT:        Offset:          0x2D
 # BIN-NEXT:      - Type:            R_WASM_TABLE_NUMBER_LEB
 # BIN-NEXT:        Index:           0
-# BIN-NEXT:        Offset:          0x38
-# BIN-NEXT:      - Type:            R_WASM_TABLE_NUMBER_LEB
-# BIN-NEXT:        Index:           0
-# BIN-NEXT:        Offset:          0x41
+# BIN-NEXT:        Offset:          0x39
 # BIN-NEXT:      - Type:            R_WASM_TABLE_NUMBER_LEB
 # BIN-NEXT:        Index:           2
-# BIN-NEXT:        Offset:          0x51
+# BIN-NEXT:        Offset:          0x49
 # BIN-NEXT:      - Type:            R_WASM_TABLE_NUMBER_LEB
 # BIN-NEXT:        Index:           2
-# BIN-NEXT:        Offset:          0x5A
+# BIN-NEXT:        Offset:          0x52
 # BIN-NEXT:    Functions:
 # BIN-NEXT:      - Index:           0
 # BIN-NEXT:        Locals:          []
@@ -171,7 +164,7 @@ table_fill:
 # BIN-NEXT:        Body:            200020012680808080000B
 # BIN-NEXT:      - Index:           3
 # BIN-NEXT:        Locals:          []
-# BIN-NEXT:        Body:            41002580808080002000FC0F808080800020006A0B
+# BIN-NEXT:        Body:            2000FC0F808080800020006A0B
 # BIN-NEXT:      - Index:           4
 # BIN-NEXT:        Locals:          []
 # BIN-NEXT:        Body:            200041002582808080002001FC1182808080000B
diff --git a/llvm/test/MC/WebAssembly/type-checker-errors.s b/llvm/test/MC/WebAssembly/type-checker-errors.s
index 1501bedec393c..aab69201d4a7e 100644
--- a/llvm/test/MC/WebAssembly/type-checker-errors.s
+++ b/llvm/test/MC/WebAssembly/type-checker-errors.s
@@ -215,6 +215,26 @@ table_fill_type_mismatch_3:
   table.fill valid_table
   end_function
 
+table_grow_non_exist_table:
+  .functype table_grow_non_exist_table () -> (i32)
+  i32.const 0
+# CHECK: [[@LINE+1]]:14: error: symbol invalid_table missing .tabletype
+  table.grow invalid_table
+  end_function
+
+table_grow_wrong_parameter:
+  .functype table_grow_non_exist_table () -> (i32)
+# CHECK: [[@LINE+1]]:3: error: empty stack while popping i32
+  table.grow valid_table
+  end_function
+
+table_grow_wrong_result:
+  .functype table_grow_non_exist_table () -> (f32)
+  i32.const 0
+  table.grow valid_table
+# CHECK: [[@LINE+1]]:3: error: popped i32, expected f32
+  end_function
+
 drop_empty_stack_while_popping:
   .functype drop_empty_stack_while_popping () -> ()
 # CHECK: :[[@LINE+1]]:3: error: empty stack while popping value

@@ -401,7 +411,7 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
for (unsigned I = II.getNumOperands(); I > II.getNumDefs(); I--) {
const auto &Op = II.operands()[I - 1];
if (Op.OperandType == MCOI::OPERAND_REGISTER) {
auto VT = WebAssembly::regClassToValType(Op.RegClass);
wasm::ValType VT = WebAssembly::regClassToValType(Op.RegClass);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? Maybe revert his line if its not needed?

return true;
if (popType(ErrorLoc, wasm::ValType::I32))
return true;
Stack.push_back(wasm::ValType::I32);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is pretty broken.

@HerrCai0907 HerrCai0907 merged commit 78b4e7c into main Feb 3, 2024
3 of 4 checks passed
@HerrCai0907 HerrCai0907 deleted the users/ccc/support-validate-wasm-table-instructions branch February 3, 2024 06:12
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
This PR add support in wasm asm type checker to implement checker of
`table.grow`
Fixes: llvm#79966.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:WebAssembly mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wasm assembly type checker throws incorrect error
3 participants