DbgEngWrapper: handle SEH exceptions from dbgeng

Ideally dbgeng would never AV. But sometimes it does, and when it does,
it can be difficult to figure out what is going on. Catching them gives
us a chance, and prevents our frames from being silently ripped through.

DbgEngWrapper/DbgEngWrapper.h

@@ -20,6 +20,8 @@ using namespace System::Diagnostics::Tracing;
 
 namespace DbgEngWrapper
 {
+    public delegate void NotifySomethingReallyBadHappened( String^ msg );
+
     // A custom deleter to let us use malloc-allocated pointers with unique_ptr.
     struct free_delete
     {
@@ -48,6 +50,8 @@ namespace DbgEngWrapper
     protected:
         TNativeInterface* m_pNative;
 
+        typedef TNativeInterface TN;
+
         WDebugEngInterface( TNativeInterface* pNative )
         {
             if( !pNative )
@@ -64,6 +68,46 @@ namespace DbgEngWrapper
             m_pNative = (TNativeInterface*) (void*) pNative;
         }
 
+
+        // This simple exception filter just saves the exception code for the caller and
+        // then requests that the handler be executed.
+        int MyExceptionFilter( EXCEPTION_POINTERS* pEp, HRESULT* pCode )
+        {
+            *pCode = pEp->ExceptionRecord->ExceptionCode;
+            return EXCEPTION_EXECUTE_HANDLER;
+        }
+
+        // My first attempt to wrap calls to dbgeng in __try/__except was to use macros to
+        // stamp it out. Unfortunately some functions were not able to have __try (C2712:
+        // "cannot use __try in functions that require object unwinding"). So that's why
+        // the fancy template: a bunch of the calls need to be in separate functions
+        // anyway, and I didn't want to have two ways to do it.
+        //
+        // One of the downsides of this template method, though, is that I had to add in a
+        // lot of explicit casts: where before things like pin_ptr<ULONG> were implicitly
+        // convertible to PULONG, and would match up with the call signature, suddenly I
+        // was getting a lot of C2893: "Failed to specialize function template 'template
+        // name'". You have to get the types just right /before/ the template stuff will
+        // work out. Oh well.
+        template<typename Method, typename ... Arguments>
+        HRESULT
+        CallMethodWithSehProtection(
+            Method pfn,
+            Arguments... args)
+        {
+            HRESULT hr = 0;
+            __try
+            {
+                return (m_pNative->*pfn)(args...);
+            }
+            __except( MyExceptionFilter( GetExceptionInformation(), &hr ) )
+            {
+                String^ msg = String::Format( "SEH exception from dbgeng: 0x{0:x}", hr );
+                WDebugClient::g_notifyBadThingCallback( msg );
+                return hr;
+            }
+        }
+
     public:
         IntPtr GetRaw()
         {
@@ -204,6 +248,8 @@ namespace DbgEngWrapper
     {
     public:
         static EventSource^ g_log = gcnew EventSource("DbgEngWrapperTraceLoggingProvider");
+        static NotifySomethingReallyBadHappened^ g_notifyBadThingCallback;
+
 
         WDebugClient( ::IDebugClient6* pDc );
 
@@ -211,7 +257,8 @@ namespace DbgEngWrapper
         WDebugClient( IntPtr pDc );
 
 
-        static int DebugCreate( [Out] WDebugClient^% dc )
+        static int DebugCreate( NotifySomethingReallyBadHappened^ notifyBadThingCallback,
+                                [Out] WDebugClient^% dc )
         {
             dc = nullptr;
             // TODO: Investigate DebugCreateEx
@@ -233,6 +280,7 @@ namespace DbgEngWrapper
             g_log->Write( L"Created IDebugClient5", gcnew TlPayload_Int( hr ) );
 #pragma warning (pop)
 
+            g_notifyBadThingCallback = notifyBadThingCallback;
             return hr;
         }
 

DbgProvider/public/Debugger/DbgEngDebugger.cs

@@ -23,6 +23,12 @@ namespace MS.Dbg
     {
         private static DbgEngDebugger g_Debugger;
 
+        private static void BadNewsFromDbgEngWrapper( string msg )
+        {
+            // This will be logged, even on Release builds.
+            Util.Fail( Util.Sprintf( "Bad news from DbgEngWrapper: {0}", msg ) );
+        }
+
 
         internal static DbgEngDebugger _GlobalDebugger
         {
@@ -34,7 +40,8 @@ namespace MS.Dbg
                         {
                             if( null == g_Debugger )
                             {
-                                StaticCheckHr( WDebugClient.DebugCreate( out WDebugClient dc ) );
+                                StaticCheckHr( WDebugClient.DebugCreate( BadNewsFromDbgEngWrapper,
+                                                                         out WDebugClient dc ) );
                                 g_Debugger = new DbgEngDebugger( dc, DbgEngThread.Singleton );
                             }
                         } );
@@ -4238,7 +4245,8 @@ namespace MS.Dbg
                     hr = m_debugControl.GetDebuggeeType( out targetClass, out qualifier );
                     if( 0 != hr )
                     {
-                        LogManager.Trace( "GetCurrentDbgEngContext: could not determine if kernel mode: {0}.", hr );
+                        LogManager.Trace( "GetCurrentDbgEngContext: could not determine if kernel mode: {0}.",
+                                          Util.FormatErrorCode( hr ) );
                         return;
                     }
 
@@ -4265,7 +4273,8 @@ namespace MS.Dbg
                         hr = m_debugSystemObjects.GetImplicitThreadDataOffset( out threadIdOrAddr );
                         if( 0 != hr )
                         {
-                            LogManager.Trace( "GetCurrentDbgEngContext: no current kernel-mode thread: {0}", hr );
+                            LogManager.Trace( "GetCurrentDbgEngContext: no current kernel-mode thread: {0}",
+                                              Util.FormatErrorCode( hr ) );
                             Util.Fail( "is this possible?" );
                             return;
                         }
@@ -4277,7 +4286,8 @@ namespace MS.Dbg
                         hr = m_debugSystemObjects.GetCurrentProcessId( out uiProcId );
                         if( 0 != hr )
                         {
-                            LogManager.Trace( "GetCurrentDbgEngContext: no current process: {0}.", hr );
+                            LogManager.Trace( "GetCurrentDbgEngContext: no current process: {0}.",
+                                              Util.FormatErrorCode( hr ) );
                             return;
                         }
                         procIdOrAddr = uiProcId;
@@ -4285,7 +4295,8 @@ namespace MS.Dbg
                         hr = m_debugSystemObjects.GetCurrentThreadId( out uiThreadId );
                         if( 0 != hr )
                         {
-                            LogManager.Trace( "GetCurrentDbgEngContext: no current thread: {0}.", hr );
+                            LogManager.Trace( "GetCurrentDbgEngContext: no current thread: {0}.",
+                                              Util.FormatErrorCode( hr ) );
                             return;
                         }
                         threadIdOrAddr = uiThreadId;
@@ -4294,7 +4305,8 @@ namespace MS.Dbg
                     hr = m_debugSymbols.GetCurrentScopeFrameIndexEx( DEBUG_FRAME.DEFAULT, out frameId );
                     if( 0 != hr )
                     {
-                        LogManager.Trace( "GetCurrentDbgEngContext: no current frame: {0}." , hr );
+                        LogManager.Trace( "GetCurrentDbgEngContext: no current frame: {0}.",
+                                          Util.FormatErrorCode( hr ) );
                         return;
                     }
                 } );