Async state machine incorrect translation

[iskiselev]

We have one more issue with incorrect goto translation in Roslyn compiled state machine. Issue only reproducible within assemblies compiled with optimizations.
Test case:

//@useroslyn
//@compileroption /optimize

using System;
using System.Threading;
using System.Threading.Tasks;

public static class Program
{
    public static void Main(string[] args)
    {
        var signal = new ManualResetEventSlim(false);

        var task = AsyncMethod(signal);

        signal.Wait();
    }

    public static async Task AsyncMethod(ManualResetEventSlim resetEvent)
    {
        await new Test().FailedMethod();
        resetEvent.Set();
    }
}

public class Test
{
    public async Task FailedMethod () {
        using (await GetLockObject()) {
        }

        using (await GetLockObject()) {
            await DoWork();
        }
    }


    private async Task DoWork()
    {
        Console.WriteLine("work");
    }

    public async Task<IDisposable> GetLockObject()
    {
        Console.WriteLine("Lock");
        return new LockObject();
    }
}

public class LockObject : IDisposable
{
    public void Dispose()
    {
        Console.WriteLine("Dispose");
    }
}

Method that translated incorrect:

    public async Task FailedMethod () {
        using (await GetLockObject()) {
        }

        using (await GetLockObject()) {
            await DoWork();
        }
    }

Expected output: Lock\nDispose\nLock\nwork\nDispose
Actualoutput: Lock\nwork\nDispose

[kg]

Hm, that's an interesting one. Thanks for spotting it

[iskiselev]

Based on ILSpy decompilation, we try to jump into middle of switch. Looks like, it is root cause of problem.
Interesting, that ILSpy newdecompiler branch provides correct control order, but they fully rewrote everything there.
Probably we still can patch JSIL to generate correct source with current branch.

[kg]

Might be a case where it would be easier to stop relying on ILSpy for control flow, and just do asm.js-style goto for anything other than if statements and for loops.

[iskiselev]

Look like I was able to simplify example. Test case:

//@useroslyn
//@compileroption /optimize

using System;

public static class Program {
    public static void Main (string[] args) {
        MoveNext();
    }


    private static void MoveNext () {
        int state = -1;
        switch (state) {
            case 2:
                goto LABEL_2;
            case 0:
                state = -1;
                break;
            case 1:
                state = -1;
                goto LABEL_1;
            default:
                Console.WriteLine("1");
                break;
        }
        Console.WriteLine("2");

        LABEL_1:
        Console.WriteLine("3");

        LABEL_2:
        Console.WriteLine("4");
    }
}

Output:

  function Program_MoveNext () {

    var $label0 = 0;
  $labelgroup0: 
    while (true) {
      switch ($label0) {
        case 0: /* $entry0 */ 
          switch (-1) {
            case 0: 
              break;

            case 1: 
              $label0 = 3 /* goto IL_32 */ ;
              continue $labelgroup0;

            case 2: 
              $label0 = 1 /* goto IL_3C */ ;
              continue $labelgroup0;

            default: 
              $T01().WriteLine("1");
              break;

          }
          $label0 = 2 /* goto $switchExit0 */ ;
          continue $labelgroup0;

        case 1: /* IL_3C */ 
          $T01().WriteLine("4");
          return;
          $label0 = 2 /* goto $switchExit0 */ ;
          continue $labelgroup0;

        case 2: /* $switchExit0 */ 

          $label0 = 3 /* goto IL_32 */ ;
          continue $labelgroup0;
        case 3: /* IL_32 */ 
          $T01().WriteLine("3");
          $label0 = 1 /* goto IL_3C */ ;
          continue $labelgroup0;

      }
    }
    /* Original label $exit0 */ 
    $T01().WriteLine("2");
  }; 

So, it skipped Console.WriteLine("2");

[iskiselev]

After ILSpy it looks like:

private static void MoveNext2()
{
	switch (-1)
	{
	case 0:
		break;
	case 1:
		goto IL_32;
	case 2:
		IL_3C:
		Console.WriteLine("4");
		return;
	default:
		Console.WriteLine("1");
		break;
	}
	Console.WriteLine("2");
	IL_32:
	Console.WriteLine("3");
	goto IL_3C;
}

So, once again we try to do goto inside switch block. Looks like LabelAnalyzer do not expect, that jump inside JSSwitchCase, JSIfStatement or JSLoopStatement possible.
Interesting, is it possible to fix it it in LabelAnalyzer? It is still too hard for me to understand it today. May be will try to look on it again some time later.

[kg]

It seems like maybe LabelAnalyzer could merge the labels for case 2 and IL_3C at the very least. Not sure if that is a generalized solution though.