Skip to content

Fix dangling-node elimination to preserve trapping WebAssembly ops#107

Open
leonardoalt wants to merge 1 commit intomainfrom
codex/fix-dangling-node-removal-vulnerability
Open

Fix dangling-node elimination to preserve trapping WebAssembly ops#107
leonardoalt wants to merge 1 commit intomainfrom
codex/fix-dangling-node-removal-vulnerability

Conversation

@leonardoalt
Copy link
Copy Markdown
Member

Motivation

  • The dangling-node removal pass could delete unused expressions that may trap (e.g., integer division/remainder, truncating float→int conversions, memory/table reads), changing Wasm semantics and removing intended runtime traps.
  • The root cause was that may_have_side_effect classified many trap-capable Wasm operations as pure, allowing their removal when outputs were unused.

Description

  • Updated may_have_side_effect in src/loader/passes/dag/dangling_removal.rs to return true for trap-capable Wasm operations so they are treated as having side effects and are not removed by the dangling-node pass.
  • Operations explicitly marked include integer div/rem, truncating float→int conversions, table.get, and memory loads (various load opcodes and vector loads).
  • No other behavior or API surface was changed; the change is localized to the dangling-removal side-effect classification.

Testing

  • Ran cargo fmt --check which succeeded.
  • Ran cargo test -q --lib which succeeded (64 passed; 0 failed).
  • Ran full test suite with cargo test -q which exercised integration/binary tests; the run exhibited pre-existing integration failures and a stack overflow in the --bin womir execution that are not introduced by this patch (observed test failures remain at integration level).

Codex Task

@lvella
Copy link
Copy Markdown
Member

lvella commented Mar 10, 2026

@codex Remove the options from the match below instead of adding a new match with the exception above.

| Op::I64TruncF32U
| Op::I64TruncF64S
| Op::I64TruncF64U
| Op::TableGet { .. }
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.

How TableGet can trap?

@leonardoalt
Copy link
Copy Markdown
Member Author

@codex check review

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants