[tests] Detect GC.KeepAlive kept-alive objects reached via a dup in HandleSafety - #26214
Conversation
…andleSafety
The C# compiler often compiles this (safe) pattern:
var obj = ...;
DoSomethingWith (obj.GetHandle ());
GC.KeepAlive (obj);
by emitting a `dup` for `obj` instead of storing it in a local: one copy
feeds `GetHandle`, the other stays on the stack until `GC.KeepAlive`
consumes it. The HandleSafety test traced the dup'd value back to the
instruction that produced it and, when that was a method call, reported
"the object was never stored anywhere" - a false positive, since the
object is explicitly kept alive.
Teach the test to treat a handle fetch as safe when the same object is
passed to `GC.KeepAlive` somewhere in the method. This resolves
`CFProxy.get_ProxyType` and nine other entries that already had a
`GC.KeepAlive` but were still flagged:
* AVFoundation.AVCaptureReactionType_Extensions.GetSystemImage
* CoreGraphics.CGColor.Create
* CoreText.CTFontDescriptor.Create
* CoreVideo.CVImageBuffer.GetCodePoint (x3)
* Security.SecKeyChain.Remove
* Security.SecKeyChain.Update
* UIKit.UIContentSizeCategoryExtensions.Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: dd86cf03-297a-43e8-ae1f-3c0b22bb67c4
There was a problem hiding this comment.
Pull request overview
Updates the Cecil-based HandleSafety test to recognize a common compiler-emitted dup pattern where an object’s handle is fetched and the object is later kept alive via GC.KeepAlive, eliminating false positives in handle-lifetime analysis.
Changes:
- Added logic in
HandleSafetyto treat handle fetches as safe when the same object is passed toGC.KeepAlive. - Removed multiple entries from
HandleSafety.KnownFailuresthat were previously (incorrectly) flagged despite havingGC.KeepAlive.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/cecil-tests/HandleSafety.cs | Adds GC.KeepAlive awareness to suppress false positives for dup-based patterns (needs a small fix to ensure KeepAlive is after the handle fetch). |
| tests/cecil-tests/HandleSafety.KnownFailures.cs | Removes failures that are no longer expected after the improved analysis. |
…eSafety Address review feedback: IsKeptAlive scanned the entire method body, so a GC.KeepAlive call that occurs *before* the handle fetch would incorrectly be treated as safe, hiding real lifetime issues. Restrict the search to GC.KeepAlive calls that occur after the handle fetch instruction, since a call before the fetch doesn't keep the object alive while its handle is in use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dd86cf03-297a-43e8-ae1f-3c0b22bb67c4
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🚀 [CI Build #8806526] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 203 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
The C# compiler often compiles this (safe) pattern:
by emitting a
dupforobjinstead of storing it in a local: one copyfeeds
GetHandle, the other stays on the stack untilGC.KeepAliveconsumes it. The HandleSafety test traced the dup'd value back to the
instruction that produced it and, when that was a method call, reported
"the object was never stored anywhere" - a false positive, since the
object is explicitly kept alive.
Teach the test to treat a handle fetch as safe when the same object is
passed to
GC.KeepAlivesomewhere in the method. This resolvesCFProxy.get_ProxyTypeand nine other entries that already had aGC.KeepAlivebut were still flagged:Copilot-Session: dd86cf03-297a-43e8-ae1f-3c0b22bb67c4