[Bug] Invalid IL sequence emitted for re-track from pointer to byref

[hamarb123]

Roslyn does not emit ref *pointer as a gc re-track in some cases. This seems unideal & potentially problematic. The runtime depends on it turning into a byref local in some places such as https://github.com/dotnet/runtime/blob/main/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs#L120 (this gets emitted problematically as shown in the last section).

If it is intentional that this doesn't emit a re-track in IL, we should not be relying on it to do so in dotnet/runtime or elsewhere presumably, but I was certainly under the impression that this should re-track, rather than potentially just keep as an unmanaged pointer indefinitely. Otherwise, this is a bug in roslyn.

/cc @stephentoub @jkotas @jaredpar

Version Used: sharplab.io

Steps to Reproduce:

e.g.,

using System;
using System.Runtime.CompilerServices;

public class C
{
    public static unsafe void Main()
    {
        byte[] arr = [1];
        ref byte b = ref Unsafe.NullRef<byte>();
        fixed (byte* ptr = arr)
        {
            b = ref *ptr;
        }
        WeakReference wr = new(arr);
        arr = null;
        GC.Collect(int.MaxValue, GCCollectionMode.Forced, true, true);
        Console.WriteLine(wr.Target != null);
        b = 1;
    }
}

Sharplab

Diagnostic Id:

N/A

Expected Behavior:

Prints True, due to the byref local keeping it alive.

Actual Behavior:

There isn't a byref local in the IL at all, and prints False (most of the time).

Example of incorrect generated IL in dotnet/runtime as a result of this:

https://github.com/dotnet/runtime/blob/main/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs#L120

[jkotas]

I was certainly under the impression that this should re-track

I agree. It looks like a bug in the Roslyn local variable optimizations. Notice that it works as expected in Debug configuration.

[jaredpar]

Agree, looks like an optimization bug where see the ref local as a redundant write and remove it. That optimization was written before we had ref locals and guessing it doesn't fully account for this type of scenario.

[hamarb123]

That optimization was written before we had ref locals and guessing it doesn't fully account for this type of scenario.

You don't need explicit ref locals to get this to occur btw, it's just the easiest way.

using System;
using System.Runtime.InteropServices;

public class C
{
    static int X(GCHandle h)
    {
        h.Target = null;
        GC.Collect(int.MaxValue, GCCollectionMode.Forced, true, true);
        return 0;
    }
    
    static void W(ref byte b, int x, bool c)
    {
        Console.WriteLine(c);
        b = 1;
    }
    
    public static unsafe void Main()
    {
        byte[] arr = [1];
        byte* b = null;
        GCHandle h = GCHandle.Alloc(arr, GCHandleType.Pinned);
        fixed (byte* ptr = arr)
        {
            b = ptr;
        }
        WeakReference wr = new(arr);
        arr = null;
        W(ref *b, X(h), wr.Target != null);
    }
}

Sharplab

[jjonescz]

I couldn't find any details about this in the language spec, so I'm not sure the behavior you want is guaranteed by the language (then you'd be relying on an implementation detail if we fix this in Roslyn).

[jkotas]

We can always fix the spec to make the behavior of these implicit conversions defined.

I think the first example ref byte b = ... is no brainer. If there is ref local, it should be reported to the GC. Implicit conversion of unmanaged pointer to ref local has initiate-GC-reporting side-effect. This side-effect should not be moved or removed by optimizations where it can produce observable behavior changes. This one is important to fix. It is hard to write reliable low-level code in dotnet/runtime with the current behavior.

The second example W(ref *b, X(h), wr.Target != null) is more subtle. It has to do with evaluation of side-effects during argument passing. Given that C# guarantees that arguments are evaluated left-to-right, it would be more natural to say that the initiate-GC-reporting side-effect happens during argument evaluation. The current behavior where it happens after all arguments are evaluated is not end of the world. This one is less important to do something about.