microsoft/CsWin32

AddVectoredExceptionHandler handler callback causes ExecutionEngineException

Closed this issue · 7 comments

Actual behavior

Using CsWin32's AddVectoredExceptionHandler, when the handler callback is called by Windows, we get the message "Fatal error. Invalid Program: attempted to call a UnmanagedCallersOnly method from managed code." printed to the console, and the program crashes with an ExecutionEngineException.

Expected behavior

The handler callback should not crash the program. In the example below, I show one way of getting the handler callback to work using RuntimeMethodHandle.GetFunctionPointer(). I don't understand why the CsWin32 version (or the other approaches I show below) are not working though. Maybe I'm missing something?

Repro steps

  1. NativeMethods.txt content:
AddVectoredExceptionHandler
RemoveVectoredExceptionHandler
  1. NativeMethods.json content (if present):
    N/A

  2. Any of your own code that should be shared?

using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Windows.Win32;
using Windows.Win32.System.Diagnostics.Debug;

class Program
{
    public unsafe static void Main(string[] args)
    {
        if (!OperatingSystem.IsWindowsVersionAtLeast(5, 1, 2600))
            throw new InvalidOperationException();

        void* vehHandle;
        int x;

        // Comment/uncomment the following blocks to try different approaches.

        {
            // RuntimeMethodHandle.GetFunctionPointer: works as expected.
            var methodHandle = typeof(Program).GetMethod(nameof(MethodHandleVEH), BindingFlags.NonPublic | BindingFlags.Static)!.MethodHandle;
            RuntimeHelpers.PrepareMethod(methodHandle);
            // Note: this is not CsWin32's PInvoke; it's my own implementation. See below.
            vehHandle = AddVectoredExceptionHandler(1, methodHandle.GetFunctionPointer());
            Console.WriteLine($"MethodHandle handle: {(nint)vehHandle}");
            x = *(int*)0; // Should throw NullReferenceException.
            RemoveVectoredExceptionHandler(vehHandle);
        }

        {
            // CsWin32: ExecutionEngineException
            vehHandle = PInvoke.AddVectoredExceptionHandler(1, CsWin32VEH);
            Console.WriteLine($"CsWin32 handle: {(nint)vehHandle}");
            x = *(int*)0; // Should throw NullReferenceException, but there is an ExecutionEngineException first.
            PInvoke.RemoveVectoredExceptionHandler(vehHandle);
        }

        {
            // Unmanaged function pointer: ExecutionEngineException
            delegate* unmanaged<EXCEPTION_POINTERS*, int> funcPointer = &FunctionPointerVEH;
            vehHandle = AddVectoredExceptionHandler(1, (nint)funcPointer);
            Console.WriteLine($"Function Pointer handle: {(nint)vehHandle}");
            x = *(int*)0; // Should throw NullReferenceException, but there is an ExecutionEngineException first.
            RemoveVectoredExceptionHandler(vehHandle);
        }

        {
            // Marshal.GetFunctionPointerForDelegate: ExecutionEngineException
            var handlerDelegate = new VectoredExceptionHandler(DelegateVEH);
            vehHandle = AddVectoredExceptionHandler(1, Marshal.GetFunctionPointerForDelegate(handlerDelegate));
            Console.WriteLine($"Delegate handle: {(nint)vehHandle}");
            x = *(int*)0; // Should throw NullReferenceException, but there is an ExecutionEngineException first.
            RemoveVectoredExceptionHandler(vehHandle);
        }
    }

    static unsafe int MethodHandleVEH(EXCEPTION_POINTERS* e) => 0;
    static unsafe int CsWin32VEH(EXCEPTION_POINTERS* e) => 0;
    [UnmanagedCallersOnly] static unsafe int FunctionPointerVEH(EXCEPTION_POINTERS* e) => 0;
    static unsafe int DelegateVEH(EXCEPTION_POINTERS* e) => 0;

    [DllImport("Kernel32.dll")]
    static extern unsafe void* AddVectoredExceptionHandler(uint first, nint handlerAddress);

    [DllImport("Kernel32.dll")]
    static extern unsafe ulong RemoveVectoredExceptionHandler(void* handle);

    unsafe delegate int VectoredExceptionHandler(EXCEPTION_POINTERS* exceptionInfo);
}

Context

  • CsWin32 version: 0.3.106
  • Target Framework: net8.0

I was able to change the first block in your sample to successfully use cswin32 by adding "allowMarshaling": false to the NativeMethods.json file:

 {
     // RuntimeMethodHandle.GetFunctionPointer: works as expected.
     var methodHandle = typeof(Program).GetMethod(nameof(MethodHandleVEH), BindingFlags.NonPublic | BindingFlags.Static)!.MethodHandle;
     RuntimeHelpers.PrepareMethod(methodHandle);
     vehHandle = PInvoke.AddVectoredExceptionHandler(1, (delegate* unmanaged[Stdcall]<EXCEPTION_POINTERS*, int>)methodHandle.GetFunctionPointer());
     Console.WriteLine($"MethodHandle handle: {(nint)vehHandle}");
     x = *(int*)0; // Should throw NullReferenceException.
     RemoveVectoredExceptionHandler(vehHandle);
 }

But when using delegate syntax, I don't see how to make it work. The ExecutionEngineException thrown by the Core CLR isn't helping with diagnosis either.

@AaronRobinsonMSFT Any ideas? I'm surprised that the only working approach above involves calling RuntimeHelpers.PrepareMethod and I wonder if there's something more CsWin32 can do to make this work out of the box.

The issue here is how the function is being called. Writing Win32 SEH handlers in C# is going to be ill-advised from an interop perspective and obviously non-portable. In this case, we might be able to address part of this, but it is going to need thought. The core issue is there is no thread GC mode switch from when the exception is triggered and the SEH handler is being invoked by the OS. The supplied callback is being called when the thread is in Cooperative mode and that is invalid for an unmanaged function pointer - which is what you have in this case.

Let me see if there is something we can do in this case.

/cc @jkotas

Also, regardless of the current semantics all of the above using Delegates are fundamentally broken. A function pointer to a Delegate instance that is stored on the unmanaged side can become invalid unless the Delegate's lifetime is extended for the duration of its use. A common solution is to do the following:

static Delegate s_del;

Delegate del = ...
s_del = del;
PInvoke.StorePointerAndCallLater(Marshal.GetFunctionPointerForDelegate(del));

Or alternatively:

Delegate del = ...
PInvoke.StorePointerAndCallLater(Marshal.GetFunctionPointerForDelegate(del));
...
PInvoke.RemovePointer(Marshal.GetFunctionPointerForDelegate(del));
GC.KeepAlive(del);

I've looked more into this and discussed with various experts on the impact to the GC and this isn't something we are going to support. As I said, writing any of these exceptions handles in C# is ill-advised from an interop perspective. From a correctness perspective it is even more difficult due to the GC's stackwalk is now basically impossible because all the exception infrastructure frames are now on the stack and the GC has no way to know how they can/should be walked. This scenario is identical to writing signal handlers in C# on non-Windows, which is also unsupported.

My general comment about Delegates above remains the case and is required to be considered correct usage for P/Invoke scenarios where the function pointer is retained after the callee returns.

Thank you, @AaronRobinsonMSFT.

CsWin32 has a banned API list that I could add this particular function to, such that an attempt to generate it could emit an error that links to this discussion.
Are there any other win32 functions that you know of that should never be invoked from a .NET (Framework) / NativeAOT app?

CsWin32 has a banned API list that I could add this particular function to

Oh, nice feature.

Are there any other win32 functions that you know of that should never be invoked from a .NET (Framework) / NativeAOT app?

I think any of the functions that accept a callback on this list and that will be called during an exception pass should be banned: https://learn.microsoft.com/windows/win32/debug/structured-exception-handling-functions

Thanks. I reviewed them all, and none are as obviously only used in the banned circumstance as the two already called out. But I'm certainly not an expert in this area, so if you have any additional recommendations please call them out.