feat(extractor): borrow checker — pooled-buffer Span view used after Return → OWN002 (POOL002)#69
Merged
Merged
Conversation
…fter Return → OWN002 (POOL002) The first bite of the borrow checker on real C#, and the heavy-artillery frontier (B4 / P-007 POOL002). The flat pass and the existing pool flow only caught a use-after-return when the BUFFER LOCAL ITSELF was referenced after Return. When the read goes through a stored Span view — `Span<byte> view = buf.AsSpan(0, n); pool.Return(buf); view[0] = 42;` — the buffer name never appears at the use site, so the buffer looked released-and-untouched: a miss (and a nasty silent aliasing bug, since the array is recycled to another caller). A `Span`/`ReadOnlySpan` view of a tracked buffer is now modelled as a BORROW: a reference to the view local lowers to a USE OF THE OWNER (`ViewOwnerOf` resolves the view through its declaration's `buf.AsSpan(..)` / `new Span<T>(buf)` initializer, the Span-returning symbol confirmed via the SemanticModel). A use after `Return(buf)` then trips OWN002. Purely extractor-side — the core needs no borrow concept; it sees a plain use-after-release (the emitted facts are identical to arraypool-use-after-return: acquire buf; release buf; use buf). A ref-struct `Span` cannot escape the method, which is exactly what makes "use of the view = use of the owner, here" sound. Conservative (0 FP): a view of an untracked/escaped buffer, or one used BEFORE the Return (after.cs), adds no finding — the borrow only lowers to a use of an owner that is still tracked, so it can never invent a release. Pinned by corpus `arraypool-span-view-after-return` (before → OWN002, after silent), lifting the benchmark recall floor 11 → 12. First slice: `Span`/`ReadOnlySpan` via `AsSpan()` / `new Span<T>`; `Memory<T>` (escapable) and view reassignment are left for later rounds. Validated locally: case.own → OWN002, corpus 10/10, constructed extractor facts → OWN002. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe extractor's ChangesSpan View-After-Return Detection (OWN002)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98ec925255
ℹ️ 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".
Comment on lines
+842
to
+845
| && model.GetSymbolInfo(inv).Symbol is IMethodSymbol { ReturnType.Name: "Span" or "ReadOnlySpan" }) | ||
| return recv.Identifier.Text; | ||
| if (e is ObjectCreationExpressionSyntax oc | ||
| && oc.Type is GenericNameSyntax { Identifier.Text: "Span" or "ReadOnlySpan" } |
There was a problem hiding this comment.
When a project has its own AsSpan extension on arrays, or a non-System.Span<T> type named Span<T>, this code still treats the value as a borrow of the pooled buffer because it only checks ReturnType.Name / the syntactic type name. For example, a custom byte[].AsSpan() that returns a span over a fresh array will be reported as OWN002 after Return(buf) even though the view does not alias buf; the new precision claim depends on checking the resolved System.MemoryExtensions/System.Span symbols (and the constructor symbol) rather than just names.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/roslyn/OwnSharp.Extractor/Program.cs (1)
838-848: Consider hardening Span identity checks to prevent potential false borrow-owner mappings.
SpanViewOwnercurrently matches by simple names (ReturnType.Name,GenericNameSyntax.Identifier.Text). While no customSpanorReadOnlySpantypes were found in this codebase, this approach is vulnerable to misclassification if such types are introduced. The suggested fix adds semantic type-identity checking to ensure only BCL types are recognized:Suggested fix
+static bool IsSystemSpanLike(ITypeSymbol? t) => + t is INamedTypeSymbol nt + && nt.Name is "Span" or "ReadOnlySpan" + && nt.ContainingNamespace?.ToDisplayString() == "System"; + static string? SpanViewOwner(ExpressionSyntax? e, SemanticModel model) { if (e is InvocationExpressionSyntax inv && inv.Expression is MemberAccessExpressionSyntax m && m.Name.Identifier.Text == "AsSpan" && m.Expression is IdentifierNameSyntax recv - && model.GetSymbolInfo(inv).Symbol is IMethodSymbol { ReturnType.Name: "Span" or "ReadOnlySpan" }) + && model.GetSymbolInfo(inv).Symbol is IMethodSymbol ms + && IsSystemSpanLike(ms.ReturnType)) return recv.Identifier.Text; if (e is ObjectCreationExpressionSyntax oc - && oc.Type is GenericNameSyntax { Identifier.Text: "Span" or "ReadOnlySpan" } + && model.GetSymbolInfo(oc).Symbol is IMethodSymbol ctor + && IsSystemSpanLike(ctor.ContainingType) && oc.ArgumentList is { Arguments.Count: > 0 } && oc.ArgumentList.Arguments[0].Expression is IdentifierNameSyntax arg) return arg.Identifier.Text; return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/roslyn/OwnSharp.Extractor/Program.cs` around lines 838 - 848, The Span and ReadOnlySpan type checks in the if conditions are vulnerable to false matches because they only check the simple name rather than verifying semantic type identity. For the AsSpan invocation check using ReturnType.Name and the ObjectCreationExpressionSyntax check using GenericNameSyntax.Identifier.Text, add semantic type-identity validation to ensure only BCL types from the System namespace are matched. Use the semantic model (already available through the model variable used in GetSymbolInfo) to verify that the matched return type and created type are specifically the BCL Span or ReadOnlySpan types, not custom types with the same names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/roslyn/OwnSharp.Extractor/Program.cs`:
- Around line 838-848: The Span and ReadOnlySpan type checks in the if
conditions are vulnerable to false matches because they only check the simple
name rather than verifying semantic type identity. For the AsSpan invocation
check using ReturnType.Name and the ObjectCreationExpressionSyntax check using
GenericNameSyntax.Identifier.Text, add semantic type-identity validation to
ensure only BCL types from the System namespace are matched. Use the semantic
model (already available through the model variable used in GetSymbolInfo) to
verify that the matched return type and created type are specifically the BCL
Span or ReadOnlySpan types, not custom types with the same names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6ac2de3-6fee-4ebf-b429-f8ed29f366a0
📒 Files selected for processing (9)
.github/workflows/ci.ymlcorpus/real-world/README.mdcorpus/real-world/arraypool-span-view-after-return/after.cscorpus/real-world/arraypool-span-view-after-return/before.cscorpus/real-world/arraypool-span-view-after-return/case.owncorpus/real-world/arraypool-span-view-after-return/expected-diagnostics.txtcorpus/real-world/arraypool-span-view-after-return/notes.mddocs/proposals/P-007-arraypool-span.mdfrontend/roslyn/OwnSharp.Extractor/Program.cs
…iew borrow Codex P2: SpanViewOwner matched on the NAME `AsSpan` / `Span<T>` only, so a project's own `AsSpan` extension, a non-System type named `Span<T>`, or an `AsSpan` that returns a span over a fresh copy would be treated as a borrow of the pooled buffer — a false OWN002 after `Return(buf)` even though the view does not alias it. Now the borrow is recognised by the RESOLVED symbols: `System.MemoryExtensions.AsSpan` (aliases its receiver array) and the `System.Span<T>` / `System.ReadOnlySpan<T>` constructor (wraps its array argument), both confirmed via `IsInNamespace(..., "System")`. A look-alike in another namespace is no longer mistaken for a borrow. The corpus case (`buf.AsSpan(0,n)` — the genuine BCL extension) still fires; behaviour on real BCL spans is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
This was referenced Jun 22, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Тяжёлая артиллерия: the borrow checker's first bite on real C#
The B4 / P-007 POOL002 frontier — the differentiating lever (lifetime/aliasing analysis, Rust's home turf, where the flat "disposed anywhere?" scanners don't go).
The flat pass and the existing pool flow only caught a use-after-return when the buffer local itself was referenced after
Return. When the read goes through a stored Span view, the buffer name never appears at the use site — so it looked released-and-untouched. A miss, and a nasty silent aliasing bug (the array is recycled to another caller, so the write corrupts someone else's data):How
A
Span/ReadOnlySpanview of a tracked buffer is modelled as a borrow: a reference to the view local lowers to a use of the owner.ViewOwnerOfresolves the view through its own declaration's initializer (buf.AsSpan(..)/new Span<T>(buf)), with the Span-returning symbol confirmed via theSemanticModel. A use afterReturn(buf)then trips OWN002.Purely extractor-side — the core needs no borrow concept; it sees a plain use-after-release (emitted facts are identical to
arraypool-use-after-return:acquire buf; release buf; use buf). A ref-structSpancannot escape the method, which is exactly what makes "use of the view = use of the owner, here" sound.Precision (0 FP): a view of an untracked/escaped buffer, or one used before the
Return(after.cs), adds no finding — the borrow only lowers to a use of an owner that is still tracked, so it can never invent a release.Pinned
New corpus case
arraypool-span-view-after-return(before → OWN002,aftersilent), lifting the benchmark recall floor 11 → 12. Validated locally:case.own → OWN002, corpus 10/10, constructed extractor facts → OWN002.First slice:
Span/ReadOnlySpanviaAsSpan()/new Span<T>. Deliberately left for later rounds:Memory<T>(which can escape the method — needs escape modelling) and view reassignment.🤖 Generated with Claude Code
https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
Generated by Claude Code
Summary by CodeRabbit
New Features
Span/ReadOnlySpanviews created from pooled buffers, when the buffer is returned before the view is used.Documentation
Chores