v.ast, v.checker: fix infinite loops and segfaults in generic instantiation and cyclic aliases - #28005
v.ast, v.checker: fix infinite loops and segfaults in generic instantiation and cyclic aliases#28005tailsmails wants to merge 30 commits into
Conversation
Limit the depth of alias resolution to prevent infinite loops.
Add a depth limit check for generic instantiation.
Reduced the cutoff limit for generic function post-processing iterations from 1000 to 50. Updated loop variables to be mutable for variants in sum type declarations.
Added tests for nested generic functions, structs, type aliases, and sum types.
|
|
I have also found a 5th related segmentation fault in convert_generic_type with this recursive alias pattern, and I will address it in a separate PR soon.. |
|
Update: Found another related segfault while testing this branch. It happens when resolving recursive generic function pointer aliases like this: type Func[T] = fn (f Func[[]T]) T
fn main() {
_ := Func[int](none)
}On master, this crashes with a segmentation fault in What was going wrong: The fix: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab9edcefea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if t.fn_generic_types[fn_name].len > generic_fn_inst_cutoff_limit { | ||
| t.panic('generic function instantiation limit exceeded') |
There was a problem hiding this comment.
Replace the total generic-instantiation cap with cycle detection
When a valid program specializes one generic function for 257 distinct concrete type lists, the map entry reaches length 257 and every subsequent call to register_fn_concrete_types—even a duplicate that would otherwise return false—panics here. The number of finite specializations is not evidence of recursive expansion, and generated serializers or dispatch code can legitimately exceed this threshold, so this turns valid compilation into a compiler crash; guard actual recursive growth instead of the accumulated specialization count.
Useful? React with 👍 / 👎.
| if inst_name.len > generic_inst_name_len_cutoff_limit { | ||
| t.panic('generic instantiation name limit exceeded') |
There was a problem hiding this comment.
Allow finite generic instance names longer than 1 KiB
A finite generic instantiation can exceed 1,024 bytes without containing a cycle—for example, a generated generic with many concrete parameters or nested qualified type names—yet it now unconditionally panics here. Because V does not otherwise impose this identifier-length restriction, valid source that previously compiled now crashes; recursion should be bounded by the instantiation graph rather than the rendered name length.
Useful? React with 👍 / 👎.
| const generic_fn_cutoff_limit_per_fn = 10_000 // how many times post_process_generic_fns, can visit the same function before bailing out | ||
|
|
||
| const generic_fn_postprocess_iterations_cutoff_limit = 1_000_000 | ||
| const generic_fn_postprocess_iterations_cutoff_limit = 50 |
There was a problem hiding this comment.
Preserve finite generic chains beyond 50 rechecks
A valid chain of more than roughly 50 generic functions can require more than 50 passes when declarations are ordered from the innermost function to the outermost and only the outermost function is initially instantiated: each pass discovers the concrete type for a function that was already visited earlier in that pass. This fixed cutoff therefore emits an infinite-instantiation error for a finite chain that previously converged; use repeated-state/no-progress detection or a limit that does not reject ordinary finite propagation.
Useful? React with 👍 / 👎.
|
I tweaked the constants a bit.
|
Raise generic post-process limit to 256The old limit of 50 was too tight for complex nested generics, leading to false-positive loop errors on valid code. At the same time, we can't set it too high because actual infinite loops will hang the compiler and spike CPU usage. |
I think maybe 128 is safer than 256 |
|
Hi there. |
Removed unused fields for generic type depth from the context structure.
|
Everything is going well so far. We just need to wait for the Actions to complete. |
|
All the main checks are passing now. The cutoff limits are still hardcoded Happy to change direction if you have other thoughts! Update: the cutoff limits are now compiler flags, not hardcoded consts: -generic-fn-inst-limit, -generic-inst-name-len-limit, -generic-inst-depth-limit (≤512 clamped), -alias-unwrap-depth-limit, -generic-fn-postprocess-iters, -max-postprocess-iterations. Defaults match the previous consts, so behavior without flags is unchanged. Panic/error messages name the flag to use. Covered by vlib/v/tests/recursion_cutoff_flags_test.v (29 assertions). |
|
I'm working on a few new features on top of this; I'll publish them once they're ready. |
This file contains tests for user-configurable recursion and cutoff guard limits in the V programming language. It includes various test cases to ensure that the limits are respected and documented correctly.
Added resource limit settings and updated command line options for generic function and alias limits.
Added resource limits for generic functions and aliases.
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
bruh |
|
Ok, now we need to wait for the Actions to complete again. |
Add new flags for checker fixture and macOS compatibility.
What this does
Fixes the compiler hangs (100% CPU forever) and segfaults (stack overflows) when the checker hits recursive generics or cyclic aliases. Repros for all 5 cases are in the comments below.
The idea is simple: put cutoff limits on the code paths that can loop forever (rustc and GHC do the same thing), and fix the validation so a generic sum type can't sneak itself in as a variant.
What changed
fully_unaliased_typecan't spin forever on circular aliases any more (100 unwrap iterations max).unwrap_generic_type_ex_with_depthtracks recursion depth and panics cleanly at 256, instead of overflowing the stack. The depth is passed as a local arg - the earlier version with counters onTablehit a data race on the Linux Clang CI.1_000_000const that master currently has there.sum_type_declstrips the generic brackets before comparing names, sotype MySum[T] = T | MySum[MySum[T]]is rejected as circular like it should be. The comparison is module-aware now, so a same-named type from another module doesn't produce a false positive.generic_insts_to_concrete, hitting a circular sum type now skips just that one instantiation instead of returning from the whole function, which was silently dropping every instantiation after it.The limits are flags now
These started out as hardcoded consts. After the review feedback I just exposed them as flags instead of adding real cycle detection - detection would tax some of the hottest checker paths to catch code that (honestly) nobody writes by hand, and anyone who genuinely needs a bigger limit can pass it:
-generic-fn-inst-limit(default 4096)-generic-inst-name-len-limit(default 8192)-generic-inst-depth-limit(default 256, clamped to 512 - go deeper and the native stack dies before the guard can fire, I ran into that segfault while testing at 512)-alias-unwrap-depth-limit(default 100)-generic-fn-postprocess-iters(default 128)-max-postprocess-iterations(default 100000)Defaults are the same values the consts had, so nothing changes if you don't pass the flags. Panic/error messages tell you which flag to raise. Also documented in
v help build.Tests
vlib/v/tests/recursion_cutoff_flags_test.v- every crash repro now exits with a controlled error/panic instead of hanging or segfaulting, each flag demonstrably changes its limit, the 512 clamp works, and valid code still compiles and runs untouched with the defaults. 29 asserts total.