From 9594dc24c7c825e81cc5c44aee38dc4d133b951c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 19 Sep 2023 15:14:24 -0700 Subject: [PATCH 1/4] test --- src/wasm/wasm-binary.cpp | 13 +++++++++++++ test/lit/binary/bad-multivalue-block.test | 16 ++++++++++++++++ test/lit/binary/bad-multivalue-block.test.wasm | Bin 0 -> 34 bytes 3 files changed, 29 insertions(+) create mode 100644 test/lit/binary/bad-multivalue-block.test create mode 100644 test/lit/binary/bad-multivalue-block.test.wasm diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 0c78442a7ee..6a1dd1e6d96 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -4236,6 +4236,15 @@ void WasmBinaryReader::pushBlockElements(Block* curr, Type type, size_t start) { } } +static void checkControlFlowType(Type type) { + if (type.isSignature()) { + auto sig = type.getSignature(); + if (type.params != Type::none) { + throwError("control flow inputs are not supported yet"); + } + } +} + void WasmBinaryReader::visitBlock(Block* curr) { BYN_TRACE("zz node: Block\n"); startControlFlow(curr); @@ -4244,6 +4253,7 @@ void WasmBinaryReader::visitBlock(Block* curr) { std::vector stack; while (1) { curr->type = getType(); + checkControlFlowType(curr->type); curr->name = getNextLabel(); breakStack.push_back({curr->name, curr->type}); stack.push_back(curr); @@ -4321,6 +4331,7 @@ void WasmBinaryReader::visitIf(If* curr) { BYN_TRACE("zz node: If\n"); startControlFlow(curr); curr->type = getType(); + checkControlFlowType(curr->type); curr->condition = popNonVoidExpression(); curr->ifTrue = getBlockOrSingleton(curr->type); if (lastSeparator == BinaryConsts::Else) { @@ -4336,6 +4347,7 @@ void WasmBinaryReader::visitLoop(Loop* curr) { BYN_TRACE("zz node: Loop\n"); startControlFlow(curr); curr->type = getType(); + checkControlFlowType(curr->type); curr->name = getNextLabel(); breakStack.push_back({curr->name, Type::none}); // find the expressions in the block, and create the body @@ -6783,6 +6795,7 @@ void WasmBinaryReader::visitTryOrTryInBlock(Expression*& out) { // within each try-body and catch-body, and let branches target those inner // blocks instead. curr->type = getType(); + checkControlFlowType(curr->type); curr->body = getBlockOrSingleton(curr->type); Builder builder(wasm); diff --git a/test/lit/binary/bad-multivalue-block.test b/test/lit/binary/bad-multivalue-block.test new file mode 100644 index 00000000000..8b100fe89ed --- /dev/null +++ b/test/lit/binary/bad-multivalue-block.test @@ -0,0 +1,16 @@ +;; Test that we error properly on a block with a bad multivalue (inputs). + +;; File contents: +;; +;; (module +;; (func $test +;; i32.const 0 +;; (block (param i32) +;; drop +;; ) +;; ) +;; ) + +;; RUN: not wasm-opt -all %s.wasm 2>&1 | filecheck %s + +;; CHECK: control flow inputs are not supported yet diff --git a/test/lit/binary/bad-multivalue-block.test.wasm b/test/lit/binary/bad-multivalue-block.test.wasm new file mode 100644 index 0000000000000000000000000000000000000000..e44b9033f2001cdaa291bb87f198d97958e2ac00 GIT binary patch literal 34 pcmZQbEY4+QU|?Y6U`k+MNMNjIU}j=u;NoKBU~pt$VwB?M1^`B110?_e literal 0 HcmV?d00001 From a26f7413efd22a7b0be4ee1ace494758f0845909 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 19 Sep 2023 15:19:57 -0700 Subject: [PATCH 2/4] work --- src/wasm-binary.h | 5 +++ src/wasm/wasm-binary.cpp | 41 +++++++++++--------- test/lit/binary/bad-multivalue-if.test | 16 ++++++++ test/lit/binary/bad-multivalue-if.test.wasm | Bin 0 -> 38 bytes 4 files changed, 44 insertions(+), 18 deletions(-) create mode 100644 test/lit/binary/bad-multivalue-if.test create mode 100644 test/lit/binary/bad-multivalue-if.test.wasm diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 72b61223fcd..e87b33e2880 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1615,6 +1615,11 @@ class WasmBinaryReader { HeapType getIndexedHeapType(); Type getConcreteType(); + + // Get the type of a control flow construct. This is like getType() but adds + // validation. + Type getControlFlowType(); + Name getInlineString(); void verifyInt8(int8_t x); void verifyInt16(int16_t x); diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 6a1dd1e6d96..85ce85a23e0 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2088,7 +2088,25 @@ HeapType WasmBinaryReader::getIndexedHeapType() { Type WasmBinaryReader::getConcreteType() { auto type = getType(); if (!type.isConcrete()) { - throw ParseException("non-concrete type when one expected"); + throwError("non-concrete type when one expected"); + } + return type; +} + +Type WasmBinaryReader::getControlFlowType() { + auto type = getType(); + if (!type.isRef()) { + return type; + } + + auto heapType = type.getHeapType(); + if (!heapType.isSignature()) { + return type; + } + + auto sig = heapType.getSignature(); + if (sig.params != Type::none) { + throwError("control flow inputs are not supported yet"); } return type; } @@ -4236,15 +4254,6 @@ void WasmBinaryReader::pushBlockElements(Block* curr, Type type, size_t start) { } } -static void checkControlFlowType(Type type) { - if (type.isSignature()) { - auto sig = type.getSignature(); - if (type.params != Type::none) { - throwError("control flow inputs are not supported yet"); - } - } -} - void WasmBinaryReader::visitBlock(Block* curr) { BYN_TRACE("zz node: Block\n"); startControlFlow(curr); @@ -4252,8 +4261,7 @@ void WasmBinaryReader::visitBlock(Block* curr) { // that is a common pattern that can be very highly nested. std::vector stack; while (1) { - curr->type = getType(); - checkControlFlowType(curr->type); + curr->type = getControlFlowType(); curr->name = getNextLabel(); breakStack.push_back({curr->name, curr->type}); stack.push_back(curr); @@ -4330,8 +4338,7 @@ Expression* WasmBinaryReader::getBlockOrSingleton(Type type) { void WasmBinaryReader::visitIf(If* curr) { BYN_TRACE("zz node: If\n"); startControlFlow(curr); - curr->type = getType(); - checkControlFlowType(curr->type); + curr->type = getControlFlowType(); curr->condition = popNonVoidExpression(); curr->ifTrue = getBlockOrSingleton(curr->type); if (lastSeparator == BinaryConsts::Else) { @@ -4346,8 +4353,7 @@ void WasmBinaryReader::visitIf(If* curr) { void WasmBinaryReader::visitLoop(Loop* curr) { BYN_TRACE("zz node: Loop\n"); startControlFlow(curr); - curr->type = getType(); - checkControlFlowType(curr->type); + curr->type = getControlFlowType(); curr->name = getNextLabel(); breakStack.push_back({curr->name, Type::none}); // find the expressions in the block, and create the body @@ -6794,8 +6800,7 @@ void WasmBinaryReader::visitTryOrTryInBlock(Expression*& out) { // For simplicity of implementation, like if scopes, we create a hidden block // within each try-body and catch-body, and let branches target those inner // blocks instead. - curr->type = getType(); - checkControlFlowType(curr->type); + curr->type = getControlFlowType(); curr->body = getBlockOrSingleton(curr->type); Builder builder(wasm); diff --git a/test/lit/binary/bad-multivalue-if.test b/test/lit/binary/bad-multivalue-if.test new file mode 100644 index 00000000000..d86001f6feb --- /dev/null +++ b/test/lit/binary/bad-multivalue-if.test @@ -0,0 +1,16 @@ +;; Test that we error properly on an if with a bad multivalue (inputs). + +;; File contents: +;; +;; (module +;; (func $test +;; i32.const 0 +;; (block (param i32) +;; drop +;; ) +;; ) +;; ) + +;; RUN: not wasm-opt -all %s.wasm 2>&1 | filecheck %s + +;; CHECK: control flow inputs are not supported yet diff --git a/test/lit/binary/bad-multivalue-if.test.wasm b/test/lit/binary/bad-multivalue-if.test.wasm new file mode 100644 index 0000000000000000000000000000000000000000..baddfec4ed77c13dfb6d31fde432c4891f6a7dfa GIT binary patch literal 38 tcmZQbEY4+QU|?Y6U`k+MNMNjIU}j=u;NoNCVQ^${WMpBKVwK|N1^`u(1CRg! literal 0 HcmV?d00001 From 8cc47585cac0e8aa0a2dac3b24a0365a7c8683f1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 19 Sep 2023 15:30:54 -0700 Subject: [PATCH 3/4] fix --- src/wasm-binary.h | 5 ----- src/wasm/wasm-binary.cpp | 32 +++++++++----------------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/src/wasm-binary.h b/src/wasm-binary.h index e87b33e2880..72b61223fcd 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1615,11 +1615,6 @@ class WasmBinaryReader { HeapType getIndexedHeapType(); Type getConcreteType(); - - // Get the type of a control flow construct. This is like getType() but adds - // validation. - Type getControlFlowType(); - Name getInlineString(); void verifyInt8(int8_t x); void verifyInt16(int16_t x); diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 85ce85a23e0..e9adc5e0ed0 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2037,7 +2037,11 @@ Type WasmBinaryReader::getType(int initial) { // Single value types are negative; signature indices are non-negative if (initial >= 0) { // TODO: Handle block input types properly. - return getSignatureByTypeIndex(initial).results; + auto sig = getSignatureByTypeIndex(initial); + if (sig.params != Type::none) { + throwError("control flow inputs are not supported yet"); + } + return sig.results; } Type type; if (getBasicType(initial, type)) { @@ -2093,24 +2097,6 @@ Type WasmBinaryReader::getConcreteType() { return type; } -Type WasmBinaryReader::getControlFlowType() { - auto type = getType(); - if (!type.isRef()) { - return type; - } - - auto heapType = type.getHeapType(); - if (!heapType.isSignature()) { - return type; - } - - auto sig = heapType.getSignature(); - if (sig.params != Type::none) { - throwError("control flow inputs are not supported yet"); - } - return type; -} - Name WasmBinaryReader::getInlineString() { BYN_TRACE("<==\n"); auto len = getU32LEB(); @@ -4261,7 +4247,7 @@ void WasmBinaryReader::visitBlock(Block* curr) { // that is a common pattern that can be very highly nested. std::vector stack; while (1) { - curr->type = getControlFlowType(); + curr->type = getType(); curr->name = getNextLabel(); breakStack.push_back({curr->name, curr->type}); stack.push_back(curr); @@ -4338,7 +4324,7 @@ Expression* WasmBinaryReader::getBlockOrSingleton(Type type) { void WasmBinaryReader::visitIf(If* curr) { BYN_TRACE("zz node: If\n"); startControlFlow(curr); - curr->type = getControlFlowType(); + curr->type = getType(); curr->condition = popNonVoidExpression(); curr->ifTrue = getBlockOrSingleton(curr->type); if (lastSeparator == BinaryConsts::Else) { @@ -4353,7 +4339,7 @@ void WasmBinaryReader::visitIf(If* curr) { void WasmBinaryReader::visitLoop(Loop* curr) { BYN_TRACE("zz node: Loop\n"); startControlFlow(curr); - curr->type = getControlFlowType(); + curr->type = getType(); curr->name = getNextLabel(); breakStack.push_back({curr->name, Type::none}); // find the expressions in the block, and create the body @@ -6800,7 +6786,7 @@ void WasmBinaryReader::visitTryOrTryInBlock(Expression*& out) { // For simplicity of implementation, like if scopes, we create a hidden block // within each try-body and catch-body, and let branches target those inner // blocks instead. - curr->type = getControlFlowType(); + curr->type = getType(); curr->body = getBlockOrSingleton(curr->type); Builder builder(wasm); From 0bf003dbe59bd9279eaf367bb432eeeb2b1c0cae Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 19 Sep 2023 15:33:06 -0700 Subject: [PATCH 4/4] test --- test/lit/binary/bad-multivalue-if.test | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/lit/binary/bad-multivalue-if.test b/test/lit/binary/bad-multivalue-if.test index d86001f6feb..8fe20601285 100644 --- a/test/lit/binary/bad-multivalue-if.test +++ b/test/lit/binary/bad-multivalue-if.test @@ -5,8 +5,14 @@ ;; (module ;; (func $test ;; i32.const 0 -;; (block (param i32) -;; drop +;; i32.const 1 +;; (if (param i32) +;; (then +;; drop +;; ) +;; (else +;; drop +;; ) ;; ) ;; ) ;; )