From 553f9f72c04a17616f44130617e3e5ecb19f2672 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Sat, 18 Nov 2017 11:58:20 -0800 Subject: [PATCH 1/3] remove unneeded code to handle a br to the return from the function. Now that we use getBlockOrSingleton there, it does that for us anyhow --- src/wasm-binary.h | 1 - src/wasm/wasm-binary.cpp | 22 +++++----------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/wasm-binary.h b/src/wasm-binary.h index bcf6cf45fa8..782dcb9a1e1 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -864,7 +864,6 @@ class WasmBinaryBuilder { }; std::vector breakStack; std::unordered_set breakTargetNames; - bool breaksToReturn; // whether a break is done to the function scope, which is in effect a return std::vector expressionStack; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index aec2efe0e4d..5d752d64c0d 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1204,8 +1204,6 @@ void WasmBinaryWriter::visitDrop(Drop *curr) { // reader -static Name RETURN_BREAK("binaryen|break-to-return"); - void WasmBinaryBuilder::read() { readHeader(); @@ -1625,17 +1623,14 @@ void WasmBinaryBuilder::readFunctions() { if (debug) std::cerr << "processing function: " << i << std::endl; nextLabel = 0; useDebugLocation = false; - breaksToReturn = false; // process body assert(breakTargetNames.size() == 0); assert(breakStack.empty()); - breakStack.emplace_back(RETURN_BREAK, func->result != none); // the break target for the function scope assert(expressionStack.empty()); assert(depth == 0); func->body = getBlockOrSingleton(func->result); assert(depth == 0); - assert(breakStack.size() == 1); - breakStack.pop_back(); + assert(breakStack.size() == 0); assert(breakTargetNames.size() == 0); if (!expressionStack.empty()) { throw ParseException("stack not empty on function exit"); @@ -1643,10 +1638,6 @@ void WasmBinaryBuilder::readFunctions() { if (pos != endOfFunction) { throw ParseException("binary offset at function exit not at expected location"); } - if (breaksToReturn) { - // we broke to return, so we need an outer block to break to - func->body = Builder(wasm).blockifyWithName(func->body, RETURN_BREAK); - } } currFunction = nullptr; functions.push_back(func); @@ -2314,15 +2305,12 @@ void WasmBinaryBuilder::visitLoop(Loop *curr) { WasmBinaryBuilder::BreakTarget WasmBinaryBuilder::getBreakTarget(int32_t offset) { if (debug) std::cerr << "getBreakTarget " << offset << std::endl; + if (breakStack.size() < 1 + size_t(offset)) { + throw ParseException("bad breakindex (low)"); + } size_t index = breakStack.size() - 1 - offset; if (index >= breakStack.size()) { - throw ParseException("bad breakindex"); - } - if (index == 0) { - // trying to access the topmost element means we break out - // to the function scope, doing in effect a return, we'll - // need to create a block for that. - breaksToReturn = true; + throw ParseException("bad breakindex (high)"); } if (debug) std::cerr << "breaktarget "<< breakStack[index].name << " arity " << breakStack[index].arity << std::endl; auto& ret = breakStack[index]; From 98a125189d2454b1599e6327329859a178a42175 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Sat, 18 Nov 2017 12:35:47 -0800 Subject: [PATCH 2/3] test --- test/br_to_exit.wasm | Bin 0 -> 26 bytes test/br_to_exit.wasm.fromBinary | 10 ++++++++++ 2 files changed, 10 insertions(+) create mode 100644 test/br_to_exit.wasm create mode 100644 test/br_to_exit.wasm.fromBinary diff --git a/test/br_to_exit.wasm b/test/br_to_exit.wasm new file mode 100644 index 0000000000000000000000000000000000000000..53bded15a3190fa73d32b901a6553150b2739cf3 GIT binary patch literal 26 hcmZQbEY4+QU|?WmVN76PU}j=u;9_HBVc=ol1^^{V0owoo literal 0 HcmV?d00001 diff --git a/test/br_to_exit.wasm.fromBinary b/test/br_to_exit.wasm.fromBinary new file mode 100644 index 00000000000..2659ab3b4c6 --- /dev/null +++ b/test/br_to_exit.wasm.fromBinary @@ -0,0 +1,10 @@ +(module + (type $0 (func)) + (memory $0 0) + (func $0 (; 0 ;) (type $0) + (block $label$0 + (br $label$0) + ) + ) +) + From ce67500dfab2ffac291a41ba90bae44bc0d45259 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Sun, 19 Nov 2017 09:33:30 -0800 Subject: [PATCH 3/3] fix a fuzz bug of popping from outside a block --- src/wasm/wasm-binary.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 5d752d64c0d..c2652e07497 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2164,6 +2164,9 @@ BinaryConsts::ASTNodes WasmBinaryBuilder::readExpression(Expression*& curr) { } void WasmBinaryBuilder::pushBlockElements(Block* curr, size_t start, size_t end) { + assert(start <= expressionStack.size()); + assert(start <= end); + assert(end <= expressionStack.size()); // the first dropped element may be consumed by code later - it was on the stack first, // and is the only thing left on the stack. there must be just one thing on the stack // since we are at the end of a block context. note that we may need to drop more than @@ -2244,6 +2247,9 @@ Expression* WasmBinaryBuilder::getBlockOrSingleton(WasmType type) { auto start = expressionStack.size(); processExpressions(); size_t end = expressionStack.size(); + if (end < start) { + throw ParseException("block cannot pop from outside"); + } breakStack.pop_back(); auto* block = allocator.alloc(); pushBlockElements(block, start, end);