fixed/pinned array escapes due to tail call

[luithefirst]

We have identified a scenario where an assumed pinned array is moved before being copied, resulting in an AccessVioloationException. It can be reproduced with net48, net8.0, and netstandard 2.0 when compiled with Tailcalls true (only tested on Windows), SDK version 9.0.203.

In particular, it escapes from an extension method like this:

static member Set<'T when 'T : unmanaged>(this : IBuffer, src : 'T[]) =
    use ptr = fixed src
    this.Set(ptr |> NativePtr.toNativeInt, nativeint src.Length * nativeint sizeof<'T>)

You can find a repository with a minimal example here:
https://github.com/luithefirst/TailCallEscape

It will crash in about 1 min. Originally, I could also see a TailCallHelperStub stack frame in WinDbg, but it is gone now in my minimal example. Still, if Tailcalls is set to false in the fsproj file, the issue cannot be reproduced.

A colleague also recalled a possible similar issue back in net 5, but I could not find any details on it.

This is where the issue actually occurs in our framework:
https://github.com/aardvark-platform/aardvark.rendering/blob/646d165829283a3a0ba03cb4badc465245fbc670/src/Aardvark.Rendering/Resources/Adaptive/ManagedBuffer.fs#L105
https://github.com/aardvark-platform/aardvark.base/blob/5b0b5c0507ca20238899260d2a08d252e6a4f1e8/src/Aardvark.Base.FSharp/Utilities/Interop/FSLibExtensions.fs#L529

There, it is implemented in a netstandard 2.0 library, and when consumed by net48 the issue occurs (but for some reason not in net8.0). Also, when disabling Tailcalls in the library, it can be avoided.

Expected behaviour: The compiler should be aware that a stackframe is pinning an object and therefore should not generate a tailcall.

Workaround: Use GCHandle + AddrOfPinnedObject

[vzarytovskii]

That should be relatively easy to fix, we have the "can tail" check in very few places. It should take the pinned info into account

[luithefirst]

Thanks for your reply, nice to hear!

For clarity, I've also outlined the original scenario where we originally discovered the issue: library with netstandard 2.0 (tailcalls=true) used in net48.

Edit: It also seems to occur in net8.0. Here are the different call stacks:

net48 + tailcall true:

TailCallEscape.Application+Buffer`1[[System.Int32, mscorlib]].Set(IntPtr, IntPtr)
TailCallEscape.Application.run2@62(System.Random, System.Collections.Generic.List`1<Int32[]>, Buffer`1, Microsoft.FSharp.Core.Unit)

net48 + tailcall false:

TailCallEscape.Application+Buffer`1[[System.Int32, mscorlib]].Set(IntPtr, IntPtr)
TailCallEscape.Application+BufferExtensions.Set[[System.Int32, mscorlib]](IBuffer, Int32[])
TailCallEscape.Application.run2@62(System.Random, System.Collections.Generic.List`1<Int32[]>, Buffer`1, Microsoft.FSharp.Core.Unit)

net8.0 + tailcall true:

TailCallEscape.Application+Buffer`1[[System.Int32, System.Private.CoreLib]].Set(IntPtr, IntPtr)
TailCallEscape.Application+Buffer`1[[System.Int32, System.Private.CoreLib]].TailCallEscape.Application.IBuffer.Set(IntPtr, IntPtr)
TailCallEscape.Application.run2@62(System.Random, System.Collections.Generic.List`1<Int32[]>, Buffer`1, Microsoft.FSharp.Core.Unit)

[hyazinthh]

Hello,

I've found a simpler repro for this issue. It seems like the fixed statement is broken in general with tail calls:

namespace App

open System
open FSharp.NativeInterop

module Program =
    let mutable dummy : int[] = null
    let mutable data : int[] = null

    let foo (address: nativeint) =
        dummy <- null
        GC.Collect()

        if address <> NativePtr.toNativeInt &&data.[0] then
            failwithf "Address changed"

    let bar() =
        use ptr = fixed data
        foo (NativePtr.toNativeInt ptr)

    [<EntryPoint>]
    let main _argv =
        dummy <- Array.zeroCreate<int> 100
        data <- Array.zeroCreate<int> 10
        bar()
        0

An easy workaround is to wrap the last expression to avoid the tail call:

match (foo (NativePtr.toNativeInt ptr)) with
| () -> ()

Tested with .NET SDK 8.0.411 and 9.0.300.

[T-Gro]

I wonder how the IL spec looks at it - but we are likely the only compiler running into that I assume.
i.e. would code that uses .tail before working with a pinned variable be treated as invalid IL ?

(and compiler should not generate that .tail here at all)

[vzarytovskii]

Oh, it's tail prefixes and in both runtimes. Must've been there forever. I wonder what does the ECMA spec say about it (if anything at all). But shouldn't change the fact that it should be easy to not add prefix if there are any pinned locals. It might, however, theoretically, result in SO for some code which was working before.

[T-Gro]

@jakobbotsch:

Hi Jakob, could you please help us out with identifying the right behavior?

We have a combination of a pinned local, a tail. instruction, and then a call to a method where the pinned local is passed. See the link below. With the tail., the pinning does not apply and address changes in this small repro. Without it, pinning works fine.

Question is:

• Is this invalid IL, and .tail must not be used?
• Is this part of the IL spec to describe how .tail and pinned should work together, if at all?
• Can this be a runtime bug?

https://sharplab.io/#v2:DYLgZgzgNALiBOBXAdgH2QQwLYFMIAcMBjHAAgEF98BYAKDoHt8dlSBlATwhhy0edYAxNgAsM8fADoAchhgBLAG44Aksh7wmdOlgYATRMDIAFTQHN42UgF46pe6SMxSWRDAwAjI6QNYsHUhBSeXUAbQBdG1JkQ2A7BycXN09vPTkMQOCwyOto2O1aB0ccZzAGBlIACgw9PXg8CCDMBWUQmABKG3ii+19/UgAeAFo84DjCnvsAcQBhSRmGMZwiGErOgHp10gAVEXkIaIZnDGAAdwwuHwY8UhgRMhh4eSIAayhSDzdb+A4Qs0d5C8yABmW7yXAHMwlA7yZwwCpESwQEQFSb2eRgUg1OoNQYAPlIshaOGMj0k8KJSlU6lIADJaWl3JJQgAGSJ3FjdNH2MAYeRnWEiTEAInItXqEAORDEyChemFqISJQ+4jWXQmk0QEDI+EeUTA8gAHjg9D50lyimUKpVKcpSfByQxbdTnLr4O0LQ41p77JtSABVZBEBh+Fhw+5YjwMZQ8hjwLEuHD1UgAAzWKYcIzuclI5wC5L5wGCB2QR1IvFhJsV9lCAwAouofsYGG08eEuYksHzWAB9cRmRTq7l9ALDCjwSwcSQALyTDBm9TkOAGrdIAEYWSyfWb3IMRuQJxcZ3OFzglyv1ASN9vtx5VR6NT0WUA

C# cannot express the .tail emission, the relevant block of the usage would be roughly this:

                    fixed (int* ptr = &array[0])
                    {
                        return foo((nint)ptr); //.tail call
                    }
                    

[jakobbotsch]

• Is this invalid IL, and .tail must not be used?

It is not invalid IL, but locals will not be pinned in the callee if it is tail. prefixed. It is similar to how it is up to the IL generator to ensure no pointer into the local stack frame remains if it emits tail. The IL generator must also ensure that pinning is not necessary inside callees (concretely for F# that would probably mean that the fixed was in some different scope).

• Is this part of the IL spec to describe how .tail and pinned should work together, if at all?

It doesn't seem to be very clearly spelled out, but II.7.1.2 says

While a method with a pinned local variable is executing, the VES shall not relocate the
object to which the local refers.

while tail. is described as

tail. (prefix) – call terminates current method

which I think covers what happens here.

• Can this be a runtime bug?

I think the behavior is expected. The runtime is required to honor tail. when possible and it does so here. It is up to the IL generator to only emit tail. when removing the current stack frame is ok (i.e. it should be ok to unpin and there shouldn't be addresses to current locals).