Skip to content

Commit

Permalink
WASM unreachable code validation is broken
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=265425
rdar://103288466

Reviewed by Keith Miller.

This patch fixes an assertion failure in the unreachable code parser
when the target of a br instruction is a block that was not added into
the control stack.

The code that checks the br target now takes into account the number of
unreachable blocks, if the br instruction is also unreachable. This is
similar to the solution employed by parseDelegateTarget and should
support cases when block, if, try, and loop were not added to the control
stack.

* JSTests/wasm/stress/wasm-unreachable-br-block.js: Added.
(async test):
* Source/JavaScriptCore/wasm/WasmFunctionParser.h:
(JSC::Wasm::FunctionParser<Context>::parseBranchTarget):
(JSC::Wasm::FunctionParser<Context>::parseUnreachableExpression):

Originally-landed-as: 274097.7@webkit-2024.2-embargoed (ab8e4a4470bb). rdar://128090590
Canonical link: https://commits.webkit.org/278882@main
  • Loading branch information
mikhailramalho authored and JonWBedard committed May 16, 2024
1 parent 26dd802 commit 96f558b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
19 changes: 19 additions & 0 deletions JSTests/wasm/stress/wasm-unreachable-br-block.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as assert from '../assert.js';
import { instantiate } from "../wabt-wrapper.js";

let wat = `
(module
(type (;0;) (func))
(func (;0;) (type 0)
unreachable
block ;; label = @1
br 1 (;@0;)
end)
)
`;

async function test() {
await instantiate(wat);
}

assert.asyncTest(test());
14 changes: 10 additions & 4 deletions Source/JavaScriptCore/wasm/WasmFunctionParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class FunctionParser : public Parser<void>, public FunctionParserTypes<typename
PartialResult WARN_UNUSED_RETURN parseIndexForGlobal(uint32_t&);
PartialResult WARN_UNUSED_RETURN parseFunctionIndex(uint32_t&);
PartialResult WARN_UNUSED_RETURN parseExceptionIndex(uint32_t&);
PartialResult WARN_UNUSED_RETURN parseBranchTarget(uint32_t&);
PartialResult WARN_UNUSED_RETURN parseBranchTarget(uint32_t&, uint32_t = 0);
PartialResult WARN_UNUSED_RETURN parseDelegateTarget(uint32_t&, uint32_t);

struct TableInitImmediates {
Expand Down Expand Up @@ -1372,11 +1372,17 @@ auto FunctionParser<Context>::parseExceptionIndex(uint32_t& result) -> PartialRe
}

template<typename Context>
auto FunctionParser<Context>::parseBranchTarget(uint32_t& resultTarget) -> PartialResult
auto FunctionParser<Context>::parseBranchTarget(uint32_t& resultTarget, uint32_t unreachableBlocks) -> PartialResult
{
uint32_t target;
WASM_PARSER_FAIL_IF(!parseVarUInt32(target), "can't get br / br_if's target"_s);
WASM_PARSER_FAIL_IF(target >= m_controlStack.size(), "br / br_if's target "_s, target, " exceeds control stack size "_s, m_controlStack.size());

auto controlStackSize = m_controlStack.size();
// Take into account the unreachable blocks in the control stack that were not added because they were unrechable
if (unreachableBlocks)
controlStackSize += (unreachableBlocks - 1);
WASM_PARSER_FAIL_IF(target >= controlStackSize, "br / br_if's target "_s, target, " exceeds control stack size "_s, controlStackSize);

resultTarget = target;
return { };
}
Expand Down Expand Up @@ -3652,7 +3658,7 @@ auto FunctionParser<Context>::parseUnreachableExpression() -> PartialResult
case Br:
case BrIf: {
uint32_t target;
WASM_FAIL_IF_HELPER_FAILS(parseBranchTarget(target));
WASM_FAIL_IF_HELPER_FAILS(parseBranchTarget(target, m_unreachableBlocks));
return { };
}

Expand Down

0 comments on commit 96f558b

Please sign in to comment.