Skip to content

Fix and improve unreachable parsing #8617

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 2 commits into from
Apr 20, 2026
Merged

Fix and improve unreachable parsing #8617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1744c8e
[NFC] Simplify printing of unreachable replacements
tlively Apr 17, 2026
c0d13e2
Fix and improve unreachable parsing
tlively Apr 17, 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
23 changes: 21 additions & 2 deletions src/ir/properties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,8 +521,8 @@ inline MemoryOrder getMemoryOrder(Expression* curr) {
}

// Whether this instruction will be unwritable in the text and binary formats
// because it requires a type index immediate giving the type of a child that
// has unreachable or null type, and therefore does not have a type index.
// because it requires a type index immediate computed from an expression with
// unreachable or null type, and therefore no type index.
inline bool hasUnwritableTypeImmediate(Expression* curr) {
#define DELEGATE_ID curr->_id

Expand Down Expand Up @@ -552,6 +552,25 @@ inline bool hasUnwritableTypeImmediate(Expression* curr) {

#include "wasm-delegations-fields.def"

if (curr->type == Type::unreachable) {
if (curr->is<StructNew>() || curr->is<ArrayNew>() ||
curr->is<ArrayNewData>() || curr->is<ArrayNewElem>() ||
curr->is<ArrayNewFixed>() || curr->is<ContNew>() ||
curr->is<ContBind>()) {
return true;
}
if (auto* cast = curr->dynCast<RefCast>()) {
if (!cast->desc) {
return true;
}
if (!cast->desc->type.isRef()) {
return true;
}
if (!cast->desc->type.getHeapType().getDescribedType()) {
Copy link
Copy Markdown
Member

@kripken kripken Apr 20, 2026

Choose a reason for hiding this comment

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

Wouldn't this fail validation?

Copy link
Copy Markdown
Member Author

@tlively tlively Apr 20, 2026

Choose a reason for hiding this comment

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

Ah, this is from an outdated version of #8616. Will update this branch.

But to answer the question, yes, this would fail validation, but this code is used from the Printer, so needs to work correctly for invalid IR as well.

return true;
};
}
}
return false;
}

Expand Down
74 changes: 4 additions & 70 deletions src/passes/Print.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,63 +320,6 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
void visitTryTable(TryTable* curr);

void printUnreachableReplacement(Expression* curr);
bool maybePrintUnreachableReplacement(Expression* curr, Type type);
void visitRefCast(RefCast* curr) {
if ((curr->desc && curr->desc->type != Type::unreachable) ||
!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitStructNew(StructNew* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitArrayNew(ArrayNew* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitArrayNewData(ArrayNewData* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitArrayNewElem(ArrayNewElem* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitArrayNewFixed(ArrayNewFixed* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitContNew(ContNew* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitContBind(ContBind* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitResume(Resume* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitResumeThrow(ResumeThrow* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}
void visitStackSwitch(StackSwitch* curr) {
if (!maybePrintUnreachableReplacement(curr, curr->type)) {
visitExpression(curr);
}
}

// Module-level visitors
void handleSignature(Function* curr, bool printImplicitNames = false);
Expand Down Expand Up @@ -3143,19 +3086,6 @@ void PrintSExpression::printUnreachableReplacement(Expression* curr) {
decIndent();
}

bool PrintSExpression::maybePrintUnreachableReplacement(Expression* curr,
Type type) {
// When we cannot print an instruction because the child from which it's
// supposed to get a type immediate is unreachable, then we print a
// semantically-equivalent block that drops each of the children and ends in
// an unreachable.
if (type == Type::unreachable) {
printUnreachableReplacement(curr);
return true;
}
return false;
}

static bool requiresExplicitFuncType(HeapType type) {
// When the `(type $f)` in a function's typeuse is omitted, the typeuse
// matches or declares an MVP function type. When the intended type is not an
Expand Down Expand Up @@ -4009,6 +3939,10 @@ std::ostream& operator<<(std::ostream& o, wasm::ModuleExpression pair) {
}

std::ostream& operator<<(std::ostream& o, wasm::ShallowExpression expression) {
if (Properties::hasUnwritableTypeImmediate(expression.expr)) {
o << "(; unreachable " << getExpressionName(expression.expr) << " ;)";
return o;
}
wasm::PrintSExpression printer(o);
printer.setModule(expression.module);
wasm::PrintExpressionContents(printer).visit(expression.expr);
Expand Down
87 changes: 55 additions & 32 deletions src/wasm/wasm-ir-builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,7 @@ MaybeResult<IRBuilder::HoistedVal> IRBuilder::hoistLastValue(bool greedy) {
}
auto*& expr = stack[valIndex];
if (expr->type == Type::unreachable) {
// Make sure the top of the stack also has an unreachable expression.
if (stack.back()->type != Type::unreachable) {
pushSynthetic(builder.makeUnreachable());
}
// No need for a scratch local to hoist an unreachable.
return HoistedVal{Index(hoistIndex), nullptr};
}
// Hoist with a scratch local. Normally the scratch local is the same type as
Expand Down Expand Up @@ -129,6 +126,11 @@ Result<> IRBuilder::packageHoistedValue(const HoistedVal& hoisted,
};

auto type = scope.exprStack.back()->type;
if (type == Type::none) {
// If we did not have a value on top of the stack and did not add a scratch
// local, then there must have been an unreachable.
type = Type::unreachable;
}

if (type.size() == sizeHint || type.size() <= 1) {
if (hoisted.hoistIndex < scope.exprStack.size() - 1) {
Expand Down Expand Up @@ -280,6 +282,8 @@ void IRBuilder::dump() {
if (tryy->name) {
std::cerr << " " << tryy->name;
}
} else if (scope.getTryTable()) {
std::cerr << "try_table";
} else {
WASM_UNREACHABLE("unexpected scope");
}
Expand All @@ -303,7 +307,8 @@ void IRBuilder::dump() {
std::cerr << ":\n";

for (auto* expr : scope.exprStack) {
std::cerr << " " << ShallowExpression{expr} << "\n";
std::cerr << " " << ShallowExpression{expr} << " (; "
<< expr->type.toString() << " ;)\n";
}
}
#endif // IR_BUILDER_DEBUG
Expand Down Expand Up @@ -356,32 +361,26 @@ struct IRBuilder::ChildPopper
Result<> popConstrainedChildren(std::vector<Child>& children) {
auto& scope = builder.getScope();

// The index of the shallowest unreachable instruction on the stack, found
// by checkNeedsUnreachableFallback.
std::optional<size_t> unreachableIndex;

// Whether popping the children past the unreachable would produce a type
// mismatch or try to pop from an empty stack.
bool needUnreachableFallback = false;
// The stack size at which we are about to pop an unreachable instruction
// that we must not pop past because the expressions underneath it do not
// have the types we need.
std::optional<size_t> unreachableFallbackSize;

// We only need to check requirements if there is an unreachable.
// Otherwise the validator will catch any problems.
if (scope.unreachable) {
needUnreachableFallback =
checkNeedsUnreachableFallback(children, unreachableIndex);
unreachableFallbackSize = checkNeedsUnreachableFallback(children);
}

// We have checked all the constraints, so we are ready to pop children.
for (int i = children.size() - 1; i >= 0; --i) {
if (needUnreachableFallback &&
scope.exprStack.size() == *unreachableIndex + 1 && i > 0) {
if (unreachableFallbackSize &&
scope.exprStack.size() == *unreachableFallbackSize && i > 0) {
// The next item on the stack is the unreachable instruction we must
// not pop past. We cannot insert unreachables in front of it because
// it might be a branch we actually have to execute, so this next item
// must be child 0. But we are not ready to pop child 0 yet, so
// synthesize an unreachable instead of popping. The deeper
// instructions that would otherwise have been popped will remain on
// the stack to become prior children of future expressions or to be
// not pop past. We could pop it as child 0, but we are not ready to pop
// child 0 yet, so synthesize an unreachable instead of popping. The
// deeper instructions that would otherwise have been popped will remain
// on the stack to become prior children of future expressions or to be
// implicitly dropped at the end of the scope.
*children[i].childp = builder.builder.makeUnreachable();
continue;
Expand All @@ -397,9 +396,10 @@ struct IRBuilder::ChildPopper
return Ok{};
}

bool checkNeedsUnreachableFallback(const std::vector<Child>& children,
std::optional<size_t>& unreachableIndex) {
std::optional<size_t>
checkNeedsUnreachableFallback(const std::vector<Child>& children) {
auto& scope = builder.getScope();
std::optional<size_t> unreachableFallbackSize;

// Two-part indices into the stack of available expressions and the vector
// of requirements, allowing them to move independently with the granularity
Expand All @@ -409,6 +409,9 @@ struct IRBuilder::ChildPopper
size_t childIndex = children.size();
size_t childTupleIndex = 0;

// Whether we are deeper than some concrete expression.
bool seenConcrete = false;

// Check whether the values on the stack will be able to meet the given
// requirements.
while (true) {
Expand All @@ -435,7 +438,7 @@ struct IRBuilder::ChildPopper
// the input unreachable instruction is executed first. If we are
// not reaching past an unreachable, the error will be caught when
// we pop.
return true;
return unreachableFallbackSize;
}
--stackIndex;
stackTupleIndex = scope.exprStack[stackIndex]->type.size() - 1;
Expand All @@ -450,22 +453,35 @@ struct IRBuilder::ChildPopper
}

// We have an available type and a constraint. Only check constraints if
// we are past an unreachable, since otherwise we can leave problems to be
// caught by the validator later.
// we are deeper than an unreachable, since otherwise we can leave
// problems to be caught by the validator later.
auto type = scope.exprStack[stackIndex]->type[stackTupleIndex];
if (unreachableIndex) {
if (unreachableFallbackSize) {
auto constraint = children[childIndex].constraint[childTupleIndex];
if (!PrincipalType::matches(type, constraint)) {
return true;
return unreachableFallbackSize;
}
}

// No problems for children after this unreachable.
// We may need an unreachable fallback if we find violated constraints in
// expressions deeper than this unreachable. Calculate the stack size at
// which this unreachable will be the next thing popped. This size is
// usually one more than the current index, but if there are no shallower
// concrete expressions, then everything down to this unreachable will be
// popped at once immediately at the current stack size.
if (type == Type::unreachable) {
unreachableIndex = stackIndex;
unreachableFallbackSize =
seenConcrete ? stackIndex + 1 : scope.exprStack.size();
} else {
// We skipped none-typed expressions and this isn't unreachable, so it
// must be concrete.
assert(type.isConcrete());
seenConcrete = true;
}
}
return false;

// No unreachable fallback necessary.
return std::nullopt;
}

// If `greedy`, then we will pop additional none-typed expressions that come
Expand Down Expand Up @@ -848,6 +864,13 @@ Result<Expression*> IRBuilder::finishScope(Block* block) {
return Err{"popping from empty stack"};
}

if (scope.exprStack.back()->type == Type::none) {
// Nothing was hoisted, which means there must have been an unreachable
// buried under none-type expressions. It is not valid to end a concretely
// typed block with none-typed expressions, so add an extra unreachable.
pushSynthetic(builder.makeUnreachable());
}

if (type.isTuple()) {
auto hoistedType = scope.exprStack.back()->type;
if (hoistedType != Type::unreachable &&
Expand Down
48 changes: 48 additions & 0 deletions test/lit/wat-kitchen-sink.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5260,6 +5260,54 @@
)
)

;; CHECK: (func $stacky-unreachable-fallback (type $0)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (block ;; (replaces unreachable StructSet we can't emit)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $stacky-unreachable-fallback
;; i32 cannot be the struct.set's ref child, so we cannot pop past the
;; unreachable. The unreachable and the following nop are popped together.
i32.const 0
unreachable
nop
struct.set $pair 0
Copy link
Copy Markdown
Member

@kripken kripken Apr 20, 2026

Choose a reason for hiding this comment

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

Can we not stop parsing entirely at the unreachable? We can skip unreachable code since we don't try to precisely roundtrip anyhow. In fact I believe we used to do that in the past?

Copy link
Copy Markdown
Member Author

@tlively tlively Apr 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We can't skip parsing instructions after unreachables if we want to be able to write tests containing expressions with unreachable children (which we definitely do). We might have skipped parsing such instructions in the binary parser in the past, but now both text and binary parsers use IRBuilder.

Copy link
Copy Markdown
Member

@kripken kripken Apr 20, 2026

Choose a reason for hiding this comment

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

Oh, right - makes sense.

)

;; CHECK: (func $stacky-unreachable-ok (type $0)
;; CHECK-NEXT: (struct.set $pair $first
;; CHECK-NEXT: (struct.new_default $pair)
;; CHECK-NEXT: (block
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $stacky-unreachable-ok
;; Now we can pop past the unreachable. The unreachable and nop are still
;; popped together.
struct.new_default $pair
unreachable
nop
struct.set $pair 0
)

;; CHECK: (func $paren-in-string (type $0)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (string.const ")")
;; CHECK-NEXT: )
(func $paren-in-string
;; We should not be tripped up by an extra close parenthesis inside a string.
(drop
Expand Down
Loading