dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.96k stars 4.02k forks source link

Roslyn codegen impacting array variance exception behavior #73215

Closed stephentoub closed 1 month ago

stephentoub commented 5 months ago

This code throws an ArrayTypeMismatchException:

Set(new Program[1]);

static void Set(object[] array)
{
    ref var o = ref array[0];
    Use(ref o);

    static void Use(ref object o) { }
}

SharpLab

; Method Program:<<Main>$>g__Set|0_0(System.Object[]) (FullOpts)
G_M000_IG01:                ;; offset=0x0000
       sub      rsp, 40

G_M000_IG02:                ;; offset=0x0004
       xor      edx, edx
       mov      r8, 0x7FFA8E1A5FA8
       call     CORINFO_HELP_LDELEMA_REF
       nop      

G_M000_IG03:                ;; offset=0x0016
       add      rsp, 40
       ret      
; Total bytes of code: 27

But this code does not:

Set(new Program[1]);

static void Set(object[] array)
{
    ref var o = ref array[0];
}

SharpLab

; Method Program:<<Main>$>g__Set|0_0(System.Object[]) (FullOpts)
G_M000_IG01:                ;; offset=0x0000
       sub      rsp, 40

G_M000_IG02:                ;; offset=0x0004
       cmp      dword ptr [rcx+0x08], 0
       jbe      SHORT G_M000_IG04

G_M000_IG03:                ;; offset=0x000A
       add      rsp, 40
       ret      

G_M000_IG04:                ;; offset=0x000F
       call     CORINFO_HELP_RNGCHKFAIL
       int3     
; Total bytes of code: 21

By eliding the CORINFO_HELP_LDELEMA_REF call that does the variance checking, does that mean the JIT's optimizations are changing program behavior in a way we try not to?

dotnet-policy-service[bot] commented 5 months ago

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

SingleAccretion commented 5 months ago

This is due to a difference in IL generated by Roslyn.

static void Set(object[] array)
{
    ref var o = ref array[0];
}

Uses ldelem instead of ldelema:

IL to import:
IL_0000  03                ldarg.1
IL_0001  16                ldc.i4.0
IL_0002  9a                ldelem.ref
IL_0003  26                pop
IL_0004  2a                ret
EgorBo commented 5 months ago

And Roslyn uses ldelema for the same C# snippet compiled in Debug

stephentoub commented 5 months ago

I hadn't noticed that IL difference. I assume that means we'd consider this a Roslyn bug and it should always be using ldelema?

SingleAccretion commented 5 months ago

Depending on what the C# specification says, yes. It's not a runtime bug in any case.