JIT: unpin locals fed by non-GC typed values

[AndyAyersMS]

Treat a non-GC typed def (eg a native int reinterpreted as a byref, as in a Span built from a pointer) as non-movable, so the pinned local can be unpinned. Fixes #129993.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[AndyAyersMS]

@EgorBo PTAL
50-60 methods with diffs.

[Copilot]

Pull request overview

This PR updates the JIT’s “no-GC definition” classification used by pinned-local unpinning so that locals fed by certain non-GC-typed values can be treated as non-movable, enabling pin elision in more cases.

Changes:

• Expands GenTree::IsNotGcDef() to early-return true when the node’s TypeGet() is not GC-typed.

[EgorBo]

I assume this sort of marks any untracked GC pointer as UB even if previously it worked, like

fixed (... = Unsafe.AsPointer(gcRef))

cc @dotnet/jit-contrib in case if you see any concern

[SingleAccretion]

any concern

This is a breaking change. I don't see how it can be justified, since the pinned state of a value is a dynamic property. Here we only know that the value is pinned at the def point, but we need to know that it "would be" pinned for at least the lifetime the caller is concerned about.

[jkotas]

ECMA spec explicitly talks about "Start GC tracking" and "Stop GC tracking" for conversions.

[AndyAyersMS]

Yes this is too broad.

Seem like though that a non-gc parameter could possibly be handled here. If the parameter actually referred to GC memory whoever passed the argument would need to have pinned or they'd already be broken.

[SingleAccretion]

If the parameter actually referred to GC memory whoever passed the argument would need to have pinned or they'd already be broken.

I don't think parameters are special in any way. The pinning state can be dynamically altered for them just as for normal variables (through opaque means such as calls, inter-thread sync, GC handles, aliased pointers, etc).

[SingleAccretion]

It seems to me the analysis is also reliant on the "global" (or at least per-method) perspective, since a "true" return value is treated as representing a value that can't be made movable 'no matter what'. It would require a very sophisticated analysis indeed to prove that the pinned state of a value [that may be movable] doesn't change beyond its use in a def, in the general case.

[hamarb123]

As others have seemingly already mentioned here 😅: #129993 (comment)

[AndyAyersMS]

The global analysis is flow insensitive so def vs use ordering shouldn't matter?

[SingleAccretion]

The global analysis is flow insensitive so def vs use ordering shouldn't matter?

What I meant is basically this:

void* p = GetPin(); // 'p' pinned here via e. g. a handle, the virtual 'pin ref count' increases: 0 -> 1

pinned j = p; // It increases again: 1 -> 2
Unpin(p); // Decreases: 2 -> 1

// ...
// 'j's value still pinned here
// ...

return; // Decreases again: 1 -> 0, unpinned.

as the situation where the pinned state (analogous to a ref count) "changes beyond its use in a def [here the j = p def]". We know that the value represented by p at the point of the def must be pinned [otherwise we have UB], but beyond that point it must be proven by some other means.

[AndyAyersMS]

Thanks all. Didn't appreciate that we have these non-lexical pinning mechanisms which makes any kind of local inference unsound.

[MichalPetryka]

@MihuBot -nuget