Fix extra stack frame on exception stack trace

[janvorli]

A recently added diagnostic test has revealed that throwing an exception from a funclet results in an extra frame on the stack trace that should not be there.
This change fixes it in a manner equivalent to how the old EH handles that.

[jkotas]

Why do we want NATIVEAOT behavior to be different?

[janvorli]

I am actually not sure if the issue occurs with NativeAOT too. It uses a different way of filtering frames in the UpdateStackTrace based on equal frame pointers, which doesn't work on coreclr. So I've made the change for coreclr, but plan to investigate the nativeaot case.

[jkotas]

What does the diagnostic test that this fixing do?

[janvorli]

It is the NestedException2 test. Basically, there is this code:

        public static void Foo()
        {
            try
            {
                Console.WriteLine("Foo calling Bar");
                ExceptionsTestHelper.Bar<int>(1);
            }
            catch (Exception inner)
            {
                throw new Exception("doh", inner);
            }
        }
    }

The ExceptionsTestHelper.Bar<int>(1); just throws InvalidOperationException. The issue is that the Exception thrown in the code above has the line of ExceptionsTestHelper.Bar<int>(1); on the stack trace.

[jkotas]

The issue is that the Exception thrown in the code above has the line of ExceptionsTestHelper.Bar(1); on the stack trace.

I see the My.Main() line duplicated:

C:\runtime\artifacts\bin\coreclr\windows.x64.Release>corerun test.exe
Foo calling Bar
Unhandled exception. System.Exception: doh
 ---> System.InvalidOperationException: Operation is not valid due to the current state of the object.
   at My.Bar[T](Int32 x)
   at My.Main()
   --- End of inner exception stack trace ---
   at My.Main()

C:\runtime\artifacts\bin\coreclr\windows.x64.Release>set DOTNET_LegacyExceptionHandling=0

C:\runtime\artifacts\bin\coreclr\windows.x64.Release>corerun test.exe
Foo calling Bar
Unhandled exception. System.Exception: doh
 ---> System.InvalidOperationException: Operation is not valid due to the current state of the object.
   at My.Bar[T](Int32 x)
   at My.Main()
   --- End of inner exception stack trace ---
   at My.Main()
   at My.Main()

There is ifdefed-out code in UpdateStackTrace that seems to be dealing with this situation:

runtime/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs

Lines 966 to 968 in 68f7885

	#if NATIVEAOT
	if ((prevFramePtr == UIntPtr.Zero) || (curFramePtr != prevFramePtr))
	#endif

Can this bug be fixed by deleting the ifdef in UpdateStackTrace?

[janvorli]

No, that's what I've mentioned above. The frame pointer check doesn't work in coreclr, since there are cases where two non-funclet methods have the same frame pointer - when they don't use Rbp as a frame pointer and use just Rsp instead. I've actually attempted to fix the problem by removing the ifdef, thinking I was originally wrong. But then another diagnostic test started to fail due the fact I've just mentioned.

[jkotas]

It would be nice to get this unified with native AOT

[janvorli]

Merging as the test run with temporarily enabled new EH passed.