Skip to content

Preserve unreachable struct.set of non-nullable locals #8844

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.

Sign up for GitHub

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

Merged
tlively merged 3 commits into from
Jun 17, 2026
Merged

Preserve unreachable struct.set of non-nullable locals #8844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68bd62c
Preserve unreachable struct.set of non-nullable locals
tlively Jun 12, 2026
eb74cb0
fix curr => child
tlively Jun 16, 2026
06f7847
check non-nullability in another place
tlively Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions scripts/fuzz_opt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,18 +229,6 @@ def randomize_fuzz_settings():
else:
LEGALIZE = False

# if GC is enabled then run --dce at the very end, to ensure that our
# binaries validate in other VMs, due to how non-nullable local validation
# and unreachable code interact. see
# https://github.com/WebAssembly/binaryen/pull/5665
# https://github.com/WebAssembly/binaryen/issues/5599
if '--disable-gc' not in FEATURE_OPTS:
GEN_ARGS += ['--dce']

# Add --dce not only when generating the original wasm but to the
# optimizations we use to create any other wasm file.
FUZZ_OPTS += ['--dce']

if CLOSED_WORLD:
GEN_ARGS += [CLOSED_WORLD_FLAG]
# Enclose the world much of the time when fuzzing closed-world, so that
Expand Down Expand Up @@ -450,13 +438,6 @@ def pick_initial_contents():
# --fuzz-exec reports a stack limit using this notation
STACK_LIMIT = '[trap stack limit]'

# V8 reports this error in rare cases due to limitations in our handling of non-
# nullable locals in unreachable code, see
# https://github.com/WebAssembly/binaryen/pull/5665
# https://github.com/WebAssembly/binaryen/issues/5599
# and also see the --dce workaround below that also links to those issues.
V8_UNINITIALIZED_NONDEF_LOCAL = 'uninitialized non-defaultable local'

# JS exceptions are logged as exception thrown: REASON
EXCEPTION_PREFIX = 'exception thrown: '

Expand Down Expand Up @@ -655,8 +636,6 @@ def filter_known_issues(output):
# strings in this list for known issues (to which more need to be
# added as necessary).
HOST_LIMIT_PREFIX,
# see comment above on this constant
V8_UNINITIALIZED_NONDEF_LOCAL,
# V8 does not accept nullable stringviews
# (https://github.com/WebAssembly/binaryen/pull/6574)
'expected (ref stringview_wtf16), got nullref',
Expand Down
7 changes: 7 additions & 0 deletions src/passes/Poppify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ struct Poppifier : BinaryenIRWriter<Poppifier> {
void emitScopeEnd(Expression* curr);
void emitFunctionEnd();
void emitUnreachable();
void emitUnreachableLocalSet(Index i);
void emitDebugLocation(Expression* curr) {}

// Tuple lowering methods
Expand Down Expand Up @@ -317,6 +318,12 @@ void Poppifier::emitUnreachable() {
instrs.push_back(builder.makeUnreachable());
}

void Poppifier::emitUnreachableLocalSet(Index i) {
auto& instrs = scopeStack.back().instrs;
instrs.push_back(builder.makeUnreachable());
instrs.push_back(builder.makeLocalSet(i, builder.makePop(Type::unreachable)));
}

void Poppifier::emitTupleExtract(TupleExtract* curr) {
auto& instrs = scopeStack.back().instrs;
auto types = curr->tuple->type;
Expand Down
5 changes: 4 additions & 1 deletion src/passes/Print.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,10 @@ struct PrintExpressionContents
printLocal(curr->index, currFunction, o);
}
void visitLocalSet(LocalSet* curr) {
if (curr->isTee()) {
// Print unreachable tees as sets. This makes the output valid WebAssembly
// in more cases because it avoids pushing a concrete type (which may not
// be the type required by the next instruction) onto a polymorphic stack.
if (curr->isTee() && curr->type != Type::unreachable) {
printMedium(o, "local.tee ");
} else {
printMedium(o, "local.set ");
Expand Down
92 changes: 73 additions & 19 deletions src/wasm-stack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#define wasm_stack_h

#include "ir/branch-utils.h"
#include "ir/find_all.h"
#include "ir/module-utils.h"
#include "ir/properties.h"
#include "pass.h"
Expand Down Expand Up @@ -123,6 +124,7 @@ class BinaryInstWriter : public OverriddenVisitor<BinaryInstWriter> {
// emit an end at the end of a function
void emitFunctionEnd();
void emitUnreachable();
void emitUnreachableLocalSet(Index index);
void mapLocalsAndEmitHeader();

MappedLocals mappedLocals;
Expand Down Expand Up @@ -243,11 +245,16 @@ class BinaryenIRWriter : public Visitor<BinaryenIRWriter<SubType>> {
void emitScopeEnd(Expression* curr) {
static_cast<SubType*>(this)->emitScopeEnd(curr);
}
void emitFunctionEnd() { static_cast<SubType*>(this)->emitFunctionEnd(); }
void emitUnreachable() { static_cast<SubType*>(this)->emitUnreachable(); }
SubType& self() { return *static_cast<SubType*>(this); }
void emitFunctionEnd() { self().emitFunctionEnd(); }
void emitUnreachable() { self().emitUnreachable(); }
void emitUnreachableLocalSet(Index index) {
self().emitUnreachableLocalSet(index);
}
void emitDebugLocation(Expression* curr) {
static_cast<SubType*>(this)->emitDebugLocation(curr);
}
void emitUnreachableLocalSets(Expression* curr);
void visitPossibleBlockContents(Expression* curr);
};

Expand All @@ -258,6 +265,23 @@ template<typename SubType> void BinaryenIRWriter<SubType>::write() {
emitFunctionEnd();
}

// Normally we do not emit any instructions between a stack-polymorphic (i.e.
// unreachable) instruction and the end of the current control flow structure.
// However, local.sets of non-nullable locals in these unreachable regions can
// still be necessary to make later local.gets valid. We can emit just these
// sets without emitting their values; they will pull whatever type they need
// out of the unreachable. This works no matter what type the current control
// flow structure returns; it will also pull whatever values it needs out of the
// unreachable, and the sets will not push any values to get in the way.
template<typename SubType>
void BinaryenIRWriter<SubType>::emitUnreachableLocalSets(Expression* curr) {
for (auto* set : FindAll<LocalSet>(curr).list) {
if (func->getLocalType(set->index).isNonNullable()) {
emitUnreachableLocalSet(set->index);
}
}
}

// Emits a node in a position that can contain a list of contents, like an if
// arm. This will emit the node, but if it is a block with no name, just emit
// its contents. This is ok to do because a list of contents is ok in the wasm
Expand All @@ -279,10 +303,12 @@ void BinaryenIRWriter<SubType>::visitPossibleBlockContents(Expression* curr) {
}
for (auto* child : block->list) {
visit(child);
// Since this child was unreachable, either this child or one of its
// descendants was a source of unreachability that was actually
// emitted. Subsequent children won't be reachable, so skip them.
if (child->type == Type::unreachable) {
// Since this child was unreachable, either this child or one of its
// descendants was a source of unreachability that was actually
// emitted. Subsequent children won't be reachable, so skip them. We also
// don't have to worry about local.sets here because they will not be
// visible past the end of the surrounding control flow structure anyway.
break;
}
}
Expand All @@ -294,19 +320,29 @@ void BinaryenIRWriter<SubType>::visit(Expression* curr) {
// unreachable instructions that just inherit unreachability from their
// children, since the latter won't be reached. This (together with logic in
// the control flow visitors) also ensures that the final instruction in each
// unreachable block is a source of unreachability, which means we don't need
// to emit an extra `unreachable` before the end of the block to prevent type
// errors.
bool hasUnreachableChild = false;
// unreachable block is a source of unreachability (possibly followed by a
// sequence of local.sets), which means we don't need to emit an extra
// `unreachable` before the end of the block to prevent type errors.
bool sawUnreachable = false;
for (auto* child : ValueChildIterator(curr)) {
if (sawUnreachable) {
emitUnreachableLocalSets(child);
continue;

@kripken kripken Jun 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In addition, even if this was child, this recurses into all sub-children of that child? Is that what we want here?

@tlively tlively Jun 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh yes, this should be child. We do want to walk the entire child tree, though, because there might be important sets arbitrarily far down in it. This is not quadratic because this is a recursive visitor, not a walker, so the child subtrees will only ever be visited this one time.

}
visit(child);
if (child->type == Type::unreachable) {
hasUnreachableChild = true;
break;
// Skip the following children, except for necessary local.sets they
// contain.
sawUnreachable = true;
}
}
if (hasUnreachableChild) {
// `curr` is not reachable, so don't emit it.
if (sawUnreachable) {
// `curr` is not reachable, so don't emit it unless it is a
// potentially-necessary local.set itself.
if (auto* set = curr->dynCast<LocalSet>();
set && func->getLocalType(set->index).isNonNullable()) {
emitUnreachableLocalSet(set->index);
}
return;
}
emitDebugLocation(curr);
Expand All @@ -323,13 +359,17 @@ template<typename SubType>
void BinaryenIRWriter<SubType>::visitBlock(Block* curr) {
auto visitChildren = [this](Block* curr, Index from) {
auto& list = curr->list;
while (from < list.size()) {
auto* child = list[from];
bool sawUnreachable = false;
for (Index i = from; i < list.size(); ++i) {
auto* child = list[i];
if (sawUnreachable) {
emitUnreachableLocalSets(child);
continue;
}
visit(child);
if (child->type == Type::unreachable) {
break;
sawUnreachable = true;
}
++from;
}
};

Expand All @@ -354,6 +394,10 @@ void BinaryenIRWriter<SubType>::visitBlock(Block* curr) {
}

auto afterChildren = [this](Block* curr) {
if (!curr->name) {
// Not emitting a block, so continue on in the parent.
return;
}
emitScopeEnd(curr);
if (curr->type == Type::unreachable) {
// Since this block is unreachable, no instructions will be emitted after
Expand All @@ -376,13 +420,17 @@ void BinaryenIRWriter<SubType>::visitBlock(Block* curr) {
Block* child;
while (!curr->list.empty() && (child = curr->list[0]->dynCast<Block>())) {
parents.push_back(curr);
emit(curr);
if (curr->name) {
emit(curr);
}
curr = child;
emitDebugLocation(curr);
}
// Emit the current block, which does not have a block as a child in the
// first position.
emit(curr);
if (curr->name) {
emit(curr);
}
visitChildren(curr, 0);
afterChildren(curr);
bool childUnreachable = curr->type == Type::unreachable;
Expand All @@ -391,6 +439,9 @@ void BinaryenIRWriter<SubType>::visitBlock(Block* curr) {
auto* parent = parents.back();
parents.pop_back();
if (!childUnreachable) {
// No need to emit unreachable sets here because we will skip everything
// up to the end of the next named block, which would hide their effects
// anyway.
visitChildren(parent, 1);
}
afterChildren(parent);
Expand Down Expand Up @@ -504,6 +555,9 @@ class BinaryenIRToBinaryWriter
writer.emitFunctionEnd();
}
void emitUnreachable() { writer.emitUnreachable(); }
void emitUnreachableLocalSet(Index index) {
writer.emitUnreachableLocalSet(index);
}
void emitDebugLocation(Expression* curr) {
if (sourceMap) {
parent.writeSourceMapLocation(curr, func);
Expand Down
22 changes: 20 additions & 2 deletions src/wasm/wasm-stack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,11 @@ void BinaryInstWriter::visitLocalSet(LocalSet* curr) {
o << static_cast<int8_t>(BinaryConsts::LocalSet)
<< U32LEB(mappedLocals[std::make_pair(curr->index, i)]);
}
if (!curr->isTee()) {
// This is not a tee, so just finish setting the values.
if (!curr->isTee() || curr->type == Type::unreachable) {
// This is not a tee or it is unreachable, so just finish setting the
// values. We emit unreachable sets as sets rather than tees to avoid
// pushing concrete types (which may not be correct for the next
// instruction) onto polymorphic stacks.
o << static_cast<int8_t>(BinaryConsts::LocalSet)
<< U32LEB(mappedLocals[std::make_pair(curr->index, 0)]);
} else if (auto it = extractedGets.find(curr); it != extractedGets.end()) {
Expand Down Expand Up @@ -3196,6 +3199,14 @@ void BinaryInstWriter::emitUnreachable() {
o << static_cast<int8_t>(BinaryConsts::Unreachable);
}

void BinaryInstWriter::emitUnreachableLocalSet(Index index) {
LocalSet set;
set.index = index;
set.type = Type::none;
set.value = nullptr;
visitLocalSet(&set);
}

void BinaryInstWriter::mapLocalsAndEmitHeader() {
assert(func && "BinaryInstWriter: function is not set");
// Map params
Expand Down Expand Up @@ -3583,6 +3594,13 @@ class StackIRGenerator : public BinaryenIRWriter<StackIRGenerator> {
void emitUnreachable() {
stackIR.push_back(makeStackInst(Builder(module).makeUnreachable()));
}
void emitUnreachableLocalSet(Index i) {
Builder builder(module);
auto unreachable = builder.makeUnreachable();
auto set = builder.makeLocalSet(i, unreachable);
emit(unreachable);
emit(set);
}
void emitDebugLocation(Expression* curr) {}

StackIR& getStackIR() { return stackIR; }
Expand Down
20 changes: 7 additions & 13 deletions test/lit/basic/polymorphic_stack.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@
;; CHECK-TEXT-NEXT: (local $y f32)
;; CHECK-TEXT-NEXT: (drop
;; CHECK-TEXT-NEXT: (i64.eqz
;; CHECK-TEXT-NEXT: (local.tee $x
;; CHECK-TEXT-NEXT: (local.set $x
;; CHECK-TEXT-NEXT: (unreachable)
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: (drop
;; CHECK-TEXT-NEXT: (local.tee $y
;; CHECK-TEXT-NEXT: (local.set $y
;; CHECK-TEXT-NEXT: (i64.eqz
;; CHECK-TEXT-NEXT: (unreachable)
;; CHECK-TEXT-NEXT: )
Expand Down Expand Up @@ -161,7 +161,7 @@
;; CHECK-TEXT-NEXT: (if
;; CHECK-TEXT-NEXT: (i32.const 259)
;; CHECK-TEXT-NEXT: (then
;; CHECK-TEXT-NEXT: (local.tee $0
;; CHECK-TEXT-NEXT: (local.set $0
;; CHECK-TEXT-NEXT: (unreachable)
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: )
Expand Down Expand Up @@ -225,11 +225,8 @@
;; CHECK-TEXT-NEXT: )
;; CHECK-TEXT-NEXT: )
;; CHECK-BIN: (func $untaken-break-should-have-value (type $0) (result i32)
;; CHECK-BIN-NEXT: (block
;; CHECK-BIN-NEXT: (drop
;; CHECK-BIN-NEXT: (i32.const 0)
;; CHECK-BIN-NEXT: )
;; CHECK-BIN-NEXT: (unreachable)
;; CHECK-BIN-NEXT: (drop
;; CHECK-BIN-NEXT: (i32.const 0)
;; CHECK-BIN-NEXT: )
;; CHECK-BIN-NEXT: (unreachable)
;; CHECK-BIN-NEXT: )
Expand Down Expand Up @@ -419,11 +416,8 @@
;; CHECK-BIN-NODEBUG-NEXT: )

;; CHECK-BIN-NODEBUG: (func $5 (type $0) (result i32)
;; CHECK-BIN-NODEBUG-NEXT: (block
;; CHECK-BIN-NODEBUG-NEXT: (drop
;; CHECK-BIN-NODEBUG-NEXT: (i32.const 0)
;; CHECK-BIN-NODEBUG-NEXT: )
;; CHECK-BIN-NODEBUG-NEXT: (unreachable)
;; CHECK-BIN-NODEBUG-NEXT: (drop
;; CHECK-BIN-NODEBUG-NEXT: (i32.const 0)
;; CHECK-BIN-NODEBUG-NEXT: )
;; CHECK-BIN-NODEBUG-NEXT: (unreachable)
;; CHECK-BIN-NODEBUG-NEXT: )
Expand Down
2 changes: 1 addition & 1 deletion test/lit/basic/stack_switching_switch.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@

;; CHECK-TEXT: (func $unreachable (type $2) (param $k (ref null $ct)) (result i32)
;; CHECK-TEXT-NEXT: (return
;; CHECK-TEXT-NEXT: (local.tee $k
;; CHECK-TEXT-NEXT: (local.set $k
;; CHECK-TEXT-NEXT: (block ;; (replaces unreachable StackSwitch we can't emit)
;; CHECK-TEXT-NEXT: (drop
;; CHECK-TEXT-NEXT: (i32.const 42)
Expand Down
Loading
Loading