Make sure Debug.Fail and other similar methods have frame on the stack (#40807)
authorJan Kotas <jkotas@microsoft.com>
Fri, 14 Aug 2020 02:41:56 +0000 (19:41 -0700)
committerGitHub <noreply@github.com>
Fri, 14 Aug 2020 02:41:56 +0000 (19:41 -0700)
Debugger treats them in a special way

Partial fix for #40793

src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs
src/coreclr/src/vm/corelib.h
src/coreclr/src/vm/jithelpers.cpp
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Debug.cs

index 5e51ac0..1d026a6 100644 (file)
@@ -16,9 +16,6 @@ namespace System.Diagnostics
         [MethodImpl(MethodImplOptions.NoInlining)]
         public static void Break() => BreakInternal();
 
-        // The VM depends on this private method.
-        private static void BreakCanThrow() => BreakInternal();
-
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern void BreakInternal();
 
index e7b66f2..5b08cc4 100644 (file)
@@ -972,7 +972,7 @@ DEFINE_CLASS_U(Threading,              WaitHandle,             WaitHandleBase)
 DEFINE_FIELD_U(_waitHandle,         WaitHandleBase,         m_safeHandle)
 
 DEFINE_CLASS(DEBUGGER,              Diagnostics,            Debugger)
-DEFINE_METHOD(DEBUGGER,             BREAK_CAN_THROW,        BreakCanThrow,          SM_RetVoid)
+DEFINE_METHOD(DEBUGGER,             BREAK,                  Break,                  SM_RetVoid)
 
 DEFINE_CLASS(BUFFER,                System,                 Buffer)
 DEFINE_METHOD(BUFFER,               MEMCPY_PTRBYTE_ARRBYTE, Memcpy,                 SM_PtrByte_Int_ArrByte_Int_Int_RetVoid)
index 54e0cb0..6eb63a5 100644 (file)
@@ -4601,17 +4601,8 @@ HCIMPLEND;
 //========================================================================
 
 /*********************************************************************/
-// JIT_UserBreakpoint
 // Called by the JIT whenever a cee_break instruction should be executed.
-// This ensures that enough info will be pushed onto the stack so that
-// we can continue from the exception w/o having special code elsewhere.
-// Body of function is written by debugger team
-// Args: None
 //
-// <TODO> make sure this actually gets called by all JITters</TODO>
-// Note: this code is duplicated in the ecall in VM\DebugDebugger:Break,
-// so propogate changes to there
-
 HCIMPL0(void, JIT_UserBreakpoint)
 {
     FCALL_CONTRACT;
@@ -4621,12 +4612,9 @@ HCIMPL0(void, JIT_UserBreakpoint)
 #ifdef DEBUGGING_SUPPORTED
     FrameWithCookie<DebuggerExitFrame> __def;
 
-    MethodDescCallSite breakCanThrow(METHOD__DEBUGGER__BREAK_CAN_THROW);
+    MethodDescCallSite debuggerBreak(METHOD__DEBUGGER__BREAK);
 
-    // Call Diagnostic.Debugger.BreakCanThrow instead. This will make us demand
-    // UnmanagedCode permission if debugger is not attached.
-    //
-    breakCanThrow.Call((ARG_SLOT*)NULL);
+    debuggerBreak.Call((ARG_SLOT*)NULL);
 
     __def.Pop();
 #else // !DEBUGGING_SUPPORTED
@@ -4648,7 +4636,6 @@ extern "C" void * _ReturnAddress(void);
 //  if (*pFlag != 0) call JIT_DbgIsJustMyCode
 // So this is only called if the flag (obtained by GetJMCFlagAddr) is
 // non-zero.
-// Body of this function is maintained by the debugger people.
 HCIMPL0(void, JIT_DbgIsJustMyCode)
 {
     FCALL_CONTRACT;
index 8f579c7..da744f6 100644 (file)
@@ -5,6 +5,7 @@
 #define DEBUG
 
 using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
 using System.Threading;
 
 namespace System.Diagnostics
@@ -114,6 +115,7 @@ namespace System.Diagnostics
 
         [System.Diagnostics.Conditional("DEBUG")]
         [DoesNotReturn]
+        [MethodImpl(MethodImplOptions.NoInlining)] // Preserve the frame for debugger
         public static void Fail(string? message, string? detailMessage) =>
             s_provider.Fail(message, detailMessage);