From 1b10901766ddb6b57e769e2dea04ad6e8e91160d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 23 Jul 2018 03:30:57 -0700 Subject: [PATCH] Remove hosthook api (dotnet/coreclr#19079) * Remove CallNeedsHostHook() API * Remove IsHostHookEnabled() API and with it related dead code * Remove code enabling host hooks (i.e. COMPlus_GenerateStubForHost) Remove function declarations for creating host hooks Update comments Commit migrated from https://github.com/dotnet/coreclr/commit/92d2c4bde42569d2aa22e44550d69f7d743bf9a0 --- .../project-docs/clr-configuration-knobs.md | 3 +- src/coreclr/src/inc/clrconfigvalues.h | 1 - src/coreclr/src/vm/amd64/UMThunkStub.asm | 1 - src/coreclr/src/vm/comdelegate.cpp | 7 +-- src/coreclr/src/vm/comdelegate.h | 1 - src/coreclr/src/vm/compile.cpp | 5 -- src/coreclr/src/vm/dllimport.cpp | 57 ++-------------------- src/coreclr/src/vm/dllimport.h | 22 --------- src/coreclr/src/vm/eeconfig.cpp | 5 +- src/coreclr/src/vm/eeconfig.h | 4 -- src/coreclr/src/vm/i386/cgenx86.cpp | 14 +----- src/coreclr/src/vm/method.hpp | 4 +- src/coreclr/src/vm/stubhelpers.cpp | 1 - src/coreclr/src/vm/syncblk.cpp | 4 +- src/coreclr/src/vm/syncblk.h | 6 +-- 15 files changed, 14 insertions(+), 121 deletions(-) diff --git a/docs/coreclr/project-docs/clr-configuration-knobs.md b/docs/coreclr/project-docs/clr-configuration-knobs.md index ea2bb04..7a6c4e6 100644 --- a/docs/coreclr/project-docs/clr-configuration-knobs.md +++ b/docs/coreclr/project-docs/clr-configuration-knobs.md @@ -288,8 +288,7 @@ Name | Description | Type | Class | Default Value | Flags `EventPipeConfig` | Configuration for EventPipe. | `STRING` | `INTERNAL` | | `EventPipeOutputFile` | The full path including file name for the trace file that will be written when COMPlus_EnableEventPipe&=1 | `STRING` | `INTERNAL` | | `EventPipeRundown` | Enable/disable eventpipe rundown. | `DWORD` | `INTERNAL` | `1` | -`ExposeExceptionsInCOM` | | `DWORD` | `INTERNAL` | | -`GenerateStubForHost` | Forces the host hook stub to be built for all unmanaged calls, even when not running hosted. | `DWORD` | `INTERNAL` | `0` | +`ExposeExceptionsInCOM` | | `DWORD` | `INTERNAL` | | `InteropLogArguments` | Log all pinned arguments passed to an interop call | `DWORD` | `EXTERNAL` | `0` | `InteropValidatePinnedObjects` | After returning from a managed-to-unmanaged interop call, validate GC heap around objects pinned by IL stubs. | `DWORD` | `UNSUPPORTED` | `0` | `legacyComHierarchyVisibility` | | `DWORD` | `EXTERNAL` | | diff --git a/src/coreclr/src/inc/clrconfigvalues.h b/src/coreclr/src/inc/clrconfigvalues.h index 503d29d..fbcc967 100644 --- a/src/coreclr/src/inc/clrconfigvalues.h +++ b/src/coreclr/src/inc/clrconfigvalues.h @@ -723,7 +723,6 @@ RETAIL_CONFIG_STRING_INFO(INTERNAL_EventNameFilter, W("EventNameFilter"), "") /// CONFIG_DWORD_INFO_DIRECT_ACCESS(INTERNAL_ExposeExceptionsInCOM, W("ExposeExceptionsInCOM"), "") RETAIL_CONFIG_DWORD_INFO(EXTERNAL_ComInsteadOfManagedRemoting, W("PreferComInsteadOfManagedRemoting"), 0, "When communicating with a cross app domain CCW, use COM instead of managed remoting.") -CONFIG_DWORD_INFO(INTERNAL_GenerateStubForHost, W("GenerateStubForHost"), 0, "Forces the host hook stub to be built for all unmanaged calls, even when not running hosted.") RETAIL_CONFIG_DWORD_INFO_DIRECT_ACCESS(EXTERNAL_legacyComHierarchyVisibility, W("legacyComHierarchyVisibility"), "") RETAIL_CONFIG_DWORD_INFO_DIRECT_ACCESS(EXTERNAL_legacyComVTableLayout, W("legacyComVTableLayout"), "") RETAIL_CONFIG_DWORD_INFO_DIRECT_ACCESS(EXTERNAL_newComVTableLayout, W("newComVTableLayout"), "") diff --git a/src/coreclr/src/vm/amd64/UMThunkStub.asm b/src/coreclr/src/vm/amd64/UMThunkStub.asm index 8b44e67..123a630 100644 --- a/src/coreclr/src/vm/amd64/UMThunkStub.asm +++ b/src/coreclr/src/vm/amd64/UMThunkStub.asm @@ -12,7 +12,6 @@ include include AsmConstants.inc gfHostConfig equ ?g_fHostConfig@@3KA -NDirect__IsHostHookEnabled equ ?IsHostHookEnabled@NDirect@@SAHXZ extern CreateThreadBlockThrow:proc extern TheUMEntryPrestubWorker:proc diff --git a/src/coreclr/src/vm/comdelegate.cpp b/src/coreclr/src/vm/comdelegate.cpp index 5db7ce7..b64802c 100644 --- a/src/coreclr/src/vm/comdelegate.cpp +++ b/src/coreclr/src/vm/comdelegate.cpp @@ -1433,15 +1433,12 @@ OBJECTREF COMDelegate::ConvertToDelegate(LPVOID pCallback, MethodTable* pMT) MethodDesc *pStubMD = pClass->m_pForwardStubMD; _ASSERTE(pStubMD != NULL && pStubMD->IsILStub()); - -#ifdef MDA_SUPPORTED +#if defined(MDA_SUPPORTED) if (MDA_GET_ASSISTANT(PInvokeStackImbalance)) { pInterceptStub = GenerateStubForMDA(pMD, pStubMD, pCallback, pInterceptStub); } #endif // MDA_SUPPORTED - - } if (pInterceptStub != NULL) @@ -1454,7 +1451,7 @@ OBJECTREF COMDelegate::ConvertToDelegate(LPVOID pCallback, MethodTable* pMT) } GCPROTECT_END(); -#endif // defined(_TARGET_X86_) +#endif // _TARGET_X86_ return delObj; } diff --git a/src/coreclr/src/vm/comdelegate.h b/src/coreclr/src/vm/comdelegate.h index 1bfe8ac..1f6d10b 100644 --- a/src/coreclr/src/vm/comdelegate.h +++ b/src/coreclr/src/vm/comdelegate.h @@ -117,7 +117,6 @@ public: #ifdef MDA_SUPPORTED static Stub *GenerateStubForMDA(MethodDesc *pInvokeMD, MethodDesc *pStubMD, LPVOID pNativeTarget, Stub *pInnerStub); #endif // MDA_SUPPORTED - static Stub *GenerateStubForHost(MethodDesc *pInvokeMD, MethodDesc *pStubMD, LPVOID pNativeTarget, Stub *pInnerStub); #endif // _TARGET_X86_ static MethodDesc * __fastcall GetMethodDesc(OBJECTREF obj); diff --git a/src/coreclr/src/vm/compile.cpp b/src/coreclr/src/vm/compile.cpp index 5244052..996f61f 100644 --- a/src/coreclr/src/vm/compile.cpp +++ b/src/coreclr/src/vm/compile.cpp @@ -6980,11 +6980,6 @@ void CompilationDomain::Init() #endif SetCompilationDomain(); - - -#ifdef _DEBUG - g_pConfig->DisableGenerateStubForHost(); -#endif } HRESULT CompilationDomain::AddDependencyEntry(PEAssembly *pFile, diff --git a/src/coreclr/src/vm/dllimport.cpp b/src/coreclr/src/vm/dllimport.cpp index d5376f1..c45b84d 100644 --- a/src/coreclr/src/vm/dllimport.cpp +++ b/src/coreclr/src/vm/dllimport.cpp @@ -800,14 +800,6 @@ public: m_slIL.AdjustTargetStackDeltaForExtraParam(); } -#if defined(_TARGET_X86_) - // unmanaged CALLI will get an extra arg with the real target address if host hook is enabled - if (SF_IsCALLIStub(m_dwStubFlags) && NDirect::IsHostHookEnabled()) - { - pcsMarshal->SetStubTargetArgType(ELEMENT_TYPE_I, false); - } -#endif // _TARGET_X86_ - // Don't touch target signatures from this point on otherwise it messes up the // cache in ILStubState::GetStubTargetMethodSig. @@ -1396,12 +1388,7 @@ public: BinderMethodID getCOMIPMethod; bool fDoPostCallIPCleanup = true; - if (!SF_IsNGENedStub(dwStubFlags) && NDirect::IsHostHookEnabled()) - { - // always use the non-optimized helper if we are hosted - getCOMIPMethod = METHOD__STUBHELPERS__GET_COM_IP_FROM_RCW; - } - else if (SF_IsWinRTStub(dwStubFlags)) + if (SF_IsWinRTStub(dwStubFlags)) { // WinRT uses optimized helpers if (SF_IsWinRTSharedGenericStub(dwStubFlags)) @@ -5196,17 +5183,6 @@ MethodDesc* GetStubMethodDescFromInteropMethodDesc(MethodDesc* pMD, DWORD dwStub } #endif // MDA_SUPPORTED - if (NDirect::IsHostHookEnabled()) - { - MethodTable *pMT = pMD->GetMethodTable(); - if (pMT->IsProjectedFromWinRT() || pMT->IsWinRTRedirectedInterface(TypeHandle::Interop_ManagedToNative)) - { - // WinRT NGENed stubs are optimized for the non-hosted scenario and - // must be rejected if we are hosted. - return NULL; - } - } - if (fGcMdaEnabled) return NULL; @@ -5229,13 +5205,6 @@ MethodDesc* GetStubMethodDescFromInteropMethodDesc(MethodDesc* pMD, DWORD dwStub #ifdef FEATURE_COMINTEROP if (SF_IsWinRTDelegateStub(dwStubFlags)) { - if (NDirect::IsHostHookEnabled() && pMD->GetMethodTable()->IsProjectedFromWinRT()) - { - // WinRT NGENed stubs are optimized for the non-hosted scenario and - // must be rejected if we are hosted. - return NULL; - } - return pClass->m_pComPlusCallInfo->m_pStubMD.GetValueMaybeNull(); } else @@ -5592,33 +5561,13 @@ VOID NDirectMethodDesc::SetNDirectTarget(LPVOID pTarget) Stub *pInterceptStub = NULL; - BOOL fHook = FALSE; - - // Host hooks are not supported for Mac CoreCLR. - if (NDirect::IsHostHookEnabled()) - { -#ifdef _WIN64 - // we will call CallNeedsHostHook on every invocation for back compat - fHook = TRUE; -#else // _WIN64 - fHook = CallNeedsHostHook((size_t)pTarget); -#endif // _WIN64 - -#ifdef _DEBUG - if (g_pConfig->ShouldGenerateStubForHost()) - { - fHook = TRUE; - } -#endif - } - #ifdef _TARGET_X86_ #ifdef MDA_SUPPORTED if (!IsQCall() && MDA_GET_ASSISTANT(PInvokeStackImbalance)) { - pInterceptStub = GenerateStubForMDA(pTarget, pInterceptStub, fHook); + pInterceptStub = GenerateStubForMDA(pTarget, pInterceptStub); } #endif // MDA_SUPPORTED @@ -5630,7 +5579,7 @@ VOID NDirectMethodDesc::SetNDirectTarget(LPVOID pTarget) EnsureWritablePages(pWriteableData); g_IBCLogger.LogNDirectCodeAccess(this); - if (pInterceptStub != NULL WIN64_ONLY(|| fHook)) + if (pInterceptStub != NULL) { ndirect.m_pNativeNDirectTarget = pTarget; diff --git a/src/coreclr/src/vm/dllimport.h b/src/coreclr/src/vm/dllimport.h index 058484c..d7bdda1 100644 --- a/src/coreclr/src/vm/dllimport.h +++ b/src/coreclr/src/vm/dllimport.h @@ -118,11 +118,6 @@ public: inline static ILStubCache* GetILStubCache(NDirectStubParameters* pParams); - - static BOOL IsHostHookEnabled(); - - static Stub *GenerateStubForHost(Module *pModule, CorUnmanagedCallingConvention callConv, WORD wArgSize); - private: NDirect() {LIMITED_METHOD_CONTRACT;}; // prevent "new"'s on this class @@ -625,23 +620,6 @@ PCODE GetStubForInteropMethod(MethodDesc* pMD, DWORD dwStubFlags = 0, MethodDesc HRESULT FindPredefinedILStubMethod(MethodDesc *pTargetMD, DWORD dwStubFlags, MethodDesc **ppRetStubMD); #endif // FEATURE_COMINTEROP -EXTERN_C BOOL CallNeedsHostHook(size_t target); - -// -// Inlinable implementation allows compiler to strip all code related to host hook -// -inline BOOL NDirect::IsHostHookEnabled() -{ - LIMITED_METHOD_CONTRACT; - return FALSE; -} - -inline BOOL CallNeedsHostHook(size_t target) -{ - LIMITED_METHOD_CONTRACT; - return FALSE; -} - // // Limit length of string field in IL stub ETW events so that the whole // IL stub ETW events won't exceed 64KB diff --git a/src/coreclr/src/vm/eeconfig.cpp b/src/coreclr/src/vm/eeconfig.cpp index da4df94..41f80d8 100644 --- a/src/coreclr/src/vm/eeconfig.cpp +++ b/src/coreclr/src/vm/eeconfig.cpp @@ -343,7 +343,6 @@ HRESULT EEConfig::Init() iGCPollType = GCPOLL_TYPE_DEFAULT; #ifdef _DEBUG - fGenerateStubForHost = FALSE; fShouldInjectFault = 0; testThreadAbort = 0; testADUnload = 0; @@ -1171,9 +1170,7 @@ HRESULT EEConfig::sync() IfFailRet(ParseTypeList(wszPerfTypes, &pPerfTypesToLog)); iPerfNumAllocsThreshold = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_PerfNumAllocsThreshold); - iPerfAllocsSizeThreshold = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_PerfAllocsSizeThreshold); - - fGenerateStubForHost = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_GenerateStubForHost); + iPerfAllocsSizeThreshold = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_PerfAllocsSizeThreshold); fShouldInjectFault = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_InjectFault); diff --git a/src/coreclr/src/vm/eeconfig.h b/src/coreclr/src/vm/eeconfig.h index e30c0e2..bcd767b 100644 --- a/src/coreclr/src/vm/eeconfig.h +++ b/src/coreclr/src/vm/eeconfig.h @@ -822,9 +822,6 @@ public: GCPollType GetGCPollType() { LIMITED_METHOD_CONTRACT; return iGCPollType; } #ifdef _DEBUG - BOOL ShouldGenerateStubForHost() const {LIMITED_METHOD_CONTRACT; return fGenerateStubForHost;} - void DisableGenerateStubForHost() {LIMITED_METHOD_CONTRACT; fGenerateStubForHost = FALSE;} - DWORD GetHostTestADUnload() const {LIMITED_METHOD_CONTRACT; return testADUnload;} DWORD GetHostTestThreadAbort() const {LIMITED_METHOD_CONTRACT; return testThreadAbort;} @@ -1103,7 +1100,6 @@ private: //---------------------------------------------------------------- GCPollType iGCPollType; #ifdef _DEBUG - BOOL fGenerateStubForHost; DWORD fShouldInjectFault; DWORD testADUnload; DWORD testThreadAbort; diff --git a/src/coreclr/src/vm/i386/cgenx86.cpp b/src/coreclr/src/vm/i386/cgenx86.cpp index e222607..c3f76c8 100644 --- a/src/coreclr/src/vm/i386/cgenx86.cpp +++ b/src/coreclr/src/vm/i386/cgenx86.cpp @@ -1048,7 +1048,7 @@ Stub *GenerateInitPInvokeFrameHelper() #ifdef MDA_SUPPORTED //----------------------------------------------------------------------------- -Stub *NDirectMethodDesc::GenerateStubForMDA(LPVOID pNativeTarget, Stub *pInnerStub, BOOL fCalledByStub) +Stub *NDirectMethodDesc::GenerateStubForMDA(LPVOID pNativeTarget, Stub *pInnerStub) { STANDARD_VM_CONTRACT; @@ -1079,17 +1079,7 @@ Stub *NDirectMethodDesc::GenerateStubForMDA(LPVOID pNativeTarget, Stub *pInnerSt if (IsVarArgs()) { // Re-push the return address as an argument to GetStackSizeForVarArgCall() - if (fCalledByStub) - { - // We will be called by another stub that doesn't know the stack size, - // so we need to skip a frame to get to the managed caller. - sl.X86EmitIndexRegLoad(kEAX, kEBP, 0); - sl.X86EmitIndexPush(kEAX, 4); - } - else - { - sl.X86EmitIndexPush(kEBP, 4); - } + sl.X86EmitIndexPush(kEBP, 4); // This will return the number of stack arguments (in DWORDs) sl.X86EmitCall(sl.NewExternalCodeLabel((LPVOID)GetStackSizeForVarArgCall), 4); diff --git a/src/coreclr/src/vm/method.hpp b/src/coreclr/src/vm/method.hpp index 3817b05..943bcdb 100644 --- a/src/coreclr/src/vm/method.hpp +++ b/src/coreclr/src/vm/method.hpp @@ -2870,9 +2870,8 @@ public: LPVOID FindEntryPoint(HINSTANCE hMod) const; private: - Stub* GenerateStubForHost(LPVOID pNativeTarget, Stub *pInnerStub); #ifdef MDA_SUPPORTED - Stub* GenerateStubForMDA(LPVOID pNativeTarget, Stub *pInnerStub, BOOL fCalledByStub); + Stub* GenerateStubForMDA(LPVOID pNativeTarget, Stub *pInnerStub); #endif // MDA_SUPPORTED public: @@ -3055,7 +3054,6 @@ struct ComPlusCallInfo LPVOID m_pInterceptStub; // used for early-bound IL stub calls }; - Stub *GenerateStubForHost(LoaderHeap *pHeap, Stub *pInnerStub); #else // _TARGET_X86_ void InitStackArgumentSize() { diff --git a/src/coreclr/src/vm/stubhelpers.cpp b/src/coreclr/src/vm/stubhelpers.cpp index 0be8be3..1b168b9 100644 --- a/src/coreclr/src/vm/stubhelpers.cpp +++ b/src/coreclr/src/vm/stubhelpers.cpp @@ -284,7 +284,6 @@ FORCEINLINE static void *GetCOMIPFromRCW_GetTargetNoInterception(IUnknown *pUnk, _ASSERTE(pComInfo->m_pInterceptStub == NULL || pComInfo->m_pInterceptStub == (LPVOID)-1); _ASSERTE(!pComInfo->HasCopyCtorArgs()); #endif // _TARGET_X86_ - _ASSERTE(!NDirect::IsHostHookEnabled()); LPVOID *lpVtbl = *(LPVOID **)pUnk; return lpVtbl[pComInfo->m_cachedComSlot]; diff --git a/src/coreclr/src/vm/syncblk.cpp b/src/coreclr/src/vm/syncblk.cpp index eba84e9..47922c2 100644 --- a/src/coreclr/src/vm/syncblk.cpp +++ b/src/coreclr/src/vm/syncblk.cpp @@ -135,11 +135,9 @@ void InteropSyncBlockInfo::FreeUMEntryThunkOrInterceptStub() { #if defined(_TARGET_X86_) Stub *pInterceptStub = GetInterceptStub(); - if (pInterceptStub != NULL) { - // There may be multiple chained stubs, i.e. host hook stub calling MDA stack - // imbalance stub, and the following DecRef will free all of them. + // There may be multiple chained stubs pInterceptStub->DecRef(); } #else // _TARGET_X86_ diff --git a/src/coreclr/src/vm/syncblk.h b/src/coreclr/src/vm/syncblk.h index c3f8a84..fcfa352 100644 --- a/src/coreclr/src/vm/syncblk.h +++ b/src/coreclr/src/vm/syncblk.h @@ -786,9 +786,9 @@ private: // to the thunk generated for unmanaged code to call back on. // If this is a delegate representing an unmanaged function pointer, // this may point to a stub that intercepts calls to the unmng target. - // It is currently used for pInvokeStackImbalance MDA and host hook. - // We differentiate between the two by setting the lowest bit if it's - // an intercept stub. + // An example of an intercept call is pInvokeStackImbalance MDA. + // We differentiate between a thunk or intercept stub by setting the lowest + // bit if it is an intercept stub. void* m_pUMEntryThunkOrInterceptStub; #ifdef FEATURE_COMINTEROP -- 2.7.4