Skip to content

Commit 35d2eed

Browse files
committed
extractor: a bare-LOCAL assignment rent is not a field rent (syntactic POOL001 pass)
Second Redis pooled-buffer FP: RedisServer scan does `RedisKey[] keys; … keys = ArrayPool<RedisKey>.Shared.Rent(count); … new ScanResult(cursor, keys, count, true)`. The buffer is transferred to ScanResult, which owns the Return (its Recycle() -> ArrayPool.Return, called by CursorEnumerable on ProcessReply/ Dispose) — confirmed in source. So OWN001 "rented but never returned" was a false positive. Root cause: the syntactic POOL001 pass classifies an assignment-target rent via FieldName(asg.Left), which is purely syntactic (returns the identifier text for ANY target). A bare local (`keys = Rent()`, keys pre-declared) was thus treated as a field-backed rent with isField=true and NO escape/transfer analysis. The pass is meant for real fields (rented in ctor, Returned class-wide in Dispose); gate the assignment arm on `model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol` so a bare-local assignment rent is not flagged here. Such a local belongs to the flow pass, which does escape analysis but does not track the assignment form — so it is honestly untracked rather than flagged unsoundly (precision-first; the rare bare-assign-then-dropped local leak is a recall gap, not a false positive). No sample regression (all sample assignment rents are `this.field = Rent()` fields or `var`/`T[] x = Rent()` declarations). Re-runs the Redis oracle to confirm RedisServer.cs:917 clears. corpus 31/31. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm
1 parent 4351f4a commit 35d2eed

2 files changed

Lines changed: 21 additions & 6 deletions

File tree

‎corpus/oracle-target.txt‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
# TEMPORARY (revert before any PR): fresh-repo oracle sweep for new FP classes / recall gaps.
77
# Target 1 (done, clean): DapperLib/Dapper — 0 own-only; oracle-only = Infer# WrappedBasicReader
88
# ownership-transfer FPs + CodeQL dispose-on-throw honest-skips. No new FP class.
9-
# Target 2: StackExchange/StackExchange.Redis — surfaced the pooled-buffer-transfer-via-escaping-
10-
# local FP class (Lease.cs:42, RedisServer.cs:917). RE-RUN to confirm the WrapperLocalEscapes fix
11-
# clears those 4 own-only findings (expect Lease.cs / RedisServer.cs `keys`/`arr` gone from own-only).
9+
# Target 2: StackExchange/StackExchange.Redis. WrapperLocalEscapes cleared Lease.cs:42 (var-decl
10+
# transfer). RedisServer.cs:917 `keys` was a bare-LOCAL assignment rent (`keys = Rent()`) mis-
11+
# classified as a field rent by the syntactic POOL001 pass (no escape analysis) — the ScanResult it
12+
# is handed to owns the Return (Recycle()). RE-RUN #2 to confirm the IFieldSymbol gate clears the
13+
# 3 RedisServer own-only findings (expect RedisServer.cs `keys` gone; own-only 9 -> 6).
1214
StackExchange/StackExchange.Redis

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4024,9 +4024,22 @@ or ImplicitObjectCreationExpressionSyntax
40244024
// --flow-locals; skip it here so it is not double-reported.
40254025
EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax vd }
40264026
=> (flowLocals ? null : vd.Identifier.Text, false),
4027-
// a field/assignment rent (`_buf = pool.Rent(...)`) is NOT a
4028-
// flow candidate, so this pass keeps it in both modes.
4029-
AssignmentExpressionSyntax asg => (FieldName(asg.Left), true),
4027+
// a FIELD-backed rent (`_buf = pool.Rent(...)`) is NOT a flow
4028+
// candidate — rented in one member, Returned class-wide in another
4029+
// (Dispose) — so this pass keeps it in both modes. GATED on the target
4030+
// being a real field: `FieldName` is purely syntactic, so a bare LOCAL
4031+
// assignment (`keys = pool.Rent(count)`, keys pre-declared) would otherwise
4032+
// be mis-classified as a field rent and flagged with NO escape/transfer
4033+
// analysis — a false positive when the local is handed to an escaping owner
4034+
// (mined on StackExchange.Redis RedisServer scan: `keys = ArrayPool<RedisKey>.
4035+
// Shared.Rent(count); … new ScanResult(cursor, keys, count, true)` — the
4036+
// ScanResult owns the Return via Recycle()). A bare-local assignment rent
4037+
// belongs to the flow pass (which does escape analysis); it does not track
4038+
// the assignment form, so such a rent is honestly not tracked here rather
4039+
// than flagged unsoundly.
4040+
AssignmentExpressionSyntax asg
4041+
when model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol
4042+
=> (FieldName(asg.Left), true),
40304043
_ => ((string?)null, false),
40314044
};
40324045
if (name != null)

0 commit comments

Comments
 (0)