GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix segfault on cyclic or deeply-nested AST in `compile()` by changjoon-park · Pull Request #7630 · RustPython/RustPython

Conversation

@changjoon-park

@changjoon-park changjoon-park commented Apr 19, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@changjoon-park
Fix segfault on cyclic or deeply-nested AST in compile()
03ca2d5
@coderabbitai

coderabbitai Bot commented Apr 19, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone
youknowone approved these changes Apr 19, 2026
View reviewed changes
@youknowone

youknowone commented Apr 19, 2026

Copy link
Copy Markdown
Member

awesome, thank you so much.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
youknowone merged commit fdb49d8 into RustPython:main Apr 19, 2026
20 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@changjoon-park

changjoon-park commented Apr 19, 2026

Copy link
Copy Markdown
Contributor Author

awesome, thank you so much.

Thanks for the review :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Apr 19, 2026
Fix stack overflow on deeply-nested JSON in json.loads() #7632
Merged
Fix process abort on large float format precision #7633
Merged
Parser stack overflow: deeply-nested source causes SIGSEGV instead of SyntaxError #7655
Closed
Recursions in rust code trigger segmentation faults. #2796
Closed
@changjoon-park
changjoon-park deleted the fix-ast-cycle-segfault branch April 27, 2026 13:23
youknowone pushed a commit that referenced this pull request Sep 16, 2026
@changjoon-park
Fix segfault on cyclic or deeply-nested AST in compile() (#7630)
7d2b37d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Segfault when compiling a nested AST

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@changjoon-park @youknowone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.