Constrained execution behavior change in .net7 preview 6

[jaredpar]

Description

The behavior around constrained execution calls observably changed between .net7 preview 5 and preview 6. This was caught by a regression test in the roslyn repository: dotnet/roslyn#63221.

This is an old regression test tracked by the following work item. https://vstfdevdiv/DevDiv2/DevDiv/_workitems/edit/1021941

Reproduction Steps

Create a Visual Basic console application with the following code:

Imports System

Interface IMoveable
    Property Position As Integer
End Interface

Class Item
    Implements IMoveable

    Public Property Name As String

    Public Property Position As Integer Implements IMoveable.Position
        Get
            Console.WriteLine("Position get for item '{0}'", Me.Name)
            Return 0
        End Get
        Set
            Console.WriteLine("Position set for item '{0}'", Me.Name)
        End Set
    End Property
End Class

Class Program
    Shared Sub Main()
        Dim item = New Item With {.Name = "Goo"}
        Shift(item)
    End Sub

    Shared Sub Shift(Of T As {Class, IMoveable})(item As T)
        item.Position += GetOffset(item)
    End Sub

    Shared Function GetOffset(Of T)(ByRef item As T) As Integer
        item = DirectCast(DirectCast(New Item With {.Name = "Bar"}, IMoveable), T)
        Return 0
    End Function
End Class

And the following project file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <RootNamespace>vbconsole</RootNamespace>
    <TargetFrameworks>net6.0;net7.0</TargetFrameworks>
  </PropertyGroup>

</Project>

Expected behavior

C:\Users\jaredpar\code\temp\vbconsole
> dotnet run --configuration Release --framework net6.0
Position get for item 'Goo'
Position set for item 'Goo'
C:\Users\jaredpar\code\temp\vbconsole 
> dotnet run --configuration Release --framework net7.0
Position get for item 'Goo'
Position set for item 'Goo'

Actual behavior

C:\Users\jaredpar\code\temp\vbconsole
> dotnet run --configuration Release --framework net6.0
Position get for item 'Goo'
Position set for item 'Goo'
C:\Users\jaredpar\code\temp\vbconsole 
> dotnet run --configuration Release --framework net7.0
Position get for item 'Goo'
Position set for item 'Bar'

Regression?

Yes this is a regression from net7.0 preview 5 and all previous .net runtime releases (core and framework).

Known Workarounds

No response

Configuration

No response

Other information

No response

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

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

Issue Details

---

Description

The behavior around constrained execution calls observably changed between .net7 preview 5 and preview 6. This was caught by a regression test in the roslyn repository: dotnet/roslyn#63221.

This is an old regression test tracked by the following work item. This link is unavailable at the moment, trying to track down the current link: http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1021941

Reproduction Steps

Create a Visual Basic console application with the following code:

Imports System

Interface IMoveable
    Property Position As Integer
End Interface

Class Item
    Implements IMoveable

    Public Property Name As String

    Public Property Position As Integer Implements IMoveable.Position
        Get
            Console.WriteLine("Position get for item '{0}'", Me.Name)
            Return 0
        End Get
        Set
            Console.WriteLine("Position set for item '{0}'", Me.Name)
        End Set
    End Property
End Class

Class Program
    Shared Sub Main()
        Dim item = New Item With {.Name = "Goo"}
        Shift(item)
    End Sub

    Shared Sub Shift(Of T As {Class, IMoveable})(item As T)
        item.Position += GetOffset(item)
    End Sub

    Shared Function GetOffset(Of T)(ByRef item As T) As Integer
        item = DirectCast(DirectCast(New Item With {.Name = "Bar"}, IMoveable), T)
        Return 0
    End Function
End Class

And the following project file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <RootNamespace>vbconsole</RootNamespace>
    <TargetFrameworks>net6.0;net7.0</TargetFrameworks>
  </PropertyGroup>

</Project>

Expected behavior

C:\Users\jaredpar\code\temp\vbconsole
> dotnet run --configuration Release --framework net6.0
Position get for item 'Goo'
Position set for item 'Goo'
C:\Users\jaredpar\code\temp\vbconsole 
> dotnet run --configuration Release --framework net7.0
Position get for item 'Goo'
Position set for item 'Bar'

Actual behavior

C:\Users\jaredpar\code\temp\vbconsole
> dotnet run --configuration Release --framework net6.0
Position get for item 'Goo'
Position set for item 'Goo'
C:\Users\jaredpar\code\temp\vbconsole 
> dotnet run --configuration Release --framework net7.0
Position get for item 'Goo'
Position set for item 'Goo'

Regression?

Yes this is a regression from net7.0 preview 5 and all previous .net runtime releases (core and framework).

Known Workarounds

No response

Configuration

No response

Other information

No response

Author:	jaredpar
Assignees:	-
Labels:	area-CodeGen-coreclr, untriaged
Milestone:	-

[jakobbotsch]

The previous behavior seems wrong to me based on the IL generated, which is the following:

IL_0000: ldarga.s  item
IL_0002: ldarga.s  item
IL_0004: constrained. !!T
IL_000A: callvirt  instance int32 vb_playground.IMoveable::get_Position()
IL_000F: ldarga.s  item
IL_0011: call      int32 vb_playground.Program::GetOffset<!!T>(!!0&)
IL_0016: add.ovf
IL_0017: constrained. !!T
IL_001D: callvirt  instance void vb_playground.IMoveable::set_Position(int32)
IL_0022: ret

Before preview 6 the call to set_Position is seeing the wrong instance of item. It is an object reference that is modified by the call to GetOffset, and the constrained call to set_Position dereferences the address loaded at IL_0000 which should see the modified instance. The previous behavior is due to a JIT bug where the JIT improperly reorders the indirection of the this byref for constrained calls with the rest of the arguments (in this case, loading item too early). The new behavior is a side effect of #66887; this PR caused the JIT to see slightly different runtime lookups which meant we now end up with the correct behavior. I believe the JIT bug still exists, so I will open an issue for that (edit: #73615).

FWIW, the same example translated to C# generates the following IL:

IL_0000: ldarga.s item
IL_0002: stloc.0
IL_0003: ldloc.0
IL_0004: ldloc.0
IL_0005: constrained. !!T
IL_000b: callvirt instance int32 IMoveable::get_Position()
IL_0010: ldarga.s item
IL_0012: call int32 Program::GetOffset<!!T>(!!0&)
IL_0017: add
IL_0018: constrained. !!T
IL_001e: callvirt instance void IMoveable::set_Position(int32)
IL_0023: ret

which looks more correct to me. (edit: actually, this still has the bug)

I guess two wrongs really do make a right ...

[jakobbotsch]

Though I'm not really sure what Roslyn can do here, I suppose making a copy would be wrong in the struct case. On the other hand I'm not sure what the JIT can do here either, there can be arbitrary trees between loading the address of the this variable and the constrained call which will cause the JIT to spill the arguments for any number of reasons.

[davidwrighton]

Updated the link the bug description such that it works. I'm taking a look at this now, trying to decipher what the intentions are here.

[davidwrighton]

Ok, so looking at this, I agree with @jakobbotsch, this appears to be incorrect codegen by Roslyn. Correct code generation for this would be something like...

ldarga.s  item
ldloca tempVar
initobj !!T
ldloc tempVar
box !!T
ldnull
ceq
brfalse hasAddressForPairedGetSetCalls
ldobj !!T
stloc tempForPairedGet
ldloca tempForPairedGet
hasAddressForPairedGetSetCalls: dup
IL_0004: constrained. !!T
IL_000A: callvirt  instance int32 vb_playground.IMoveable::get_Position()
IL_000F: ldarga.s  item
IL_0011: call      int32 vb_playground.Program::GetOffset<!!T>(!!0&)
IL_0016: add.ovf
IL_0017: constrained. !!T
IL_001D: callvirt  instance void vb_playground.IMoveable::set_Position(int32)
IL_0022: ret

Note: this design relies on the detail that Nullable implements no interfaces, and there are no constrained calls on Nullable which would need this sort of paired call.

[jakobbotsch]

C# example:

using System;
using System.Runtime.CompilerServices;

public interface IMoveable
{
    int Position { get; set; }
}

public class Item : IMoveable
{
    public string Name { get; set; }

    public int Position
    {
        get
        {
            Console.WriteLine("Position get for item '{0}'", Name);
            return 0;
        }
        set
        {
            Console.WriteLine("Position get for item '{0}'", Name);
        }
    }
}

public class Program
{
    public static void Main()
    {
        Item item = new Item { Name = "Goo" };
        Shift(item);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void Shift<T>(T item) where T : class, IMoveable
    {
        item.Position += GetOffset(ref item);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static int GetOffset<T>(ref T item)
    {
        item = (T)(IMoveable)(new Item { Name = "Bar" });
        return 0;
    }
}

[jakobbotsch]

F# may have a similar bug, though it does not have compound operators, so I'm not quite sure what the output is supposed to be.
@TIHan helped me translate to the F# source

open System
type IMoveable =
    abstract Position : int with get, set
   
type Item() =
    member val Name = "Bar" with get, set

    interface IMoveable with
        member this.Position
            with get() = Console.WriteLine $"Position get for item '{this.Name}'"; 0
            and set (value : int) = Console.WriteLine $"Position set for item '{this.Name}'"

module Program =
    let GetOffset<'T> (item : 'T byref) : int =
        item <- ((Item(Name = "Bar") :> IMoveable) :> obj) :?> 'T
        0

    let Shift<'T when 'T :> IMoveable and 'T : not struct> (item : 'T) =
        let mutable item = item
        item.Position <- item.Position + (GetOffset &item)

    [<EntryPoint>]
    let main (argv : string[]) =
        let item = Item(Name = "Goo")
        Shift(item)
        0

The generated IL for Shift is

            IL_0000: ldarg.0
            IL_0001: stloc.0
            IL_0002: ldloca.s 0
            IL_0004: ldloca.s 0
            IL_0006: constrained. !!T
            IL_000c: callvirt instance int32 _/IMoveable::get_Position()
            IL_0011: ldloca.s 0
            IL_0013: call int32 _/Program::GetOffset<!!T>(!!0&)
            IL_0018: add
            IL_0019: constrained. !!T
            IL_001f: callvirt instance void _/IMoveable::set_Position(int32)
            IL_0024: ret

which also prints "Goo" followed by "Bar" (when the JIT bug does not trigger).
@dsyme What is the expected behavior of the F# source code here?