Cranelift: ISLE recursion check by mmcloughlin · Pull Request #12474 · bytecodealliance/wasmtime

mmcloughlin · 2026-01-29T19:55:14Z

Extends the ISLE compiler with a recursive terms check. Terms with rules that include a reference cycle are only allowed if the term explicitly opts in with the rec attribute.

Recently, we have seen problems where recursive rules enable user-controlled recursion and therefore potential stack overflow enabling a compilation DoS. For example, see #12368, #12333, and #12369. With this additional compilation check, recursion is not disallowed but is more explicit, making audits easier and offering a path to eventually disallowing recursive rules altogether.

cfallin

Thanks for this! I think this will be a great improvement in our ability to reason about lowering complexity.

My main question/concern is about the semantics. It appears (after reading through it all -- some comments below are written as I worked this out) that the semantics are: any term that participates in a rec cycle must have rec, and any term with rec must participate in a recursive cycle, is that right?

Naively, at least coming from experience with other "permissive superset" features of languages, I would expect rec to permit recursion but not require it (e.g., unsafe in Rust, or letrec chains of bindings in a Lisp/Scheme, or ...). But maybe this is actually good if it means it forces us to rethink carefully (and keep justifications up-to-date) if we remove the recursion, too. Regardless, I'd like to see a description of the precise semantics and a distillation of the thinking behind it in the langref, so we can preserve that intent going forward.

A few other minor nits but otherwise seems fine -- thanks!

cfallin · 2026-01-29T21:17:19Z

cranelift/isle/isle/src/ast.rs

    pub multi: bool,
    /// Whether this term's constructor can fail to match.
    pub partial: bool,
+    /// Whether this term is recursive.


s/is recursive/is permitted to be recursive/, right?

If you wouldn't mind, it'd be great to update the langref at the same time (cranelift/isle/docs/language-reference.md). In particular the semantics could use a little bit of preciseness: is it that in any cycle in the callgraph, at least one term must have rec?

Ah, after reading further, I see that actually all terms in the cycle must have rec, and all terms with rec must participate in a cycle; is that right?

Updated to the "permit recursion" semantics. Added a section to the language reference.

cfallin · 2026-01-29T21:18:03Z

cranelift/isle/isle/src/error.rs

        rules: Vec<Span>,
    },

+    /// Recurive rules error. Term is recursive without explicit opt-in, or vice versa.


"or vice versa" implies that if a term is marked rec then it must participate in a recursive cycle?

No longer has the "must participate" semantics. Updated comment accordingly.

cfallin · 2026-01-29T21:19:57Z

cranelift/isle/isle/src/recursion.rs

+    let mut errors = Vec::new();
+    for (term_id, _) in terms {
+        // Check if this term is involved in a reference cycle.
+        let reachable = terms_reachable_from(*term_id, &term_rule_sets);


I'm somewhat concerned about runtime here: for every term we're computing all reachable terms, which in the limit in an arbitrary graph has O(V^2) complexity, right?

Could we instead do a DFS over the forest (i.e., DFS with a toplevel loop over all nodes that starts a new DFS from a not-yet-visited node) to detect cycles? That should have O(V + E) complexity.

Updated to use a DFS algorithm over the term-reference graph which should have O(V+E) complexity.

cfallin · 2026-01-29T21:22:30Z

cranelift/codegen/src/isa/aarch64/inst.isle

 ;; Helper for emitting `MInst.FpuCSel16` / `MInst.FpuCSel32` / `MInst.FpuCSel64`
 ;; instructions.
-(decl fpu_csel (Type Cond Reg Reg) ConsumesFlags)
+(decl rec fpu_csel (Type Cond Reg Reg) ConsumesFlags)


In each case where we have a rec, could we have a comment describing why it's bounded?

In particular I'm thinking these should be modeled after something like Rust safety comments -- so e.g. // Recursion: may recurse once to reuse implementation for F32 in the case of F16 or // Recursion: bounded by explicit depth parameter 'depth' or // Recursion: each iteration of count_the_bits_in_isle removes one bit from value and completes when zero or something like that.

Added ; Recursion: ... comments for all (decl rec ...) terms.

github-actions · 2026-01-29T22:51:24Z

Subscribe to Label Action

cc @cfallin, @fitzgen

Details

This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:x64", "isle"

Thus the following users have been cc'd because of the following labels:

cfallin: isle
fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

mmcloughlin · 2026-02-12T04:01:44Z

Updated in response to review comments. Main changes:

Cycle finding via plain DFS with back-edge detection. Ideally we might use something like Johnson's Algorithm or a variant of Tarjan's, but DFS will find a cycle if there is one, and
should be good enough in practice for the term-reference graphs we have.
Change rec attribute semantics to "permit recursion" rather than "permit and require recursion".
Add pass and fail test cases under isle_examples.
Document the rec attribute in the language reference.
Provide safety comments for all existing uses of rec in ISLE backends.

cfallin

LGTM; thanks!

mmcloughlin requested a review from a team as a code owner January 29, 2026 19:55

mmcloughlin requested review from alexcrichton and removed request for a team January 29, 2026 19:55

mmcloughlin changed the title ~~Isle recursion check~~ Cranelift: ISLE recursion check Jan 29, 2026

cfallin reviewed Jan 29, 2026

View reviewed changes

github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen isle Related to the ISLE domain-specific language labels Jan 29, 2026

mmcloughlin added 10 commits February 10, 2026 18:39

add decl rec attribute

9e6ed7e

implement recursion check

37be9e0

add required req attributes

989a491

document the RecursionError variant

8ac037b

add required req attributes to pully ISLE rules

e4d09b5

add rec to isle example

e1b8c97

use dfs for cycle detection

9e2b69c

document the rec attribute

794e140

recursion safety comments

719bb00

"is permitted to be recursive"

2d3ab30

mmcloughlin force-pushed the isle-recursion-check branch from 732f337 to 2d3ab30 Compare February 12, 2026 03:46

fix documentation of RecursionError variant

94a469e

cfallin approved these changes Feb 12, 2026

View reviewed changes

cfallin added this pull request to the merge queue Feb 12, 2026

Merged via the queue into bytecodealliance:main with commit 8ec3ff3 Feb 12, 2026
76 checks passed

Conversation

mmcloughlin commented Jan 29, 2026

Uh oh!

cfallin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 29, 2026

Subscribe to Label Action

Uh oh!

mmcloughlin commented Feb 12, 2026

Uh oh!

cfallin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants