From a811fa10037efa1073ddcbdafccee28bd432a1a4 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Thu, 11 Feb 2021 18:12:22 -0800 Subject: [PATCH 1/3] Port dotnet/runtime#46617: Convert Debugger.Log and Debugger.Launch to QCalls --- .../src/System/Diagnostics/Debugger.cs | 10 ++- src/coreclr/src/vm/debugdebugger.cpp | 73 ++++++++----------- src/coreclr/src/vm/debugdebugger.h | 4 +- src/coreclr/src/vm/ecalllist.h | 4 +- 4 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs index 1d026a6432339c..64e08d404cdbd1 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs @@ -5,6 +5,7 @@ // and is used for communicating with a debugger. using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace System.Diagnostics { @@ -50,7 +51,7 @@ public static void NotifyOfCrossThreadDependency() } } - [MethodImpl(MethodImplOptions.InternalCall)] + [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] private static extern bool LaunchInternal(); // Returns whether or not a debugger is attached to the process. @@ -74,8 +75,11 @@ public static extern bool IsAttached // Posts a message for the attached debugger. If there is no // debugger attached, has no effect. The debugger may or may not // report the message depending on its settings. - [MethodImpl(MethodImplOptions.InternalCall)] - public static extern void Log(int level, string? category, string? message); + public static void Log(int level, string? category, string? message) => LogInternal(level, category, message); + + + [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] + private static extern void LogInternal(int level, string? category, string? message); // Checks to see if an attached debugger has logging enabled // diff --git a/src/coreclr/src/vm/debugdebugger.cpp b/src/coreclr/src/vm/debugdebugger.cpp index 29e6e3b8d6f7f8..fd6359db1a8a8f 100644 --- a/src/coreclr/src/vm/debugdebugger.cpp +++ b/src/coreclr/src/vm/debugdebugger.cpp @@ -158,35 +158,30 @@ FCIMPL0(void, DebugDebugger::Break) } FCIMPLEND -FCIMPL0(FC_BOOL_RET, DebugDebugger::Launch) +BOOL QCALLTYPE DebugDebugger::Launch() { - FCALL_CONTRACT; + QCALL_CONTRACT; + + BOOL retVal = FALSE; + + BEGIN_QCALL; #ifdef DEBUGGING_SUPPORTED if (CORDebuggerAttached()) { - FC_RETURN_BOOL(TRUE); + retVal = TRUE; } else if (g_pDebugInterface != NULL) { - HRESULT hr = S_OK; - - HELPER_METHOD_FRAME_BEGIN_RET_0(); - - hr = g_pDebugInterface->LaunchDebuggerForUser(GetThread(), NULL, TRUE, TRUE); - - HELPER_METHOD_FRAME_END(); - - if (SUCCEEDED (hr)) - { - FC_RETURN_BOOL(TRUE); - } + HRESULT hr = g_pDebugInterface->LaunchDebuggerForUser(GetThread(), NULL, TRUE, TRUE); + retVal = SUCCEEDED(hr); } #endif // DEBUGGING_SUPPORTED - FC_RETURN_BOOL(FALSE); + END_QCALL; + + return retVal; } -FCIMPLEND FCIMPL0(FC_BOOL_RET, DebugDebugger::IsDebuggerAttached) @@ -229,42 +224,35 @@ FCIMPLEND // appending a newline to anything. // It will also call OutputDebugString() which will send a native debug event. The message // string there will be a composite of the two managed string parameters and may include a newline. -FCIMPL3(void, DebugDebugger::Log, - INT32 Level, - StringObject* strModuleUNSAFE, - StringObject* strMessageUNSAFE - ) +void QCALLTYPE DebugDebugger::Log(INT32 Level, PCWSTR pwzModule, PCWSTR pwzMessage) { CONTRACTL { - FCALL_CHECK; - PRECONDITION(CheckPointer(strModuleUNSAFE, NULL_OK)); - PRECONDITION(CheckPointer(strMessageUNSAFE, NULL_OK)); + QCALL_CHECK; + PRECONDITION(CheckPointer(pwzModule, NULL_OK)); + PRECONDITION(CheckPointer(pwzMessage, NULL_OK)); } CONTRACTL_END; - STRINGREF strModule = (STRINGREF)ObjectToOBJECTREF(strModuleUNSAFE); - STRINGREF strMessage = (STRINGREF)ObjectToOBJECTREF(strMessageUNSAFE); - - HELPER_METHOD_FRAME_BEGIN_2(strModule, strMessage); + BEGIN_QCALL; // OutputDebugString will log to native/interop debugger. - if (strModule != NULL) + if (pwzModule != NULL) { - WszOutputDebugString(strModule->GetBuffer()); + WszOutputDebugString(pwzModule); WszOutputDebugString(W(" : ")); } - if (strMessage != NULL) + if (pwzMessage != NULL) { - WszOutputDebugString(strMessage->GetBuffer()); + WszOutputDebugString(pwzMessage); } // If we're not logging a module prefix, then don't log the newline either. // Thus if somebody is just logging messages, there won't be any extra newlines in there. // If somebody is also logging category / module information, then this call to OutputDebugString is // already prepending that to the message, so we append a newline for readability. - if (strModule != NULL) + if (pwzModule != NULL) { WszOutputDebugString(W("\n")); } @@ -283,21 +271,21 @@ FCIMPL3(void, DebugDebugger::Log, // Strings may contain embedded nulls, but we need to handle null-terminated // strings, so use truncate now. StackSString switchName; - if( strModule != NULL ) + if(pwzModule != NULL) { // truncate if necessary - COUNT_T iLen = (COUNT_T) wcslen(strModule->GetBuffer()); + COUNT_T iLen = (COUNT_T) wcslen(pwzModule); if (iLen > MAX_LOG_SWITCH_NAME_LEN) { iLen = MAX_LOG_SWITCH_NAME_LEN; } - switchName.Set(strModule->GetBuffer(), iLen); + switchName.Set(pwzModule, iLen); } SString message; - if( strMessage != NULL ) + if(pwzMessage != NULL) { - message.Set(strMessage->GetBuffer(), (COUNT_T) wcslen(strMessage->GetBuffer())); + message.Set(pwzMessage, (COUNT_T) wcslen(pwzMessage)); } g_pDebugInterface->SendLogMessage (Level, &switchName, &message); @@ -306,9 +294,8 @@ FCIMPL3(void, DebugDebugger::Log, #endif // DEBUGGING_SUPPORTED - HELPER_METHOD_FRAME_END(); + END_QCALL; } -FCIMPLEND FCIMPL0(FC_BOOL_RET, DebugDebugger::IsLogging) @@ -1174,14 +1161,14 @@ void DebugStackTrace::DebugStackTraceElement::InitPass2() _ASSERTE(!ThreadStore::HoldingThreadStore()); - bool bRes = false; + bool bRes = false; #ifdef DEBUGGING_SUPPORTED // Calculate the IL offset using the debugging services if ((this->ip != NULL) && g_pDebugInterface) { // To get the source line number of the actual code that threw an exception, the dwOffset needs to be - // adjusted in certain cases when calculating the IL offset. + // adjusted in certain cases when calculating the IL offset. // // The dwOffset of the stack frame points to either: // diff --git a/src/coreclr/src/vm/debugdebugger.h b/src/coreclr/src/vm/debugdebugger.h index 3a36f9875ebb5a..28b82181c70c15 100644 --- a/src/coreclr/src/vm/debugdebugger.h +++ b/src/coreclr/src/vm/debugdebugger.h @@ -20,9 +20,9 @@ class DebugDebugger { public: static FCDECL0(void, Break); - static FCDECL0(FC_BOOL_RET, Launch); + static BOOL QCALLTYPE Launch(); static FCDECL0(FC_BOOL_RET, IsDebuggerAttached); - static FCDECL3(void, Log, INT32 Level, StringObject* strModule, StringObject* strMessage); + static void QCALLTYPE Log(INT32 Level, PCWSTR pwzModule, PCWSTR pwzMessage); // receives a custom notification object from the target and sends it to the RS via // code:Debugger::SendCustomDebuggerNotification diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 2c2dd518c09bfa..a192ce133b4cca 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -133,9 +133,9 @@ FCFuncEnd() FCFuncStart(gDiagnosticsDebugger) FCFuncElement("BreakInternal", DebugDebugger::Break) - FCFuncElement("LaunchInternal", DebugDebugger::Launch) + QCFuncElement("LaunchInternal", DebugDebugger::Launch) FCFuncElement("get_IsAttached", DebugDebugger::IsDebuggerAttached) - FCFuncElement("Log", DebugDebugger::Log) + QCFuncElement("LogInternal", DebugDebugger::Log) FCFuncElement("IsLogging", DebugDebugger::IsLogging) FCFuncElement("CustomNotification", DebugDebugger::CustomNotification) FCFuncEnd() From 3f8a2240a7457c328030269c76f5c9930438a6c4 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Thu, 11 Feb 2021 22:26:54 -0800 Subject: [PATCH 2/3] Convert BindToMethodName to QCall --- .../src/System/Delegate.CoreCLR.cs | 16 ++++++- src/coreclr/src/vm/comdelegate.cpp | 46 +++++++++---------- src/coreclr/src/vm/comdelegate.h | 2 +- src/coreclr/src/vm/ecalllist.h | 2 +- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs index ab338cd0bc9cdf..06346a0d92ba67 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs @@ -418,9 +418,21 @@ internal static Delegate CreateDelegateNoSecurityCheck(Type type, object? target // internal implementation details (FCALLS and utilities) // + [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] + private static extern bool BindToMethodName(ObjectHandleOnStack delegateObj, ObjectHandleOnStack target, QCallTypeHandle methodType, string method, DelegateBindingFlags flags); + [RequiresUnreferencedCode("The target method might be removed")] - [MethodImpl(MethodImplOptions.InternalCall)] - private extern bool BindToMethodName(object? target, RuntimeType methodType, string method, DelegateBindingFlags flags); + private bool BindToMethodName(object? target, RuntimeType methodType, string method, DelegateBindingFlags flags) + { + Delegate delArg = this; + return BindToMethodName( + ObjectHandleOnStack.Create(ref delArg), + ObjectHandleOnStack.Create(ref target), + new QCallTypeHandle(ref methodType), + method, + flags + ); + } [MethodImpl(MethodImplOptions.InternalCall)] private extern bool BindToMethodInfo(object? target, IRuntimeMethodInfo method, RuntimeType methodType, DelegateBindingFlags flags); diff --git a/src/coreclr/src/vm/comdelegate.cpp b/src/coreclr/src/vm/comdelegate.cpp index 9c5b48cc8ce730..2fb71cb699d0c6 100644 --- a/src/coreclr/src/vm/comdelegate.cpp +++ b/src/coreclr/src/vm/comdelegate.cpp @@ -789,33 +789,31 @@ static PCODE GetVirtualCallStub(MethodDesc *method, TypeHandle scopeType) return pTargetCall; } -FCIMPL5(FC_BOOL_RET, COMDelegate::BindToMethodName, - Object *refThisUNSAFE, - Object *targetUNSAFE, - ReflectClassBaseObject *pMethodTypeUNSAFE, - StringObject* methodNameUNSAFE, - int flags) +BOOL QCALLTYPE COMDelegate::BindToMethodName( + QCall::ObjectHandleOnStack delegate, + QCall::ObjectHandleOnStack target, + QCall::TypeHandle methodTypeHandle, + PCWSTR pwzMethodName, + int flags) { - FCALL_CONTRACT; + QCALL_CONTRACT; + + MethodDesc *pMatchingMethod = NULL; + + BEGIN_QCALL; + GCX_COOP(); struct _gc { DELEGATEREF refThis; OBJECTREF target; - STRINGREF methodName; - REFLECTCLASSBASEREF refMethodType; } gc; - gc.refThis = (DELEGATEREF) ObjectToOBJECTREF(refThisUNSAFE); - gc.target = (OBJECTREF) targetUNSAFE; - gc.methodName = (STRINGREF) methodNameUNSAFE; - gc.refMethodType = (REFLECTCLASSBASEREF) ObjectToOBJECTREF(pMethodTypeUNSAFE); + gc.refThis = (DELEGATEREF) delegate.Get(); + gc.target = (OBJECTREF) target.Get(); + TypeHandle methodType = methodTypeHandle.AsTypeHandle(); - TypeHandle methodType = gc.refMethodType->GetType(); - - MethodDesc *pMatchingMethod = NULL; - - HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); + GCPROTECT_BEGIN(gc); // Caching of MethodDescs (impl and decl) for MethodTable slots provided significant // performance gain in some reflection emit scenarios. @@ -832,7 +830,7 @@ FCIMPL5(FC_BOOL_RET, COMDelegate::BindToMethodName, // // get the name in UTF8 format - SString wszName(SString::Literal, gc.methodName->GetBuffer()); + SString wszName(SString::Literal, pwzMethodName); StackScratchBuffer utf8Name; LPCUTF8 szNameStr = wszName.GetUTF8(utf8Name); @@ -900,19 +898,21 @@ FCIMPL5(FC_BOOL_RET, COMDelegate::BindToMethodName, pCurMethod, methodType.GetMethodTable(), fIsOpenDelegate); - pMatchingMethod = pCurMethod; + goto done; } } } done: ; - HELPER_METHOD_FRAME_END(); - FC_RETURN_BOOL(pMatchingMethod != NULL); + GCPROTECT_END(); + + END_QCALL; + + return pMatchingMethod != NULL; } -FCIMPLEND FCIMPL5(FC_BOOL_RET, COMDelegate::BindToMethodInfo, Object* refThisUNSAFE, Object* targetUNSAFE, ReflectMethodObject *pMethodUNSAFE, ReflectClassBaseObject *pMethodTypeUNSAFE, int flags) diff --git a/src/coreclr/src/vm/comdelegate.h b/src/coreclr/src/vm/comdelegate.h index 5226f6357ffd7e..3e34370d62cf3c 100644 --- a/src/coreclr/src/vm/comdelegate.h +++ b/src/coreclr/src/vm/comdelegate.h @@ -63,7 +63,7 @@ class COMDelegate static FCDECL3(PCODE, AdjustTarget, Object* refThis, Object* target, PCODE method); static FCDECL2(PCODE, GetCallStub, Object* refThis, PCODE method); - static FCDECL5(FC_BOOL_RET, BindToMethodName, Object* refThisUNSAFE, Object* targetUNSAFE, ReflectClassBaseObject *pMethodTypeUNSAFE, StringObject* methodNameUNSAFE, int flags); + static BOOL QCALLTYPE BindToMethodName(QCall::ObjectHandleOnStack delegate, QCall::ObjectHandleOnStack target, QCall::TypeHandle methodType, PCWSTR pwzMethodName, int flags); static FCDECL5(FC_BOOL_RET, BindToMethodInfo, Object* refThisUNSAFE, Object* targetUNSAFE, ReflectMethodObject *method, ReflectClassBaseObject *pMethodTypeUNSAFE, int flags); diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index a192ce133b4cca..7d0df930440ce6 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -510,7 +510,7 @@ FCFuncStart(gAssemblyBuilderFuncs) FCFuncEnd() FCFuncStart(gDelegateFuncs) - FCFuncElement("BindToMethodName", COMDelegate::BindToMethodName) + QCFuncElement("BindToMethodName", COMDelegate::BindToMethodName) FCFuncElement("BindToMethodInfo", COMDelegate::BindToMethodInfo) FCFuncElement("GetMulticastInvoke", COMDelegate::GetMulticastInvoke) FCFuncElement("GetInvokeMethod", COMDelegate::GetInvokeMethod) From f64780875588b85708ed6daa7fe1c0bfd1beb23c Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Fri, 12 Feb 2021 03:41:32 -0800 Subject: [PATCH 3/3] Convert StubHelpers::ValidateObject to QCall --- .../src/System/StubHelpers.cs | 11 +++++++++-- src/coreclr/src/vm/ecalllist.h | 2 +- src/coreclr/src/vm/stubhelpers.cpp | 14 +++++++------- src/coreclr/src/vm/stubhelpers.h | 2 +- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs b/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs index b507b28559dac2..62565d12e440aa 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/StubHelpers.cs @@ -1328,8 +1328,15 @@ internal static void CheckStringLength(uint length) [MethodImpl(MethodImplOptions.InternalCall)] internal static extern uint CalcVaListSize(IntPtr va_list); - [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern void ValidateObject(object obj, IntPtr pMD, object pThis); + [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] + internal static extern void ValidateObject(ObjectHandleOnStack obj, IntPtr pMD, ObjectHandleOnStack pThis); + + internal static void ValidateObject(object obj, IntPtr pMD, object pThis) + { + object objLocal = obj; + object localThis = pThis; + ValidateObject(ObjectHandleOnStack.Create(ref objLocal), pMD, ObjectHandleOnStack.Create(ref localThis)); + } [MethodImpl(MethodImplOptions.InternalCall)] internal static extern void LogPinnedArgument(IntPtr localDesc, IntPtr nativeArg); diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 7d0df930440ce6..47e62686eba7bc 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -971,7 +971,7 @@ FCFuncStart(gStubHelperFuncs) FCFuncElement("MarshalToUnmanagedVaListInternal", StubHelpers::MarshalToUnmanagedVaListInternal) FCFuncElement("MarshalToManagedVaListInternal", StubHelpers::MarshalToManagedVaListInternal) FCFuncElement("CalcVaListSize", StubHelpers::CalcVaListSize) - FCFuncElement("ValidateObject", StubHelpers::ValidateObject) + QCFuncElement("ValidateObject", StubHelpers::ValidateObject) FCFuncElement("ValidateByref", StubHelpers::ValidateByref) FCFuncElement("LogPinnedArgument", StubHelpers::LogPinnedArgument) FCIntrinsic("GetStubContext", StubHelpers::GetStubContext, CORINFO_INTRINSIC_StubHelpers_GetStubContext) diff --git a/src/coreclr/src/vm/stubhelpers.cpp b/src/coreclr/src/vm/stubhelpers.cpp index ea21bcbf4bc35b..7b79f157c75db4 100644 --- a/src/coreclr/src/vm/stubhelpers.cpp +++ b/src/coreclr/src/vm/stubhelpers.cpp @@ -1011,16 +1011,18 @@ FCIMPL2(void, StubHelpers::MarshalToManagedVaListInternal, va_list va, VARARGS* } FCIMPLEND -FCIMPL3(void, StubHelpers::ValidateObject, Object *pObjUNSAFE, MethodDesc *pMD, Object *pThisUNSAFE) +void QCALLTYPE StubHelpers::ValidateObject(QCall::ObjectHandleOnStack object, MethodDesc *pMD, QCall::ObjectHandleOnStack pThis) { - FCALL_CONTRACT; + QCALL_CONTRACT; #ifdef VERIFY_HEAP - HELPER_METHOD_FRAME_BEGIN_0(); + BEGIN_QCALL; + GCX_COOP(); StackSString errorString; EX_TRY { + Object* pObjUNSAFE = OBJECTREFToObject(object.Get()); AVInRuntimeImplOkayHolder AVOkay; // don't validate the next object if a BGC is in progress. we can race with background // sweep which could make the next object a Free object underneath us if it's dead. @@ -1028,18 +1030,16 @@ FCIMPL3(void, StubHelpers::ValidateObject, Object *pObjUNSAFE, MethodDesc *pMD, } EX_CATCH { + Object* pThisUNSAFE = OBJECTREFToObject(pThis.Get()); FormatValidationMessage(ResolveInteropMethod(pThisUNSAFE, pMD), errorString); EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_EXECUTIONENGINE, errorString.GetUnicode()); } EX_END_CATCH_UNREACHABLE; - - HELPER_METHOD_FRAME_END(); + END_QCALL; #else // VERIFY_HEAP - FCUnique(0xa3); UNREACHABLE_MSG("No validation support without VERIFY_HEAP"); #endif // VERIFY_HEAP } -FCIMPLEND FCIMPL3(void, StubHelpers::ValidateByref, void *pByref, MethodDesc *pMD, Object *pThisUNSAFE) { diff --git a/src/coreclr/src/vm/stubhelpers.h b/src/coreclr/src/vm/stubhelpers.h index 9a133069024cce..974fb30e978fc4 100644 --- a/src/coreclr/src/vm/stubhelpers.h +++ b/src/coreclr/src/vm/stubhelpers.h @@ -86,7 +86,7 @@ class StubHelpers static FCDECL0(void*, GetStubContextAddr); #endif // TARGET_64BIT static FCDECL1(DWORD, CalcVaListSize, VARARGS *varargs); - static FCDECL3(void, ValidateObject, Object *pObjUNSAFE, MethodDesc *pMD, Object *pThisUNSAFE); + static void QCALLTYPE ValidateObject(QCall::ObjectHandleOnStack object, MethodDesc *pMD, QCall::ObjectHandleOnStack pThis); static FCDECL3(void, ValidateByref, void *pByref, MethodDesc *pMD, Object *pThisUNSAFE); #ifdef PROFILING_SUPPORTED