RemoteExec ignores AV's and CLR crashes

[danmoseley]

Run tests like this:

        [Fact]
        public static unsafe void AV()
        {
            RemoteExecutor.Invoke(() => 
            {
                *(int*)0x10000 = 0;
            }).Dispose();
        }

        [Fact]
        public static unsafe void FatalRuntimeError()
        {
            RemoteExecutor.Invoke(() =>
            {
                System.Runtime.InteropServices.Marshal.StructureToPtr(1, new IntPtr(1), true);
            }).Dispose();
        }

result:

  Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
     at System.Text.Tests.StringBuilderTests+<>c.<AV>b__168_0()
     at System.RuntimeMethodHandle.InvokeMethod(System.Object, System.Object[], System.Signature, Boolean, Boolean)
     at System.Reflection.MethodBase.Invoke(System.Object, System.Object[])
     at Microsoft.DotNet.RemoteExecutor.Program.Main(System.String[])
  Fatal error. Internal CLR error. (0x80131506)
     at System.Runtime.InteropServices.Marshal.StructureToPtr(System.Object, IntPtr, Boolean)
     at System.Runtime.InteropServices.Marshal.StructureToPtr[[System.Int32, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](Int32, IntPtr, Boolean)
     at System.Text.Tests.StringBuilderTests+<>c.<FatalError>b__169_0()
     at System.RuntimeMethodHandle.InvokeMethod(System.Object, System.Object[], System.Signature, Boolean, Boolean)
     at System.Reflection.MethodBase.Invoke(System.Object, System.Object[])
     at Microsoft.DotNet.RemoteExecutor.Program.Main(System.String[])
     ...
    Finished:    System.Runtime.Tests
  === TEST EXECUTION SUMMARY ===
     System.Runtime.Tests  Total: 36214, Errors: 0, Failed: 0, Skipped: 11, Time: 10.167s
  ----- end Thu 07/30/2020 13:48:03.15 ----- exit code 0 ----------------------------------------------------------

Ideally this should detect that the process terminated. In this case, there is no return code. Perhaps, RemoteExec should wrap the test MethodInfo somehow with one that returns 42 (if the invoked method doesn't return a code) and always verify that comes back.

This might mean we could eliminate CheckExitCode, or at least no longer wipe out CheckExitCode when passed an Action<> or Func:
https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.RemoteExecutor/src/RemoteExecutor.cs#L84

[danmoseley]

fyi @stephentoub

[danmoseley]

Workaround is to always return an exit code, eg:

        [Fact]
        public static unsafe void FatalError()
        {
            RemoteExecutor.Invoke(() =>
            {
                System.Runtime.InteropServices.Marshal.StructureToPtr(1, new IntPtr(1), true); // will AV
                return RemoteExecutor.SuccessExitCode;
            }).Dispose();
        }

[aik-jahoda]

I already did similar fix: #4927 The problem was that async invoke without return value swallows exceptions. The workaround could be same as yours.

My personal opinion is to remove CheckExitCode at all and drive the failure by exceptions + process termination.

If the test depends on exit code, it would be natural to assert the return value instead let RemoteExecutor do some magic.