Skip to content

[R26] Bound recursive struct validation in compiler extensions - #31

Open
JetF0x wants to merge 1 commit into
Carsillas:rewritefrom
JetF0x:fix/review-r26
Open

JetF0x wants to merge 1 commit into
Carsillas:rewritefrom
JetF0x:fix/review-r26

Conversation

@JetF0x

@JetF0x JetF0x commented Sep 12, 2026

Copy link
Copy Markdown

Addresses review finding R26.

While editing an invalid struct such as struct V { public V X; }, Roslyn still provides its symbols to compiler extensions. Using that struct in a network list, dictionary value/key, RPC, or broadcast could overflow the analyzer stack and terminate the compiler or IDE process. Expanding generic fields such as V<T> -> V<V<T>> could also bypass the generator's existing exact-symbol cycle guard.

Track the active struct path using SymbolEqualityComparer.Default, report the existing unsupported-type diagnostic when a cycle is encountered, and check cancellation throughout recursive validation. Reuse the property's existing path policy and add a 128-struct nesting limit to handle expanding constructed generics. Apply the same limit to generated struct models, carry the generator cancellation token into those models, and stop expanding sibling fields as soon as one is unsupported. Shared sibling types and finite nesting such as Box<Box<int>> remain supported.

Validation on Windows with SDK 10.0.400 and net8.0 test hosts:

  • Added 73 analyzer and 55 generator cases covering direct, mutual, nullable, closed-generic, expanding-generic, branched expanding-generic, and unresolved-field declarations; shared acyclic structs; finite nested constructed generics; cancellation controls; and the analyzer's 128/129 nesting boundary.
  • Generator cases inspect both generator failures and the resulting compilation. Invalid cycles retain CS0523; unresolved fields retain CS0246; valid shared/generic controls compile without errors.
  • The complete Release solution suite passes: 356 tests (126 analyzer, 75 generator, 155 runtime), zero failures/skips. The only build warning is the existing CS8600 in MemoryRelayTransport.cs:21.
  • Isolated baseline tests for the direct list cycle and expanding-generic RPC generation both aborted with a test-host stack overflow and test-command exit code 1. Each ran in a hidden child process with a 30-second outer bound and a 10-second test hang bound, with hang dumps disabled. Neither hit the outer timeout; no dumps were produced.

Compatibility: no runtime API, wire format, package dependency, or diagnostic ID changes. The existing CN0015/CN0016/CN0023/CN0028 diagnostics cover rejected schemas. The defensive limit newly rejects otherwise acyclic schemas exceeding 128 nested serialized structs. R25's arbitrary-symbol cast, R37's recursive nullable collection-field validation, and R12's hidden/builtin struct policy remain outside this independent fix.

Integration with the other independent review PRs requires equivalent bounds and cancellation in their earlier traversal hooks: R22 (#27), Shared/NetworkDeclarationShape.HasReadonlySerializedField, and R23 (#28), NetworkTypeValidation.FindInaccessibleType (type arguments/public struct fields). Those walks run before supported-type/model validation and must not bypass this protection when the changes are combined. This branch does not include either sibling PR.

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.

1 participant