Skip to content

Fix use child typer in the validator 6613 - #9125

Open
ArkadySkv wants to merge 3 commits into
WebAssembly:mainfrom
ArkadySkv:fix-use-ChildTyper-in-the-validator-6613
Open

ArkadySkv wants to merge 3 commits into
WebAssembly:mainfrom
ArkadySkv:fix-use-ChildTyper-in-the-validator-6613

Conversation

@ArkadySkv

@ArkadySkv ArkadySkv commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Problem: FunctionValidator::visitUnary and visitBinary contain
~400 lines of per-opcode child-type checks. The same constraints
are already encoded in src/ir/child-typer.h and consumed by the
IRBuilder. The duplication is error-prone: a missed case is a
soundness hole. visitRefIsNull and visitRefEq have the same
shape at smaller scale.

Fix (Path A): Add a ValidatorTypeChecker subclass of
ChildTyper<ValidatorTypeChecker> in wasm-validator.cpp. Its
note() callback checks each child's type against the emitted
constraint via PrincipalType::matches, skipping unreachable
children. Replace the switches in visitUnary and visitBinary
with single calls, and migrate the per-child checks in
visitRefIsNull and visitRefEq. Feature gates, structural
checks, and the cross-child sharedness check on visitRefEq stay
in the validator, since ChildTyper does not express them.

Verification:

git show HEAD~3:src/wasm/wasm-validator.cpp \
  | awk '/^void FunctionValidator::visitUnary/{p=1} p{print} p && /^}/{exit}' \
  | grep -oE 'case [A-Za-z0-9_]+:' | sort -u > /tmp/val_unary.txt
sed -n '/void visitUnary(/,/^  }/p' src/ir/child-typer.h \
  | grep -oE 'case [A-Za-z0-9_]+:' | sort -u > /tmp/ct_unary.txt
diff /tmp/val_unary.txt /tmp/ct_unary.txt   # must be empty
# same for visitBinary
python3 check.py lit                          # 1002 pass

New regression tests: binary-bad-operand.wast,
ref-is-null-bad-operand.wast, i31-get-bad-operand.wast.

Two migrations were attempted and reverted, both blocked by
ChildTyper constraints that drop sharedness: visitI31Get
(constraint fixed to unshared i31; the validator uses
shouldBeSubTypeIgnoringShared) and visitCallRef (target noted
as Type(getSignature(), Nullable), which loses sharedness).
Details in #NNNN.

Follow-up: #NNNN tracks completing the remaining ~70 methods;
#NNNN tracks the sharedness incompatibility. Per @tlively's
suggestion, a utility that returns an expression's full principal
type would address both — it would let the validator reason about
sharedness directly and would also serve the Outlining pass.

validation performance.

Built base and new from the same commit (version_132-98-gd88447204) so the only difference is the PR. Corpus: 349 test/lit/passes/*.wast files that both binaries parse and validate cleanly. hyperfine --warmup 3 --runs 30:

base:  3.353 s ± 0.725 s
new:   3.371 s ± 0.623 s
base is 1.01 ± 0.29 times faster than new

The interval straddles 1.0. No measurable difference at this corpus size. The change replaces each shouldBeEqualOrFirstIsUnreachable with a ValidatorTypeChecker::note that calls PrincipalType::matches; the work per check is comparable, which matches the measurement.

If you have a larger real-world wasm you would like me to run against, happy to; the current corpus is bounded by process startup cost rather than by validation work.

Refs #6613. Does not close it.

@ArkadySkv
ArkadySkv requested a review from a team as a code owner September 20, 2026 12:01
@ArkadySkv
ArkadySkv requested review from tlively and removed request for a team September 20, 2026 12:01

@tlively tlively left a comment

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.

Seems promising! Can you:

  1. Rewrite the description to remove extraneous detail.
  2. Measure how this change affects the performance of validation.

Comment on lines +2226 to +2228
// ChildTyper constrains each child to the eqref family, but each note()
// call runs with a fresh VarAssignments, so the sharedness variable does
// not unify across left and right. Keep the cross-child check here.

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.

Presumably if we had a utility that gave the full principal type of an expression, we wouldn't have this problem any more. Such a utility would also be useful for e.g. the Outlining pass.

(func (result i32)
(i32.add (i32.const 0) (f64.const 0.0))
)
) No newline at end of file

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.

Please add newlines at the end of the test files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've done it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've done it

Phase 2 of WebAssembly#6613. Add a ValidatorTypeChecker subclass of
ChildTyper<ValidatorTypeChecker> whose note callback validates
each child's type against its constraint via
PrincipalType::matches. Replace the manual per-opcode switch in
FunctionValidator::visitUnary (~200 lines) with a single call.

The none guard and the feature check stay, since neither is a
child-type constraint. Unreachable children are skipped to
preserve the existing shouldBeEqualOrFirstIsUnreachable
semantics.

Coverage diff between the pre-patch visitUnary case labels and
ChildTyper::visitUnary case labels is empty, so no opcode is
lost. The diagnostic changes from opcode-specific messages to
"child type does not match its constraint" at the failing child,
exercised by test/lit/validation/unary-bad-operand.wast.

Partially fixes WebAssembly#6613.
Phase 3 Batch 1 of WebAssembly#6613. Extend ValidatorTypeChecker to visitBinary.
The tuple guard and sibling left->type == right->type check stay in the
validator, since ChildTyper does not express them.

Restore the FP16 feature gates that Phase 2 removed from visitUnary and
that this change would have removed from visitBinary. ChildTyper cannot
express the gate because FP16 ops share their operand types with their
non-FP16 counterparts; the gate is applied via requiresFP16(UnaryOp)
and requiresFP16(BinaryOp) before the ChildTyper check, matching the
pre-WebAssembly#6613 ordering.

Coverage diffs for both methods are empty. Exercised by
test/lit/validation/binary-bad-operand.wast, and by the pre-existing
fp16-unary.wast and fp16.wast, which now pass.

Refs WebAssembly#6613.
Phase 3 Batch 2 of WebAssembly#6613. Extend ValidatorTypeChecker to the two ref
visitors whose ChildTyper constraints are polymorphic over the heap
type and sharedness dimensions, and therefore match the validator's
semantic exactly. Feature gates stay in the validator. The cross-child
shareability check in visitRefEq also stays: ValidatorTypeChecker
calls PrincipalType::matches, which builds a fresh VarAssignments per
child, so the sharedness variable that ChildTyper's visitRefEq emits
does not unify across the two note() calls. Unifying it is a Phase 4
change that requires a persistent VarAssignments member.

visitI31Get is not migrated. ChildTyper::visitI31Get constrains the
argument to the unshared Type(HeapType::i31, Nullable), but the
validator's pre-existing check uses shouldBeSubTypeIgnoringShared,
which accepts (ref null (shared i31)) as well. Migrating would reject
shared i31 modules and regress basic/shared-i31.wast,
passes/make-shared-objects.wast, and passes/unsubtyping.wast. The
discrepancy is the same sharedness gap that the unsubtyping work
tracks; resolving it requires a sharedness-polymorphic constraint
facility in ValidatorTypeChecker, which is Phase 4 scope.

Adds two regression tests:
- test/lit/validation/ref-is-null-bad-operand.wast locks in the
  migrated visitRefIsNull rejection path.
- test/lit/validation/i31-get-bad-operand.wast locks in the
  pre-existing visitI31Get rejection path (a function reference is
  not an i31ref). The fixture uses (ref.func $f) rather than
  (ref.null $struct) because the parser types the latter as nullref,
  which is correctly accepted as an i31ref subtype.

visitTupleExtract, visitTupleMake, visitRefI31, visitRefAs, and the
SIMD family are skipped in this batch. Their switches are dominated
by result-type checks, immediate validation, or feature gates, with
at most one migratable child-type check apiece; migration would add
a cross-file dependency without a code reduction.

Refs WebAssembly#6613.
@ArkadySkv
ArkadySkv force-pushed the fix-use-ChildTyper-in-the-validator-6613 branch from adf4589 to d884472 Compare September 21, 2026 08:45
@ArkadySkv

Copy link
Copy Markdown
Contributor Author

Seems promising! Can you:

  1. Rewrite the description to remove extraneous detail.
  2. Measure how this change affects the performance of validation.

Two cases are done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants