From c68018d1c2403eddad7f22cbfbe9b46700cfdc9c Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:24:50 -0700 Subject: [PATCH 01/16] Moving parsing from TypeNameParser ctor to a separate method. It seems a bit odd to have the constructor parsing and then use a dummy method (MakeRotorHappy) to make it look more normal. --- src/vm/typeparse.h | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/vm/typeparse.h b/src/vm/typeparse.h index a2ec689814db..7985527701f7 100644 --- a/src/vm/typeparse.h +++ b/src/vm/typeparse.h @@ -121,7 +121,7 @@ class TypeName : public ITypeName private: class TypeNameParser { - TypeNameParser(LPCWSTR szTypeName, TypeName* pTypeName, DWORD* pError) + TypeNameParser(LPCWSTR szTypeName, TypeName* pTypeName) { CONTRACTL { @@ -139,13 +139,9 @@ class TypeName : public ITypeName m_currentToken = TypeNameEmpty; m_nextToken = TypeNameEmpty; - *pError = (DWORD)-1; m_pTypeName = pTypeName; m_sszTypeName = szTypeName; - m_currentItr = m_itr = m_sszTypeName; - - if (!START()) - *pError = (DWORD)(m_currentItr - m_sszTypeName) - 1; + m_currentItr = m_itr = m_sszTypeName; } private: @@ -264,7 +260,20 @@ class TypeName : public ITypeName // id '+' NESTNAME public: - void MakeRotorHappy() { WRAPPER_NO_CONTRACT; } + void Parse(DWORD* pError) + { + CONTRACTL + { + THROWS; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + *pError = (DWORD)-1; + if (!START()) + *pError = (DWORD)(m_currentItr - m_sszTypeName) - 1; + } private: TypeName* m_pTypeName; @@ -300,8 +309,8 @@ class TypeName : public ITypeName MODE_ANY; } CONTRACTL_END; - TypeNameParser parser(szTypeName, this, pError); - parser.MakeRotorHappy(); + TypeNameParser parser(szTypeName, this); + parser.Parse(pError); } virtual ~TypeName(); From a4a1f13490cf297f0ea9398a64b0bfe66c1d7bd0 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:28:54 -0700 Subject: [PATCH 02/16] Remove CorMarkThreadInThreadPool. It is neither referenced nor exported. --- src/inc/MSCOREE.IDL | 1 - src/pal/prebuilt/inc/mscoree.h | 1 - src/vm/threads.cpp | 14 -------------- 3 files changed, 16 deletions(-) diff --git a/src/inc/MSCOREE.IDL b/src/inc/MSCOREE.IDL index e23e847153aa..5917e4c0974c 100644 --- a/src/inc/MSCOREE.IDL +++ b/src/inc/MSCOREE.IDL @@ -105,7 +105,6 @@ cpp_quote("EXTERN_GUID(IID_ITypeNameFactory, 0xB81FF171, 0x20F3, 0x11d2, 0x8d, 0 #pragma midl_echo("DEPRECATED_CLR_STDAPI CorBindToRuntime(LPCWSTR pwszVersion, LPCWSTR pwszBuildFlavor, REFCLSID rclsid, REFIID riid, LPVOID FAR *ppv);") #pragma midl_echo("DEPRECATED_CLR_STDAPI CorBindToCurrentRuntime(LPCWSTR pwszFileName, REFCLSID rclsid, REFIID riid, LPVOID FAR *ppv);") #pragma midl_echo("DEPRECATED_CLR_STDAPI ClrCreateManagedInstance(LPCWSTR pTypeName, REFIID riid, void **ppObject);") -#pragma midl_echo("DECLARE_DEPRECATED void STDMETHODCALLTYPE CorMarkThreadInThreadPool();") #pragma midl_echo("DEPRECATED_CLR_STDAPI RunDll32ShimW(HWND hwnd, HINSTANCE hinst, LPCWSTR lpszCmdLine, int nCmdShow);") #pragma midl_echo("DEPRECATED_CLR_STDAPI LoadLibraryShim(LPCWSTR szDllName, LPCWSTR szVersion, LPVOID pvReserved, HMODULE *phModDll);") #pragma midl_echo("DEPRECATED_CLR_STDAPI CallFunctionShim(LPCWSTR szDllName, LPCSTR szFunctionName, LPVOID lpvArgument1, LPVOID lpvArgument2, LPCWSTR szVersion, LPVOID pvReserved);") diff --git a/src/pal/prebuilt/inc/mscoree.h b/src/pal/prebuilt/inc/mscoree.h index a03058564e0c..c8ac97ec041a 100644 --- a/src/pal/prebuilt/inc/mscoree.h +++ b/src/pal/prebuilt/inc/mscoree.h @@ -263,7 +263,6 @@ DEPRECATED_CLR_STDAPI CorBindToRuntimeByCfg(IStream* pCfgStream, DWORD reserved, DEPRECATED_CLR_STDAPI CorBindToRuntime(LPCWSTR pwszVersion, LPCWSTR pwszBuildFlavor, REFCLSID rclsid, REFIID riid, LPVOID FAR *ppv); DEPRECATED_CLR_STDAPI CorBindToCurrentRuntime(LPCWSTR pwszFileName, REFCLSID rclsid, REFIID riid, LPVOID FAR *ppv); DEPRECATED_CLR_STDAPI ClrCreateManagedInstance(LPCWSTR pTypeName, REFIID riid, void **ppObject); -DECLARE_DEPRECATED void STDMETHODCALLTYPE CorMarkThreadInThreadPool(); DEPRECATED_CLR_STDAPI RunDll32ShimW(HWND hwnd, HINSTANCE hinst, LPCWSTR lpszCmdLine, int nCmdShow); DEPRECATED_CLR_STDAPI LoadLibraryShim(LPCWSTR szDllName, LPCWSTR szVersion, LPVOID pvReserved, HMODULE *phModDll); DEPRECATED_CLR_STDAPI CallFunctionShim(LPCWSTR szDllName, LPCSTR szFunctionName, LPVOID lpvArgument1, LPVOID lpvArgument2, LPCWSTR szVersion, LPVOID pvReserved); diff --git a/src/vm/threads.cpp b/src/vm/threads.cpp index 8557f9c9f388..e6f7c45f5ea7 100644 --- a/src/vm/threads.cpp +++ b/src/vm/threads.cpp @@ -815,20 +815,6 @@ Thread* SetupThread(BOOL fInternal) return pThread; } -//------------------------------------------------------------------------- -void STDMETHODCALLTYPE CorMarkThreadInThreadPool() -{ - LIMITED_METHOD_CONTRACT; - BEGIN_ENTRYPOINT_VOIDRET; - END_ENTRYPOINT_VOIDRET; - - // this is no longer needed after our switch to - // the Win32 threadpool. - // keeping in mscorwks for compat reasons and to keep rotor sscoree and - // mscoree consistent. -} - - //------------------------------------------------------------------------- // Public function: SetupUnstartedThread() // This sets up a Thread object for an exposed System.Thread that From dcb003829e158d328e0a1a0f1abc2bdae33a6876 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:30:14 -0700 Subject: [PATCH 03/16] Remove reference to rotor from securitywrapper.h --- src/debug/ee/rcthread.cpp | 3 ++- src/inc/securitywrapper.h | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/debug/ee/rcthread.cpp b/src/debug/ee/rcthread.cpp index ebdd720475e6..7ea362451fa7 100644 --- a/src/debug/ee/rcthread.cpp +++ b/src/debug/ee/rcthread.cpp @@ -12,8 +12,9 @@ #include "stdafx.h" - +#ifndef FEATURE_PAL #include "securitywrapper.h" +#endif #include #include diff --git a/src/inc/securitywrapper.h b/src/inc/securitywrapper.h index a14d90a9224b..0cb5f8d6cc47 100644 --- a/src/inc/securitywrapper.h +++ b/src/inc/securitywrapper.h @@ -12,7 +12,9 @@ #ifndef _SECURITY_WRAPPER_H #define _SECURITY_WRAPPER_H -// This file should not even be included on Rotor. +#ifdef FEATURE_PAL +#error This file should not be included on non-Windows platforms. +#endif //----------------------------------------------------------------------------- // Wrapper around a PSID. From 5a1c334c172d209166d52755d49c62a55975ff23 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:32:31 -0700 Subject: [PATCH 04/16] Remove reference to rotor from Strike/vm.cpp. This file is only built for Windows. --- src/ToolBox/SOS/Strike/vm.cpp | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/src/ToolBox/SOS/Strike/vm.cpp b/src/ToolBox/SOS/Strike/vm.cpp index 70e9210dbd94..7eb6e93cdebe 100644 --- a/src/ToolBox/SOS/Strike/vm.cpp +++ b/src/ToolBox/SOS/Strike/vm.cpp @@ -2,11 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -// ==++== -// - -// -// ==--== /*++ Module Name: @@ -22,7 +17,10 @@ Revision History: --*/ -#ifndef FEATURE_PAL +#ifdef FEATURE_PAL +#error This file is Win32 only. +#endif // #ifdef FEATURE_PAL + #include @@ -712,21 +710,3 @@ Return Value: } } // DECLARE_API( vmmap ) - -#else - -#include -#include - -void vmstat() -{ - assert(false); -} - -void vmmap() -{ - - assert(false); -} - -#endif // #ifndef FEATURE_PAL From 33802192f8bd6c20abc078f1aa395d8be511cbe0 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:33:21 -0700 Subject: [PATCH 05/16] Remove reference to rotor from debugreturn.h This is the only file the defines these macros, so there is no need to undef them first. --- src/inc/debugreturn.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/inc/debugreturn.h b/src/inc/debugreturn.h index e5013ccabe59..73edd010ef81 100644 --- a/src/inc/debugreturn.h +++ b/src/inc/debugreturn.h @@ -105,10 +105,6 @@ typedef __SafeToReturn __ReturnOK; #define DEBUG_ASSURE_NO_RETURN_BEGIN(arg) { typedef __YouCannotUseAReturnStatementHere __ReturnOK; if (0 && __ReturnOK::used()) { } else { #define DEBUG_ASSURE_NO_RETURN_END(arg) } } -// rotor_pal.h defaulted these to empty macros; this file redefines them -#undef DEBUG_OK_TO_RETURN_BEGIN -#undef DEBUG_OK_TO_RETURN_END - #define DEBUG_OK_TO_RETURN_BEGIN(arg) { typedef __SafeToReturn __ReturnOK; if (0 && __ReturnOK::used()) { } else { #define DEBUG_OK_TO_RETURN_END(arg) } } From 9bf8a7b145cafb78cfd7de52cabb4e5cc50ca248 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:34:01 -0700 Subject: [PATCH 06/16] Remove unused code refering to rotor from PAL. --- src/pal/inc/rt/common.ver | 33 --------------- src/pal/inc/rt/palrt.h | 54 ------------------------ src/pal/src/include/pal/dtraceprotocol.h | 39 ----------------- 3 files changed, 126 deletions(-) delete mode 100644 src/pal/inc/rt/common.ver delete mode 100644 src/pal/src/include/pal/dtraceprotocol.h diff --git a/src/pal/inc/rt/common.ver b/src/pal/inc/rt/common.ver deleted file mode 100644 index 9007a3de2d25..000000000000 --- a/src/pal/inc/rt/common.ver +++ /dev/null @@ -1,33 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -#include "fxver.h" - -VS_VERSION_INFO VERSIONINFO - -FILEVERSION VER_FILEVERSION -PRODUCTVERSION VER_PRODUCTVERSION -FILEFLAGSMASK VER_FILEFLAGSMASK -FILEFLAGS VER_FILEFLAGS -FILEOS VER_FILEOS -FILETYPE VER_FILETYPE -FILESUBTYPE VER_FILESUBTYPE - -BEGIN - BLOCK "StringFileInfo" - BEGIN - BLOCK VER_VERSION_UNICODE_LANG - BEGIN - VALUE "CompanyName", VER_COMPANYNAME_STR - VALUE "LegalCopyright", VER_LEGALCOPYRIGHT_STR - VALUE "FileVersion", VER_FILEVERSION_STR - VALUE "FileDescription", "Rotor File" - END - END - - BLOCK "VarFileInfo" - BEGIN - VALUE "Translation", VER_VERSION_TRANSLATION - END -END diff --git a/src/pal/inc/rt/palrt.h b/src/pal/inc/rt/palrt.h index 6a1d57c0ee46..36c9270de17b 100644 --- a/src/pal/inc/rt/palrt.h +++ b/src/pal/inc/rt/palrt.h @@ -151,60 +151,6 @@ inline void *__cdecl operator new(size_t, void *_P) #include -#if defined(_DEBUG) -#define ROTOR_PAL_CTOR_TEST_BODY(TESTNAME) \ - class TESTNAME ## _CTOR_TEST { \ - public: \ - class HelperClass { \ - public: \ - HelperClass(const char *String) { \ - _ASSERTE (m_s == NULL); \ - m_s = String; \ - } \ - \ - void Validate (const char *String) { \ - _ASSERTE (m_s); \ - _ASSERTE (m_s == String); \ - _ASSERTE (!strncmp ( \ - m_s, \ - String, \ - 1000)); \ - } \ - \ - private: \ - const char *m_s; \ - }; \ - \ - TESTNAME ## _CTOR_TEST() { \ - _ASSERTE (m_This == NULL); \ - m_This = this; \ - } \ - \ - void Validate () { \ - _ASSERTE (m_This == this); \ - m_String.Validate(#TESTNAME "_CTOR_TEST"); \ - } \ - \ - private: \ - void *m_This; \ - static HelperClass m_String; \ - }; \ - \ - static TESTNAME ## _CTOR_TEST \ - g_ ## TESTNAME ## _CTOR_TEST; \ - TESTNAME ## _CTOR_TEST::HelperClass \ - TESTNAME ## _CTOR_TEST::m_String(#TESTNAME "_CTOR_TEST"); - -#define ROTOR_PAL_CTOR_TEST_RUN(TESTNAME) \ - g_ ## TESTNAME ##_CTOR_TEST.Validate() - -#else // DEBUG - -#define ROTOR_PAL_CTOR_TEST_BODY(TESTNAME) -#define ROTOR_PAL_CTOR_TEST_RUN(TESTNAME) do {} while (0) - -#endif // DEBUG - #define NTAPI __cdecl #define WINAPI __cdecl #define CALLBACK __cdecl diff --git a/src/pal/src/include/pal/dtraceprotocol.h b/src/pal/src/include/pal/dtraceprotocol.h deleted file mode 100644 index d1a17a71aeda..000000000000 --- a/src/pal/src/include/pal/dtraceprotocol.h +++ /dev/null @@ -1,39 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. -// -// File: rotor/pal/corunix/include/pal/dtrace_protocal.h -// - -// -// Header file for the protocals between CLR and Dtrace server -// -// ====================================================================================== - -#ifndef DTRACE_PROTOCOL_H -#define DTRACE_PROTOCOL_H - -// Start DTrace Consumer by Unix Domain App -#define kServerSocketPath "/Library/Application Support/com.microsoft.clr.CFDtraceServer/Socket" -#define kPacketTypeStartDtrace 1 -#define kPacketTypeReply 3 -#define kMaxMessageSize 318 -#define kPacketMaximumSize 102400 - -struct PacketHeader { - int fType; // for request from client to server, it should be kPacketTypeStartDtrace - // for reply from server to client, it should be kPacketTypeReply - unsigned int fSize; // includes size of header itself -}; - -struct PacketStartDTrace { // reply: PacketReply - PacketHeader fHeader; // fType is kPacketTypeStartDtrace - char fMessage[kMaxMessageSize]; // message to print -}; - -struct PacketReply { // reply: n/a - PacketHeader fHeader; // fType is kPacketTypeReply - int fErr; // result of operation, errno-style -}; - -#endif // DTRACE_PROTOCOL From e250696be8d3d3b2e345f1b1e464c32a7ce6beb0 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:34:30 -0700 Subject: [PATCH 07/16] Remove references to Rotor from PAL. --- src/pal/inc/rt/vsassert.h | 10 +++++----- src/pal/tests/palsuite/c_runtime/errno/test1/test1.cpp | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pal/inc/rt/vsassert.h b/src/pal/inc/rt/vsassert.h index 2a7fa5fd925c..3c50e5925e10 100644 --- a/src/pal/inc/rt/vsassert.h +++ b/src/pal/inc/rt/vsassert.h @@ -55,18 +55,18 @@ do { \ #define VSAlloc(cb) HeapAlloc(GetProcessHeap(), 0, cb) #define VSAllocZero(cb) HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, cb) #define VSRealloc(pv, cb) HeapReAlloc(GetProcessHeap(), 0, pv, cb) -#define VSReallocZero(pv,cb) Rotors_pal_doesnt_have_vsrealloczero +#define VSReallocZero(pv,cb) CoreCLR_pal_doesnt_have_vsrealloczero #define VSFree(pv) HeapFree(GetProcessHeap(), 0, pv) -#define VSSize(pv) Rotors_pal_doesnt_have_heapsize +#define VSSize(pv) CoreCLR_pal_doesnt_have_heapsize #define VsDebAlloc(flags,cb) VSAlloc(cb) #define VsDebRealloc(pv,flags,cb) VSRealloc(pv,cb) -#define VsDebSafeRealloc(pv,flags,cb) Rotors_pal_doenst_have_saferealloc +#define VsDebSafeRealloc(pv,flags,cb) CoreCLR_pal_doenst_have_saferealloc #define VsDebFree(pv) VSFree(pv) #define VsDebHeapSize(heap, pv) VSSize(pv) -#define VsDebHeapCreate(flags, name) Rotor_doesnt_have_heapcreate -#define VsDebHeapDestroy(heap, fLeakCheck) Rotor_doesnt_have_heapdestroy +#define VsDebHeapCreate(flags, name) CoreCLR_doesnt_have_heapcreate +#define VsDebHeapDestroy(heap, fLeakCheck) CoreCLR_doesnt_have_heapdestroy #define VsDebugAllocInternal(hheap,dwFlags,cb,pszFile,uLine,dwInst,pszExtra) \ HeapAlloc(GetProcessHeap(), dwFlags, cb) diff --git a/src/pal/tests/palsuite/c_runtime/errno/test1/test1.cpp b/src/pal/tests/palsuite/c_runtime/errno/test1/test1.cpp index 3ae25fb02ade..e1dd4a6b36d4 100644 --- a/src/pal/tests/palsuite/c_runtime/errno/test1/test1.cpp +++ b/src/pal/tests/palsuite/c_runtime/errno/test1/test1.cpp @@ -25,7 +25,7 @@ int __cdecl main(int argc, char *argv[]) } /* - From rotor.doc: The only value that must be supported is + The only value that must be supported is ERANGE, in the event that wcstoul() fails due to overflow. */ From c8e383704d388c414bbe2fa8190dea4f2860d505 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:35:02 -0700 Subject: [PATCH 08/16] Remove references to deleted tests from DisabledTests.txt I can't find any evidence that this file is actually used. --- src/pal/tests/palsuite/DisabledTests.txt | 38 ++---------------------- 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/src/pal/tests/palsuite/DisabledTests.txt b/src/pal/tests/palsuite/DisabledTests.txt index b3ca3705867b..36c09d2d9ec4 100644 --- a/src/pal/tests/palsuite/DisabledTests.txt +++ b/src/pal/tests/palsuite/DisabledTests.txt @@ -8,12 +8,12 @@ debug_api/debugbreak/test1 debug_api/outputdebugstringa/test1 debug_api/outputdebugstringw/test1 debug_api/writeprocessmemory/test1 -debug_api/writeprocessmemory/test2 debug_api/writeprocessmemory/test3 debug_api/writeprocessmemory/test4 ======================================= The above testcases were disabled in the palsuite, because they depend heavily on -WaitForDebugEvent,DebugActiveProcess and ContinueDebugEvent, where these api's have been removed from the PAL and listed in the rotor_pal.doc (6.0) +WaitForDebugEvent,DebugActiveProcess and ContinueDebugEvent, where these api's +have been removed from the PAL. file_io/gettempfilenamea/test2 : @@ -86,38 +86,6 @@ pal_specific/pal_get_stdin/test1 : ======================================= This test case should be run manually. Requires user input. - -threading/setconsolectrlhandler/test1 -threading/setconsolectrlhandler/test2 -threading/setconsolectrlhandler/test3 -threading/setconsolectrlhandler/test4 -======================================= -These tests cases should be run manually. Requires user input. - -threading/getprocesstimes/test1 -======================================= -According to rotor_pal.doc, GetProcessTimes() should be supported for the current -process only. I've removed this test because it tests GetProcessTimes() with -invalid process handle value. - -threading/getthreadcontext/test1,1 -======================================= -According to rotor_pal.doc, when GetThreadContext is call for a thread within -the same process, it could only be called for te current thread. I've removed -this test, because it tests GetThreadContext() with a non current thread handle -belong the current procees. - -loader/loadlibrarya/test4 -loader/loadlibraryw/test4 -======================================= -PAL BSD should not append the .so extension in LoadLibrary if the extension is -omitted. -file_io/deletefilea/test3 -file_io/deletefilew/test3 -======================================= -No code in Rotor depends on DeleteFile[A/W] failing on readonly files so we -don't need to exactly emulate this feature of the Win32 filesystems on FreeBSD. - filemapping_memmgt\MapViewOfFile\test1 ======================================= -Refer this github issue https://github.com/dotnet/coreclr/issues/5176 \ No newline at end of file +Refer this github issue https://github.com/dotnet/coreclr/issues/5176 From 84fb01a9f00b3f52a3ca29c6f44dc46afe3198e6 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:35:38 -0700 Subject: [PATCH 09/16] Remove unneeded casts. --- src/vm/eetoprofinterfaceimpl.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/vm/eetoprofinterfaceimpl.cpp b/src/vm/eetoprofinterfaceimpl.cpp index 1f22386d5ee9..63003caf7c49 100644 --- a/src/vm/eetoprofinterfaceimpl.cpp +++ b/src/vm/eetoprofinterfaceimpl.cpp @@ -2132,16 +2132,15 @@ HRESULT EEToProfInterfaceImpl::DetermineAndSetEnterLeaveFunctionHooksForJit() // intermediary FCALL which then calls the profiler's hook) with extra information // about the current function. - // The casts below are to appease rotor. cl.exe doesn't need them. hr = SetEnterLeaveFunctionHooksForJit( (m_pEnter3WithInfo != NULL) ? - reinterpret_cast(PROFILECALLBACK(ProfileEnter)) : + PROFILECALLBACK(ProfileEnter) : m_pEnter3, (m_pLeave3WithInfo != NULL) ? - reinterpret_cast(PROFILECALLBACK(ProfileLeave)) : + PROFILECALLBACK(ProfileLeave) : m_pLeave3, (m_pTailcall3WithInfo != NULL) ? - reinterpret_cast(PROFILECALLBACK(ProfileTailcall)) : + PROFILECALLBACK(ProfileTailcall) : m_pTailcall3); } else @@ -2175,16 +2174,15 @@ HRESULT EEToProfInterfaceImpl::DetermineAndSetEnterLeaveFunctionHooksForJit() BOOL fLeave = (m_pLeave != NULL) || (m_pLeave2 != NULL); BOOL fTailcall = (m_pTailcall != NULL) || (m_pTailcall2 != NULL); - // The casts below are to appease rotor. cl.exe doesn't need them. hr = SetEnterLeaveFunctionHooksForJit( fEnter ? - reinterpret_cast(PROFILECALLBACK(ProfileEnter)) : + PROFILECALLBACK(ProfileEnter) : NULL, fLeave ? - reinterpret_cast(PROFILECALLBACK(ProfileLeave)) : + PROFILECALLBACK(ProfileLeave) : NULL, fTailcall ? - reinterpret_cast(PROFILECALLBACK(ProfileTailcall)) : + PROFILECALLBACK(ProfileTailcall) : NULL); } } From 3cde18e8456a9ac30a95151020b0cd68c0808ed9 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:38:19 -0700 Subject: [PATCH 10/16] Remove dead and misleading code from profilinghelper.cpp. FEATURE_PROFAPI_EVENT_LOGGING is always defined when PROFILING_SUPPORTED is defined. And the entire contents of profilinghelper.cpp is surrounded with "ifdef PROFILING_SUPPORTED". So all sections in "ifndef FEATURE_PROFAPI_EVENT_LOGGING" are dead. Furthermore, in coreclr this does not use the eventlog, so the macro name is misleading. --- src/inc/switches.h | 10 ---------- src/vm/profilinghelper.cpp | 23 +---------------------- 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/src/inc/switches.h b/src/inc/switches.h index cd404d5c5347..3d8c876bcffc 100644 --- a/src/inc/switches.h +++ b/src/inc/switches.h @@ -145,16 +145,6 @@ #endif // _DEBUG - - -#if defined(PROFILING_SUPPORTED) -// On desktop CLR builds, the profiling API uses the event log for end-user-friendly -// diagnostic messages. CoreCLR on Windows ouputs debug strings for diagnostic messages. -// Rotor builds have no access to event log message resources, though, so they simply -// display popup dialogs for now. -#define FEATURE_PROFAPI_EVENT_LOGGING -#endif // defined(PROFILING_SUPPORTED) - // MUST NEVER CHECK IN WITH THIS ENABLED. // This is just for convenience in doing performance investigations in a checked-out enlistment. // #define FEATURE_ENABLE_NO_RANGE_CHECKS diff --git a/src/vm/profilinghelper.cpp b/src/vm/profilinghelper.cpp index a7c64e77ba89..8452eb8d6559 100644 --- a/src/vm/profilinghelper.cpp +++ b/src/vm/profilinghelper.cpp @@ -365,20 +365,6 @@ void ProfilingAPIUtility::LogProfEventVA( } CONTRACTL_END; -#ifndef FEATURE_PROFAPI_EVENT_LOGGING - - - // Rotor messages go to message boxes - - EEMessageBoxCatastrophic( - iStringResourceID, // Text message to display - IDS_EE_PROFILING_FAILURE, // Titlebar of message box - insertionArgs); // Insertion strings for text message - -#else // FEATURE_PROFAPI_EVENT_LOGGING - - // Non-rotor messages go to the event log - StackSString messageFromResource; StackSString messageToLog; @@ -395,10 +381,8 @@ void ProfilingAPIUtility::LogProfEventVA( AppendSupplementaryInformation(iStringResourceID, &messageToLog); - // CoreCLR on Windows ouputs debug strings for diagnostic messages. + // Ouput debug strings for diagnostic messages. WszOutputDebugString(messageToLog); - -#endif // FEATURE_PROFAPI_EVENT_LOGGING } // See code:ProfilingAPIUtility.LogProfEventVA for description of arguments. @@ -440,10 +424,6 @@ void ProfilingAPIUtility::LogProfInfo(int iStringResourceID, ...) } CONTRACTL_END; -// Rotor uses message boxes instead of event log, and it would be disruptive to -// pop a messagebox in the user's face every time an app runs with a profiler -// configured to load. So only log this only when we don't do a pop-up. -#ifdef FEATURE_PROFAPI_EVENT_LOGGING va_list insertionArgs; va_start(insertionArgs, iStringResourceID); LogProfEventVA( @@ -451,7 +431,6 @@ void ProfilingAPIUtility::LogProfInfo(int iStringResourceID, ...) EVENTLOG_INFORMATION_TYPE, insertionArgs); va_end(insertionArgs); -#endif //FEATURE_PROFAPI_EVENT_LOGGING } #ifdef PROF_TEST_ONLY_FORCE_ELT From e83c7f276a9cc43ed2236c07f9a2fc2ca73ac32c Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:40:26 -0700 Subject: [PATCH 11/16] Remove dead code in excep.cpp. This entire function is surrounded with "ifndef FEATURE_PAL". --- src/vm/excep.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/vm/excep.cpp b/src/vm/excep.cpp index 275b3ed1c91e..6eb91053a3a2 100644 --- a/src/vm/excep.cpp +++ b/src/vm/excep.cpp @@ -7436,7 +7436,6 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) } #endif // defined(WIN64EXCEPTIONS) && defined(FEATURE_HIJACK) -#ifndef FEATURE_PAL if (IsSOExceptionCode(pExceptionInfo->ExceptionRecord->ExceptionCode)) { // @@ -7475,9 +7474,6 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) //END_ENTRYPOINT_VOIDRET; // return retVal; -#else // !FEATURE_PAL - return CLRVectoredExceptionHandlerPhase2(pExceptionInfo); -#endif // !FEATURE_PAL } LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo) From 020500676c39222ebaad97b1335cc1fc1cec298c Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 15:57:42 -0700 Subject: [PATCH 12/16] Remove refererences to rotor from safemath.h This does not appear to cause any compile problems, so nobody was using safemath.h without _ASSERTE defined. Also S_SIZE_T_WP64BUG is not used anywhere. --- src/inc/safemath.h | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/inc/safemath.h b/src/inc/safemath.h index 073c998548fd..0bd3e87a9ad7 100644 --- a/src/inc/safemath.h +++ b/src/inc/safemath.h @@ -17,17 +17,7 @@ #include "debugmacrosext.h" #ifndef _ASSERTE_SAFEMATH -#ifdef _ASSERTE -// Use _ASSERTE if we have it (should always be the case in the CLR) #define _ASSERTE_SAFEMATH _ASSERTE -#else -// Otherwise (eg. we're being used from a tool like SOS) there isn't much -// we can rely on that is both available everywhere and rotor-safe. In -// several other tools we just take the recourse of disabling asserts, -// we'll do the same here. -// Ideally we'd have a collection of common utilities available evererywhere. -#define _ASSERTE_SAFEMATH(a) -#endif #endif #include "static_assert.h" @@ -855,18 +845,4 @@ typedef ClrSafeInt S_UINT16; typedef ClrSafeInt S_UINT64; typedef ClrSafeInt S_SIZE_T; -// Note: we can get bogus /Wp64 compiler warnings when S_SIZE_T is used. -// This is due to VSWhidbey 138322 which the C++ folks have said they can't -// currently fix. We can work around the problem by using this macro to force -// a no-op cast on 32-bit MSVC platforms. It's not yet clear why we need to -// use this in some places (specifically, rotor lkgvc builds) and not others. -// We also make the error less likely by using a #define instead of a -// typedef for S_UINT32 above since that means we're less likely to instantiate -// ClrSafeInt AND ClrSafeInt in the same compliation unit. -#if defined(_TARGET_X86_) && defined( _MSC_VER ) -#define S_SIZE_T_WP64BUG(v) S_SIZE_T( static_cast( v ) ) -#else -#define S_SIZE_T_WP64BUG(v) S_SIZE_T( v ) -#endif - #endif // SAFEMATH_H_ From 3b08fa7502b91698eca38b0d24823dc07c4a9b7e Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 16:08:35 -0700 Subject: [PATCH 13/16] Remove dead code from palclr.h. I don't know why these check to see if the macro is undefined immediately after defining them. Also the comment appears to reference some unions that are no longer in this file. --- src/inc/palclr.h | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/inc/palclr.h b/src/inc/palclr.h index 6ed2454753c6..dabe86f3d36a 100644 --- a/src/inc/palclr.h +++ b/src/inc/palclr.h @@ -141,31 +141,17 @@ #define IMAGE_IMPORT_DESC_FIELD(img, f) ((img).f) #endif -//Remove these "unanonymous" unions from newer builds for now. Confirm that they were never needed when we -//bring back Rotor. #define IMAGE_RDE_ID(img) ((img)->Id) -#ifndef IMAGE_RDE_ID -#define IMAGE_RDE_ID(img) ((img)->Id) -#endif #define IMAGE_RDE_NAME(img) ((img)->Name) -#ifndef IMAGE_RDE_NAME -#define IMAGE_RDE_NAME(img) ((img)->Name) -#endif #define IMAGE_RDE_OFFSET(img) ((img)->OffsetToData) -#ifndef IMAGE_RDE_OFFSET -#define IMAGE_RDE_OFFSET(img) ((img)->OffsetToData) -#endif #ifndef IMAGE_RDE_NAME_FIELD #define IMAGE_RDE_NAME_FIELD(img, f) ((img)->f) #endif #define IMAGE_RDE_OFFSET_FIELD(img, f) ((img)->f) -#ifndef IMAGE_RDE_OFFSET_FIELD -#define IMAGE_RDE_OFFSET_FIELD(img, f) ((img)->f) -#endif #ifndef IMAGE_FE64_FIELD #define IMAGE_FE64_FIELD(img, f) ((img).f) From 9a99ec9fe558d1dbb53df9de48b3c35faa3b2db0 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 16:18:29 -0700 Subject: [PATCH 14/16] Expose ISymUnmanagedWriter2 from SymWriter as required by COM. The comment talks about the C# compiler using this, however I cannot see a way for the C# compiler to get an instance of this. It is only used internally by AssemblyBuilder and not exposed otherwise. --- src/debug/ildbsymlib/symwrite.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/debug/ildbsymlib/symwrite.cpp b/src/debug/ildbsymlib/symwrite.cpp index f925850624bb..7f327612bb62 100644 --- a/src/debug/ildbsymlib/symwrite.cpp +++ b/src/debug/ildbsymlib/symwrite.cpp @@ -78,11 +78,8 @@ COM_METHOD SymWriter::QueryInterface(REFIID riid, void **ppInterface) if (riid == IID_ISymUnmanagedWriter ) *ppInterface = (ISymUnmanagedWriter*)this; - /* ROTORTODO: Pretend that we do not implement ISymUnmanagedWriter2 to prevent C# compiler from using it. - This is against COM rules since ISymUnmanagedWriter3 inherits from ISymUnmanagedWriter2. else if (riid == IID_ISymUnmanagedWriter2 ) *ppInterface = (ISymUnmanagedWriter2*)this; - */ else if (riid == IID_ISymUnmanagedWriter3 ) *ppInterface = (ISymUnmanagedWriter3*)this; else if (riid == IID_IUnknown) From 420dba4aa74608fc3a526a8318afcc7a06f6172f Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Sun, 7 Oct 2018 22:39:59 -0700 Subject: [PATCH 15/16] Restore check for _ASSERTE in safemath.h. On Windows sometimes that this file is included without _ASSERTE being defined. As the existing comment suggests, it appears that SOS explicitly does not want _ASSERTE to do anything. --- src/inc/safemath.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/inc/safemath.h b/src/inc/safemath.h index 0bd3e87a9ad7..6649f312726f 100644 --- a/src/inc/safemath.h +++ b/src/inc/safemath.h @@ -17,7 +17,17 @@ #include "debugmacrosext.h" #ifndef _ASSERTE_SAFEMATH +#ifdef _ASSERTE +// Use _ASSERTE if we have it (should always be the case in the CLR) #define _ASSERTE_SAFEMATH _ASSERTE +#else +// Otherwise (eg. we're being used from a tool like SOS) there isn't much +// we can rely on that is both available everywhere. In +// several other tools we just take the recourse of disabling asserts, +// we'll do the same here. +// Ideally we'd have a collection of common utilities available evererywhere. +#define _ASSERTE_SAFEMATH(a) +#endif #endif #include "static_assert.h" From c236e3ffdd5776a41c4e7bc6a3c332afb99590e6 Mon Sep 17 00:00:00 2001 From: Austin Wise Date: Mon, 8 Oct 2018 12:22:37 -0700 Subject: [PATCH 16/16] Respond to PR feedback. --- src/inc/safemath.h | 2 +- src/inc/securitywrapper.h | 10 ---- src/utilcode/securitywrapper.cpp | 90 -------------------------------- 3 files changed, 1 insertion(+), 101 deletions(-) diff --git a/src/inc/safemath.h b/src/inc/safemath.h index 6649f312726f..473e8467af13 100644 --- a/src/inc/safemath.h +++ b/src/inc/safemath.h @@ -22,7 +22,7 @@ #define _ASSERTE_SAFEMATH _ASSERTE #else // Otherwise (eg. we're being used from a tool like SOS) there isn't much -// we can rely on that is both available everywhere. In +// we can rely on that is available everywhere. In // several other tools we just take the recourse of disabling asserts, // we'll do the same here. // Ideally we'd have a collection of common utilities available evererywhere. diff --git a/src/inc/securitywrapper.h b/src/inc/securitywrapper.h index 0cb5f8d6cc47..1dad3a6cf503 100644 --- a/src/inc/securitywrapper.h +++ b/src/inc/securitywrapper.h @@ -62,8 +62,6 @@ class SidBuffer BYTE * m_pBuffer; }; -#ifndef FEATURE_PAL - //----------------------------------------------------------------------------- // Access Control List. //----------------------------------------------------------------------------- @@ -101,13 +99,5 @@ class Win32SecurityDescriptor PSECURITY_DESCRIPTOR m_pDesc; }; -#endif // FEATURE_PAL - -//----------------------------------------------------------------------------- -// Check if the handle owner belongs to either the process specified by the pid -// or the current process. This lets us know if the handle is spoofed. -//----------------------------------------------------------------------------- -bool IsHandleSpoofed(HANDLE handle, DWORD pid); - #endif // _SECURITY_WRAPPER_H diff --git a/src/utilcode/securitywrapper.cpp b/src/utilcode/securitywrapper.cpp index 10672b70045c..6dc5d767ca16 100644 --- a/src/utilcode/securitywrapper.cpp +++ b/src/utilcode/securitywrapper.cpp @@ -748,93 +748,3 @@ void Win32SecurityDescriptor::InitFromHandle(HANDLE h) ThrowHR(hr); } } - -//----------------------------------------------------------------------------- -// We open several named kernel objects that are well-known names decorated with -// pid of some target process (usually a debuggee). -// Since anybody can create any kernel object with any name, we we want to make -// sure the objects we're opening were actually created by who we think they -// were. Each kernel object has an "owner" property which serves as the -// fingerprints of who created the handle. -// -// Check if the handle owner belongs to either the process specified by the -// pid or the current process (in case the target process is impersonating us). -// This lets us know if the handle is spoofed. -// -// Parameters: -// handle - handle for kernel object to test -// pid - target process that it may belong to. -// -// Returns: -// false- if we can verify that Owner(handle) is in the set of { Owner(Process(pid)), or Owner(this Process) } -// -// -// true - Elsewise, including if we can't verify that it's false. -//----------------------------------------------------------------------------- -bool IsHandleSpoofed(HANDLE handle, DWORD pid) -{ - CONTRACTL - { - NOTHROW; - } - CONTRACTL_END; - - _ASSERTE(handle != NULL); - _ASSERTE(pid != 0); - bool fIsSpoofed = true; - - EX_TRY - { - // Get the owner of the kernel object referenced by the handle. - Win32SecurityDescriptor sdHandle; - sdHandle.InitFromHandle(handle); - Sid sidOwner(sdHandle.GetOwner()); - - SidBuffer sbPidOther; - SidBuffer sbPidThis; - DWORD pidThis; - - // Is the object owner the "other" pid? - sbPidOther.InitFromProcess(pid); - if (Sid::Equals(sbPidOther.GetSid(), sidOwner)) - { - // We now know that the kernel object was created by the user of the "other" pid. - // This should be the common case by far. It's not spoofed. All is well. - fIsSpoofed = false; - goto Label_Done; - } - - // Test against our current pid if it's different than the "other" pid. - // This can happen if the other process impersonates us. The most common case would - // be if we're an admin and the other process (say some service) is impersonating Admin - // when it spins up the CLR. - pidThis = GetCurrentProcessId(); - if (pidThis != pid) - { - sbPidThis.InitFromProcess(pidThis); - if (Sid::Equals(sbPidThis.GetSid(), sidOwner)) - { - // The object was created by somebody pretending to be us. If they had sufficient permissions - // to pretend to be us, then we still trust them. - fIsSpoofed = false; - goto Label_Done; - } - } - - // This should only happen if we're being attacked. - _ASSERTE(fIsSpoofed); - STRESS_LOG2(LF_CORDB, LL_INFO1000, "Security Check failed with mismatch. h=%x,pid=%x", handle, pid); - -Label_Done: - ; - } - EX_CATCH - { - // This should only happen if something goes really bad and we can't find the information. - STRESS_LOG2(LF_CORDB, LL_INFO1000, "Security Check failed with exception. h=%x,pid=%x", handle, pid); - _ASSERTE(fIsSpoofed); // should still have its default value - } - EX_END_CATCH(SwallowAllExceptions); - - return fIsSpoofed; -}