Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finally block belonging to unexecuted try runs anyway #29481

Closed
jkotas opened this Issue Aug 23, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@jkotas
Copy link
Member

jkotas commented Aug 23, 2018

From @jakobbotsch on August 23, 2018 12:50

The following program prints something in release even though it shouldn't:

// Debug: Prints 0 lines
// Release: Prints 1 line
using System;

public class Program
{
    public static void Main()
    {
        try
        {
            bool b = false;
            if (b)
            {
                try
                {
                    return;
                }
                finally
                {
                    Console.WriteLine("Prints");
                }
            }
            else
            {
                return;
            }
        }
        finally
        {
            GC.KeepAlive(null);
        }
    }
}

Disassembly is:

; Assembly listing for method Program:Main()
; Emitting BLENDED_CODE for X64 CPU with AVX
; optimized code
; rbp based frame
; fully interruptible
; Final local variable assignments
;
;  V00 OutArgs      [V00    ] (  1,  1   )  lclBlk (32) [rsp+0x00]
;  V01 PSPSym       [V01    ] (  1,  1   )    long  ->  [rbp-0x10]   do-not-enreg[X] addr-exposed
;
; Lcl frame size = 48

G_M5092_IG01:
       55                   push     rbp
       4883EC30             sub      rsp, 48
       488D6C2430           lea      rbp, [rsp+30H]
       488965F0             mov      qword ptr [rbp-10H], rsp

G_M5092_IG02:
       48B968302ED59B010000 mov      rcx, 0x19BD52E3068
       488B09               mov      rcx, gword ptr [rcx]
       E850FCFFFF           call     System.Console:WriteLine(ref)
       90                   nop

G_M5092_IG03:
       33C9                 xor      rcx, rcx
       E8B85B1E5E           call     System.GC:KeepAlive(ref)
       90                   nop

G_M5092_IG04:
       488D6500             lea      rsp, [rbp]
       5D                   pop      rbp
       C3                   ret

G_M5092_IG05:
       55                   push     rbp
       4883EC30             sub      rsp, 48
       488B6920             mov      rbp, qword ptr [rcx+32]
       48896C2420           mov      qword ptr [rsp+20H], rbp
       488D6D30             lea      rbp, [rbp+30H]

G_M5092_IG06:
       33C9                 xor      rcx, rcx
       E8985B1E5E           call     System.GC:KeepAlive(ref)
       90                   nop

G_M5092_IG07:
       4883C430             add      rsp, 48
       5D                   pop      rbp
       C3                   ret

; Total bytes of code 79, prolog size 14 for method Program:Main()
; ============================================================

Copied from original issue: dotnet/coreclr#19627

@jkotas

This comment has been minimized.

Copy link
Member Author

jkotas commented Aug 23, 2018

From @AndyAyersMS on August 23, 2018 15:11

This looks like it might be a bug in the C# compiler's optimizer. The IL for Main (from 2.10.0.63119 (03b68dc)) (with /o+) is:

.method public hidebysig static void  Main() cil managed
{
  .entrypoint
  // Code size       23 (0x17)
  .maxstack  1
  .try
  {
    IL_0000:  ldc.i4.0
    IL_0001:  pop
    .try
    {
      IL_0002:  leave.s    IL_0016
    }  // end .try
    finally
    {
      IL_0004:  ldstr      "Prints"
      IL_0009:  call       void [System.Console]System.Console::WriteLine(string)
      IL_000e:  endfinally
    }  // end handler
  }  // end .try
  finally
  {
    IL_000f:  ldnull
    IL_0010:  call       void [System.Private.CoreLib]System.GC::KeepAlive(object)
    IL_0015:  endfinally
  }  // end handler
  IL_0016:  ret
} // end of method Program::Main

cc @jaredpar @VSadov

@jkotas

This comment has been minimized.

Copy link
Member Author

jkotas commented Aug 23, 2018

From @jakobbotsch on August 23, 2018 15:18

I see, I should've checked. Should I move this over to https://github.com/dotnet/roslyn?

@jkotas

This comment has been minimized.

Copy link
Member Author

jkotas commented Aug 23, 2018

From @VSadov on August 23, 2018 16:11

Looks like Roslyn bug.
Something is wrong with dead code elimination. A known to be false condition is eliminated but the consequence block is still emitted - as a fallthrough. So it runs when it should not.

@jkotas

This comment has been minimized.

Copy link
Member Author

jkotas commented Aug 23, 2018

From @jakobbotsch on August 23, 2018 17:40

I don't think this is related to dead code elimination. This was the original example:

// Debug: Outputs True
// Release: Outputs False
public class Program
{
    static bool s_1 = true;
    static bool[] s_2 = new bool[]{false};
    static long s_6;
    public static void Main()
    {
        M0();
        System.Console.WriteLine(s_1);
    }

    static void M0()
    {
        try
        {
            if (s_2[0])
            {
                try
                {
                    return;
                }
                finally
                {
                    s_1 = s_2[0];
                }
            }
            else
            {
                return;
            }
        }
        finally
        {
            s_6 = s_6;
        }
    }
}
@AlekseyTs

This comment has been minimized.

Copy link
Contributor

AlekseyTs commented Aug 24, 2018

It looks like VB has similar problem:

Imports System

Class C
    Shared Sub Main()
        try
            Dim b As Boolean = false
            if b
                try
                    return
                finally
                    Console.WriteLine("Prints")
                end try
            else
                return
            end if
        finally
            GC.KeepAlive(Nothing)
        end try
    End Sub
End Class

AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Aug 24, 2018

AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Aug 24, 2018

AlekseyTs added a commit that referenced this issue Sep 5, 2018

@AlekseyTs AlekseyTs closed this Sep 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.