Skip to content

Fix segfault on cyclic or deeply-nested AST in compile()#7630

Merged
youknowone merged 1 commit into
RustPython:mainfrom
changjoon-park:fix-ast-cycle-segfault
Apr 19, 2026
Merged

Fix segfault on cyclic or deeply-nested AST in compile()#7630
youknowone merged 1 commit into
RustPython:mainfrom
changjoon-park:fix-ast-cycle-segfault

Conversation

@changjoon-park
Copy link
Copy Markdown
Contributor

@changjoon-park changjoon-park commented Apr 19, 2026

Closes #4862.

Symptom

compile() with a cyclic or pathologically deep AST object (built via the ast module) overflows the native stack and crashes RustPython with SIGSEGV. CPython raises RecursionError: maximum recursion depth exceeded while traversing <kind> node.

Reproduction

import ast
d = ast.UnaryOp(op=ast.Not(), lineno=0, col_offset=0)
d.operand = d  # self-reference -> infinite traversal
compile(ast.Expression(d), '<t>', 'eval')
Before After
RustPython SIGSEGV RecursionError: maximum recursion depth exceeded while traversing AST node
CPython 3.14 RecursionError: maximum recursion depth exceeded while traversing 'expr' node (unchanged)

Root cause

The ast_from_object conversion from a Python ast object tree to the internal ruff_python_ast types in crates/vm/src/stdlib/_ast/node.rs recursed unconditionally:

  • Box<T>::ast_from_object called T::ast_from_object once per descent (no guard)
  • Vec<T>::ast_from_object called Node::ast_from_object per element (no guard)

A cycle in the Python-level AST turns either call into unbounded recursion, which overflows the Rust native stack. Rust cannot recover from a stack overflow — the process is killed.

Fix

Wrap both recursive descents in vm.with_recursion(...), which is the existing machinery RustPython uses elsewhere to translate native recursion into a Python-level RecursionError at the configured limit (default 1000). No new infrastructure, no configuration changes.

// Box<T>::ast_from_object — single recursive descent
vm.with_recursion("while traversing AST node", || {
    T::ast_from_object(vm, source_file, object).map(Self::new)
})

// Vec<T>::ast_from_object — per-element descent
vm.extract_elements_with(&object, |obj| {
    vm.with_recursion("while traversing AST node", || {
        Node::ast_from_object(vm, source_file, obj)
    })
})

The "while traversing AST node" label matches CPython's RecursionError message style for this failure mode.

Why both Box<T> and Vec<T> must be guarded

All recursive AST fields use one of these two containers:

  • Box etc. — direct single-child descent (most binary/unary expressions, attribute access, subscripts, etc.)
  • Vec etc. — multi-child descent (list/tuple/set literals, dict values, match patterns, call args, etc.)

An earlier iteration of this patch only guarded Box<T>. Testing showed that List.elts = [self] and Tuple.elts = [self] still crashed because they traverse via Vec<T>. Guarding both closes every cycle path I could construct.

Verification

Direct-cycle cases (were segfaulting; now raise RecursionError)

  • UnaryOp.operand = self
  • BinOp.left = self
  • Call.func = self
  • Attribute.value = self
  • Subscript.value = self
  • IfExp.test = self
  • List.elts = [self]
  • Tuple.elts = [self]
  • Set.elts = [self]
  • Dict.values = [self]
  • ListComp.elt = self

Indirect cycles

  • A.operand = B; B.operand = A — caught on the same counter.

Deep non-cyclic input

Depth Behavior
500 compiles OK
900 compiles OK
999 compiles OK
1500 / 3000 / 5000 RecursionError (safe; no crash)

Regression

$ ./target/release/rustpython -m test test_ast test_compile
All 2 tests OK.
Result: SUCCESS

A new regression test in extra_tests/snippets/stdlib_ast.py exercises 6 representative cycle patterns across both Box and Vec descent paths; it passes on both RustPython (with this patch) and CPython 3.14.

Scope

  • Targeted at the ast_from_object path, which is the only way a cyclic tree can enter RustPython's compilation pipeline (parsed source cannot be cyclic; ruff parser produces trees, not DAGs).
  • ast_to_object (reverse direction) is only called on trees produced by ruff_python_parser, which does not emit cycles; no user-controlled path reaches it.
  • validate_mod runs after ast_from_object. Since ast_from_object now caps depth via the VM's recursion counter, any AST reaching validate_mod is already depth-bounded; local testing at depth 999 confirms validate_mod handles it without crashing.

Why this approach (vs. a dedicated counter in codegen)

The initial attempt placed a depth counter inside Compiler::compile_expression (crates/codegen/src/compile.rs). That turned out to be downstream of the crash site — the process is already dead before reaching codegen. The fix lives where the crash happens: the ast_from_object traversal. Using the VM's existing with_recursion keeps the mechanism consistent with how RustPython already converts native overflows to RecursionError elsewhere (e.g. during __repr__ recursion, __eq__ recursion).

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of cyclic and pathologically deep AST structures to prevent stack overflow and raise proper recursion errors instead.
  • Tests

    • Added comprehensive regression tests for cyclic AST nodes to ensure proper error handling across various node types.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: e33827b6-5e24-44b5-82cc-f6c7899420ab

📥 Commits

Reviewing files that changed from the base of the PR and between 9669118 and 03ca2d5.

📒 Files selected for processing (2)
  • crates/vm/src/stdlib/_ast/node.rs
  • extra_tests/snippets/stdlib_ast.py

📝 Walkthrough

Walkthrough

Adds VM recursion guards around AST node deserialization for Vec<T> and Box<T> types to prevent stack overflow on cyclic AST structures, converting potential segfaults into managed RecursionError exceptions.

Changes

Cohort / File(s) Summary
AST Node Recursion Guards
crates/vm/src/stdlib/_ast/node.rs
Wrapped element conversions in Vec<T>::ast_from_object and inner conversions in Box<T>::ast_from_object with vm.with_recursion(...) calls to manage recursion depth during deserialization.
Cyclic AST Regression Test
extra_tests/snippets/stdlib_ast.py
Added _cyclic_cases() helper and test logic covering cyclic references in UnaryOp, BinOp, Call, Attribute, List, and Tuple nodes, verifying that compile() raises RecursionError instead of segfaulting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • youknowone

Poem

🐰 A rabbit hops through AST trees,
No cycles shall cause our disease!
With guards and recursion care,
Stack overflow? Never there! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly describes the main change: fixing a segfault on cyclic or deeply-nested AST in compile(), which matches the core modification of adding recursion guards to AST deserialization.
Linked Issues check ✅ Passed The PR successfully addresses issue #4862 by wrapping recursive AST node deserialization in VM recursion guards, preventing stack overflow and raising RecursionError instead, matching the stated objective.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the cyclic AST issue: recursion guards in ast_from_object conversions and a regression test for cyclic structures, with no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone
Copy link
Copy Markdown
Member

awesome, thank you so much.

@youknowone youknowone merged commit fdb49d8 into RustPython:main Apr 19, 2026
20 checks passed
@changjoon-park
Copy link
Copy Markdown
Contributor Author

awesome, thank you so much.

Thanks for the review :)

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.

Segfault when compiling a nested AST

2 participants