feat(P-007): pin POOL005 full-length view stored into a field; defer object-level escape#206
Conversation
…ead) Issue #198, slice (a): a full-length view of a pooled buffer cached into another field (`_view = _buf.AsMemory()`, a Memory<T>/ReadOnlyMemory<T> field — Span<T> is a ref struct and cannot be a field) and read in a later member reaches past the logical rented length into the stale [n, Length) tail. Verified end to end with the real Roslyn extractor (a local .NET SDK, not CI-only): the POOL005 field pass fires on the full-length view *expression*, so the RHS of the store is itself caught — OWN025 at the store, on both the Memory and ReadOnlyMemory spellings; the bounded fix (`AsMemory(0, _n)`) is silent. No extractor change was needed; this turns that already-correct behaviour into a pinned contract and closes the P-007 "left next" note. * corpus/real-world/arraypool-view-into-field-overread/ — before.cs (OWN025 on both field stores) / after.cs (bounded, silent — the zero-FP specificity half), case.own (OWN025 reduction), expected-diagnostics.txt, notes.md. Confirmed via own-check.sh --format sarif (before -> [OWN025], after -> []). * tests/test_ownir.py — pins the OwnIR fact shape the extractor emits for the store (pool-tagged acquire/overspan/release -> OWN025), the frozen target. * docs/proposals/P-007-arraypool-span.md + the field-fullspan notes — updated. * tests/fixtures/cfg_parity.json — regenerated for the new case.own (the #197 CFG-parity ratchet; Rust own-cfg round-trips it). Slice (b), deeper POOL004 escape: the one shape past this — a *bounded* view cached in a field, read after the owner is Return'ed in a *different* member — is an object-level, cross-member escape whose bug-ness depends on the caller's method-call order (Setup->Done->Late vs Setup->Late->Done). That needs interprocedural ordering the per-method + field passes lack, so flagging it would be a soft false positive. Deferred with evidence as a follow-up (#205), per the issue's deferral clause; the intraprocedural escapes (return-after-return, using-owner-return) are already covered and stay green. Closes #198 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://cdn.github.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: 2193d77399
ℹ️ 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".
| Fill(_buf, n); | ||
| _view = _buf.AsMemory(0, _n); // FIX: bounded to the logical length | ||
| _metaLen = 8; | ||
| _metaView = _meta.AsMemory(0, _metaLen); // FIX: bounded |
There was a problem hiding this comment.
In the fixed corpus case, Capture() bounds _metaView to 8 bytes but never writes _meta[0.._metaLen) after renting _meta from ArrayPool. For any Capture(); FlushMeta() call, FlushMeta() still copies stale bytes from a previous renter, so the after.cs file labeled as the clean specificity half is still an information disclosure; initialize the metadata slice before storing/exposing the view, as the _buf path does with Fill(_buf, n).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 9de7cbd. _meta is now written with Fill(_meta, _metaLen) before the bounded view is cached, in both before.cs and after.cs, so the "clean fix" half no longer exposes uninitialized pooled bytes and the "valid payload + stale tail" story holds. Checker behaviour is unchanged: before.cs still raises OWN025 on both full-length field stores, after.cs stays silent (verified via own-check.sh --format sarif).
Generated by Claude Code
…review) The view-into-field fixture rented `_meta` but never wrote its valid bytes, so after.cs (the "clean fix" specificity half) still exposed 8 uninitialized pooled bytes via FlushMeta — a latent stale read in a file meant to be genuinely correct. Write `_meta[0.._metaLen)` (Fill) before caching the bounded view, mirroring the `_buf`/Fill path, in both before.cs and after.cs so the "valid payload + stale tail" story holds. No checker-behaviour change: before still raises OWN025 on both full-length field stores, after stays silent (verified via own-check.sh --format sarif). case.own is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
2 participants
Что и зачем
Issue #198, оставшиеся Pool/Span-срезы P-007. Проверив поведение против реального Roslyn-экстрактора локально (поднял .NET SDK в песочнице — не только CI), обнаружил, что срез (a) уже срабатывает: POOL005 field-pass ловит полноразмерный вид по выражению, поэтому правая часть присваивания в поле
_view = _buf.AsMemory()сама является таким выражением → OWN025 на месте сохранения (и дляMemory<T>, и дляReadOnlyMemory<T>;Span<T>— ref struct и полем быть не может). Ограниченный фикс (AsMemory(0, _n)) молчит. Изменений вProgram.csне потребовалось — PR превращает это уже-корректное поведение в закреплённый контракт и закрывает пометку P-007 «left next». Precision-first: срез (b) — object-level cross-member escape — вынесен в отдельный issue (см. ниже), а не форсирован ложным срабатыванием.Состав:
corpus/real-world/arraypool-view-into-field-overread/— before.cs (OWN025 на обоих сохранениях вида в поле) / after.cs (ограниченный вид, молчит — specificity-половина zero-FP гейта) / case.own (редукция → OWN025) / expected-diagnostics.txt / notes.md. Провереноown-check.sh --format sarif: before →[OWN025], after →[].tests/test_ownir.py— закрепляет форму OwnIR-фактов, которую экстрактор эмитит для сохранения (pool-тегированныйacquire/overspan/release→ OWN025) — замороженная цель, как советовал в брифе.docs/proposals/P-007-arraypool-span.md+ notesarraypool-field-fullspan-overread— обновлены.tests/fixtures/cfg_parity.json— перегенерирован под новыйcase.own(ратчет CFG-паритета из P-022 step 3: port own-cfg (AST → CFG lowering) to Rust with exact differential parity #197; Rustown-cfground-trip'ит его).Срез (b), deeper POOL004 escape: единственная форма за этой границей — ограниченный вид, закэшированный в поле и прочитанный после
Returnв другом методе — это object-level cross-member escape, чья баговость зависит от порядка вызовов у вызывающего (Setup→Done→Late— баг;Setup→Late→Done— норма). Это требует межпроцедурного порядка, которого у per-method + field пассов нет, так что флаг был бы soft FP. Отложено с доказательством как #205 (по клаузе отсрочки в issue). Внутрипроцедурные escapes (return-after-return, using-owner-return) уже покрыты и остаются зелёными.Про CI: PR не трогает
.github/workflows/ci.yml— новый corpus-кейс подхватывается существующимиcorpus-benchmarkиtest_corpusавто-дискавери, так что с #204 (который правит ci.yml) конфликта по файлу нет.Тип изменения
Как проверено
python tests/run_tests.py— зелено (corpus 32/32 вкл. новый кейс; CFG parity 67; ownir 249/249)ruff check .иmypy— чистоown-check.sh --format sarifbefore→[OWN025], after→[]; и OK-контрасты (bounded, non-pool, return-after-use) молчат / без ложного OWN002cd rust && cargo test—own-cfground-trip'ит новыйcase.ownСвязанные issue
Closes #198
Follow-up: #205 (POOL004 object-level cross-member escape — deferred)
Чеклист
feat:)🤖 Generated with Claude Code
https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG
Generated by Claude Code