Fix caught foreign native exception handling take 2 (#116693)

* Fix reporting of foreign native exception not handled by runtime take 2

The previous fix was reverted, as it didn't take into account hardware
exceptions stemming from managed code. This has broken debugger tests.

When a native exception occurs on a foreign thread, it is propagated
through managed frames and then flows into the foreign native frames
and is handled there, the runtime still reports the exception as unhandled
via AppDomain.CurrentDomain.UnhandledException event and also prints
out the unhandled exception message. Even though the process then continues
running just fine.

This change fixes the behavior so that only managed exceptions
thrown by the .NET runtime are reported as unhandled in such case.
Foreign exceptions, like e.g. C++ exceptions, are not reported as
unhandled and they are just propagated into the foreign native
frames. If that exception is not handled by the native frames,
then C++ reports them as unhandled.

I have also added a test that fails without this fix.

Close #115654

* Fix the problem of the previous solution

Also add hardware unhandled exception test

* Update src/coreclr/vm/exceptionhandling.cpp

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

---------

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

Author: janvorli

src/coreclr/vm/exceptionhandling.cpp

@@ -4014,13 +4014,16 @@ extern "C" CLR_BOOL QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollid
             // Check if there are any further managed frames on the stack or a catch for all exceptions in native code (marked by
             // DebuggerU2MCatchHandlerFrame with CatchesAllExceptions() returning true).
             // If not, the exception is unhandled.
-            if ((pFrame == FRAME_TOP) ||
+            bool isNotHandledByRuntime = 
+                (pFrame == FRAME_TOP) ||
                 (IsTopmostDebuggerU2MCatchHandlerFrame(pFrame) && !((DebuggerU2MCatchHandlerFrame*)pFrame)->CatchesAllExceptions())
 #ifdef HOST_UNIX
                 // Don't allow propagating exceptions from managed to non-runtime native code
                 || isPropagatingToExternalNativeCode
 #endif
-               )
+                ;
+
+            if (isNotHandledByRuntime && IsExceptionFromManagedCode(pTopExInfo->m_ptrs.ExceptionRecord))
             {
                 EH_LOG((LL_INFO100, "SfiNext (pass %d): no more managed frames on the stack, the exception is unhandled", pTopExInfo->m_passNumber));
                 if (pTopExInfo->m_passNumber == 1)

src/tests/baseservices/exceptions/exceptioninterop/ExceptionInterop.cs

@@ -158,7 +158,7 @@ static void ThrowInFrameWithFinally()
     }
 
     [DllImport(nameof(ExceptionInteropNative))]
-    public static extern void InvokeCallbackCatchCallbackAndRethrow(delegate*unmanaged<void> callBack1, delegate*unmanaged<void> callBack2);
+    public static extern void InvokeCallbackCatchCallbackAndRethrow(delegate* unmanaged<void> callBack1, delegate* unmanaged<void> callBack2);
 
     [UnmanagedCallersOnly]
     static void CallPInvoke()
@@ -193,4 +193,25 @@ public static void PropagateAndRethrowCppException()
             Console.WriteLine($"Caught {ex}");
         }
     }
+    
+    [DllImport(nameof(ExceptionInteropNative))]
+    public static extern void InvokeCallbackOnNewThread(delegate*unmanaged<void> callBack);
+
+    [Fact]
+    [PlatformSpecific(TestPlatforms.Windows)]
+    [SkipOnMono("Exception interop not supported on Mono.")]
+    public static void PropagateAndCatchCppException()
+    {
+        bool reportedUnhandledException = false;
+        UnhandledExceptionEventHandler handler = (sender, e) =>
+        {
+            Console.WriteLine($"Exception reported as unhandled: {e.ExceptionObject}");
+            reportedUnhandledException = true;
+        };
+
+        AppDomain.CurrentDomain.UnhandledException += handler;
+        InvokeCallbackOnNewThread(&CallPInvoke);
+        AppDomain.CurrentDomain.UnhandledException -= handler;
+        Assert.False(reportedUnhandledException, "Exception should not be reported as unhandled");
+    }
 }

src/tests/baseservices/exceptions/exceptioninterop/ExceptionInteropNative.cpp

@@ -3,6 +3,12 @@
 
 #include <exception>
 #include <platformdefines.h>
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable:4265 4577)
+#include <thread>
+#pragma warning(pop)
+#endif // _WIN32
 
 extern "C" DLL_EXPORT void STDMETHODCALLTYPE ThrowException()
 {
@@ -33,3 +39,22 @@ extern "C" DLL_EXPORT void InvokeCallbackCatchCallbackAndRethrow(PFNACTION1 call
     }
 }
 
+#ifdef _WIN32
+extern "C" DLL_EXPORT void STDMETHODCALLTYPE InvokeCallbackAndCatchNative(PFNACTION1 callback)
+{
+    try
+    {
+        callback();
+    }
+    catch (std::exception& ex)
+    {
+        printf("Caught exception %s in native code\n", ex.what());
+    }
+}
+
+extern "C" DLL_EXPORT void STDMETHODCALLTYPE InvokeCallbackOnNewThread(PFNACTION1 callback)
+{
+    std::thread t1(InvokeCallbackAndCatchNative, callback);
+    t1.join();
+}
+#endif // _WIN32

src/tests/baseservices/exceptions/unhandled/unhandled.cs

@@ -48,6 +48,11 @@ static void Main(string[] args)
             {
                 throw new Exception("Test");
             }
+            if (args[0] == "mainhardware")
+            {
+                string s = null;
+                Console.WriteLine(s.Length); // This will cause a NullReferenceException
+            }
             else if (args[0] == "foreign")
             {
                 InvokeCallbackOnNewThread(&ThrowException);
@@ -58,6 +63,22 @@ static void Main(string[] args)
                 t.Start();
                 t.Join();
             }
+            else if (args[0] == "secondaryhardware")
+            {
+                Thread t = new Thread(() =>
+                {
+                    string s = null;
+                    Console.WriteLine(s.Length); // This will cause a NullReferenceException
+                });
+                t.Start();
+                t.Join();
+            }
+            else if (args[0] == "secondaryunhandled")
+            {
+                Thread t = new Thread(() => throw new Exception("Test"));
+                t.Start();
+                t.Join();
+            }
         }
     }
 }

src/tests/baseservices/exceptions/unhandled/unhandledTester.cs

@@ -47,15 +47,23 @@ static void RunExternalProcess(string unhandledType, string assembly)
             }
             else if (!OperatingSystem.IsWindows())
             {
-                expectedExitCode = 128 + 6;
+                expectedExitCode = 128 + 6; // SIGABRT
             }
             else if (TestLibrary.Utilities.IsNativeAot)
             {
                 expectedExitCode = unchecked((int)0xC0000409);
             }
             else
             {
-                expectedExitCode = unchecked((int)0xE0434352);
+                if (unhandledType.EndsWith("hardware"))
+                {
+                    // Null reference exception code
+                    expectedExitCode = unchecked((int)0xC0000005);
+                }
+                else
+                {
+                    expectedExitCode = unchecked((int)0xE0434352);
+                }
             }
 
             if (expectedExitCode != testProcess.ExitCode)
@@ -128,7 +136,9 @@ static void RunExternalProcess(string unhandledType, string assembly)
         public static void TestEntryPoint()
         {
             RunExternalProcess("main", "unhandled.dll");
+            RunExternalProcess("mainhardware", "unhandled.dll");
             RunExternalProcess("secondary", "unhandled.dll");
+            RunExternalProcess("secondaryhardware", "unhandled.dll");
             RunExternalProcess("foreign", "unhandled.dll");
             File.Delete(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "dependencytodelete.dll"));
             RunExternalProcess("missingdependency", "unhandledmissingdependency.dll");