From 4246ba19bd196c5f374d94e5c1fc7b21d53bd9fc Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 16 May 2024 21:14:19 -0700 Subject: [PATCH] Use alternative mechanisms to allow us to use the default new/delete implementations (#101947) --- src/coreclr/clrdefinitions.cmake | 1 - src/coreclr/debug/inc/dbgipcevents.h | 4 +- src/coreclr/ilasm/main.cpp | 5 - src/coreclr/ildasm/windasm.cpp | 8 +- src/coreclr/inc/clrhost.h | 6 +- src/coreclr/inc/corhlpr.cpp | 2 - src/coreclr/inc/corhlpr.h | 30 +--- src/coreclr/inc/corhlprpriv.cpp | 6 - src/coreclr/inc/corhlprpriv.h | 2 +- src/coreclr/inc/ex.h | 41 ++++-- src/coreclr/inc/new.hpp | 18 --- src/coreclr/inc/palclr.h | 2 + src/coreclr/inc/utilcode.h | 4 +- src/coreclr/md/external.h | 4 +- src/coreclr/pal/inc/pal.h | 2 + .../utilcode/clrhost_nodependencies.cpp | 134 ------------------ src/coreclr/utilcode/stresslog.cpp | 6 + src/coreclr/vm/.vscode/c_cpp_properties.json | 1 - src/coreclr/vm/exceptmacros.h | 8 ++ src/tests/profiler/native/CMakeLists.txt | 3 +- .../native/rejitprofiler/ilrewriter.cpp | 2 + 21 files changed, 75 insertions(+), 214 deletions(-) delete mode 100644 src/coreclr/inc/new.hpp diff --git a/src/coreclr/clrdefinitions.cmake b/src/coreclr/clrdefinitions.cmake index 5dc10ae36570f..73de2d5bce257 100644 --- a/src/coreclr/clrdefinitions.cmake +++ b/src/coreclr/clrdefinitions.cmake @@ -42,7 +42,6 @@ if(CLR_CMAKE_TARGET_LINUX_MUSL) add_definitions(-DNO_FIXED_STACK_LIMIT) endif(CLR_CMAKE_TARGET_LINUX_MUSL) -add_definitions(-D_BLD_CLR) add_definitions(-DDEBUGGING_SUPPORTED) add_compile_definitions($<$>>:PROFILING_SUPPORTED>) add_compile_definitions($<$>:PROFILING_SUPPORTED_DATA>) diff --git a/src/coreclr/debug/inc/dbgipcevents.h b/src/coreclr/debug/inc/dbgipcevents.h index 23e67bd5673f8..6c39939f00307 100644 --- a/src/coreclr/debug/inc/dbgipcevents.h +++ b/src/coreclr/debug/inc/dbgipcevents.h @@ -10,7 +10,7 @@ #ifndef _DbgIPCEvents_h_ #define _DbgIPCEvents_h_ -#include +#include #include #include #include // for ICorDebugInfo::VarLocType & VarLoc @@ -25,6 +25,8 @@ #include "./common.h" +using std::nothrow; + //----------------------------------------------------------------------------- // V3 additions to IPC protocol between LS and RS. //----------------------------------------------------------------------------- diff --git a/src/coreclr/ilasm/main.cpp b/src/coreclr/ilasm/main.cpp index 0fe683838d899..ebb63e3434fec 100644 --- a/src/coreclr/ilasm/main.cpp +++ b/src/coreclr/ilasm/main.cpp @@ -151,11 +151,6 @@ extern "C" int _cdecl wmain(int argc, _In_ WCHAR **argv) memset(wzOutputFilename,0,sizeof(wzOutputFilename)); memset(wzPdbFilename, 0, sizeof(wzPdbFilename)); -#ifdef _DEBUG - DisableThrowCheck(); - //CONTRACT_VIOLATION(ThrowsViolation); -#endif - if(argc < 2) goto ErrorExit; #ifdef _PREFAST_ #pragma warning(push) diff --git a/src/coreclr/ildasm/windasm.cpp b/src/coreclr/ildasm/windasm.cpp index 8a4c5d06a7aa2..970b0e400d01f 100644 --- a/src/coreclr/ildasm/windasm.cpp +++ b/src/coreclr/ildasm/windasm.cpp @@ -17,7 +17,9 @@ #include #include "resource.h" -#include "new.hpp" +#include + +using std::nothrow; #define MODE_DUMP_ALL 0 #define MODE_DUMP_CLASS 1 @@ -465,10 +467,6 @@ int main(int argc, char* argv[]) } #endif -#ifdef _DEBUG - DisableThrowCheck(); -#endif - int iCommandLineParsed = 0; WCHAR* wzCommandLine = NULL; char* szCommandLine = NULL; diff --git a/src/coreclr/inc/clrhost.h b/src/coreclr/inc/clrhost.h index a9376a0a422ab..73d17c74e082b 100644 --- a/src/coreclr/inc/clrhost.h +++ b/src/coreclr/inc/clrhost.h @@ -8,6 +8,8 @@ #ifndef __CLRHOST_H__ #define __CLRHOST_H__ +#include + #include "windows.h" // worth to include before mscoree.h so we are guaranteed to pick few definitions #ifdef CreateSemaphore #undef CreateSemaphore @@ -16,7 +18,9 @@ #include "clrinternal.h" #include "switches.h" #include "holder.h" -#include "new.hpp" + +using std::nothrow; + #include "staticcontract.h" #include "predeftlsslot.h" #include "safemath.h" diff --git a/src/coreclr/inc/corhlpr.cpp b/src/coreclr/inc/corhlpr.cpp index f4dc78d4f37c4..61dc050ce52e2 100644 --- a/src/coreclr/inc/corhlpr.cpp +++ b/src/coreclr/inc/corhlpr.cpp @@ -8,9 +8,7 @@ ****************************************************************************/ #ifndef SOS_INCLUDE -#ifdef _BLD_CLR #include "utilcode.h" -#endif #include "corhlpr.h" #include diff --git a/src/coreclr/inc/corhlpr.h b/src/coreclr/inc/corhlpr.h index 3cc0a36701e7e..0bd3b1e1dfe85 100644 --- a/src/coreclr/inc/corhlpr.h +++ b/src/coreclr/inc/corhlpr.h @@ -22,14 +22,9 @@ #include "corhdr.h" #include "corerror.h" #include "unreachable.h" +#include -// This header is consumed both within the runtime and externally. In the former -// case we need to wrap memory allocations, in the latter there is no -// infrastructure to support this. Detect which way we're building and provide a -// very simple abstraction layer (handles allocating bytes only). -#ifdef _BLD_CLR -#include "new.hpp" - +using std::nothrow; #define NEW_NOTHROW(_bytes) new (nothrow) BYTE[_bytes] #define NEW_THROWS(_bytes) new BYTE[_bytes] @@ -38,27 +33,6 @@ inline void DECLSPEC_NORETURN THROW_OUT_OF_MEMORY() { ThrowOutOfMemory(); } -#else -#define NEW_NOTHROW(_bytes) new BYTE[_bytes] -#define NEW_THROWS(_bytes) __CorHlprNewThrows(_bytes) -static inline void DECLSPEC_NORETURN __CorHlprThrowOOM() -{ - RaiseException(STATUS_NO_MEMORY, 0, 0, NULL); - __UNREACHABLE(); -} -static inline BYTE *__CorHlprNewThrows(size_t bytes) -{ - BYTE *pbMemory = new BYTE[bytes]; - if (pbMemory == NULL) - __CorHlprThrowOOM(); - return pbMemory; -} -inline void DECLSPEC_NORETURN THROW_OUT_OF_MEMORY() -{ - __CorHlprThrowOOM(); -} -#endif - //***************************************************************************** // There are a set of macros commonly used in the helpers which you will want diff --git a/src/coreclr/inc/corhlprpriv.cpp b/src/coreclr/inc/corhlprpriv.cpp index c5d26cc6ce86e..97c25d3cfb58d 100644 --- a/src/coreclr/inc/corhlprpriv.cpp +++ b/src/coreclr/inc/corhlprpriv.cpp @@ -8,9 +8,7 @@ ****************************************************************************/ #ifndef SOS_INCLUDE -#ifdef _BLD_CLR #include "utilcode.h" -#endif #include "corhlprpriv.h" #include @@ -23,7 +21,6 @@ template HRESULT CQuickMemoryBase::ReSizeNoThrow(SIZE_T iItems) { -#ifdef _BLD_CLR #ifdef _DEBUG #ifndef DACCESS_COMPILE // Exercise heap for OOM-fault injection purposes @@ -38,7 +35,6 @@ HRESULT CQuickMemoryBase::ReSizeNoThrow(SIZE_T iItems) delete [] pTmp; } #endif -#endif #endif BYTE *pbBuffNew; if (iItems <= cbTotal) @@ -47,12 +43,10 @@ HRESULT CQuickMemoryBase::ReSizeNoThrow(SIZE_T iItems) return NOERROR; } -#ifdef _BLD_CLR #ifndef DACCESS_COMPILE // not allowed to do allocation if current thread suspends EE if (IsSuspendEEThread ()) return E_OUTOFMEMORY; -#endif #endif pbBuffNew = NEW_NOTHROW(iItems + INCREMENT); if (!pbBuffNew) diff --git a/src/coreclr/inc/corhlprpriv.h b/src/coreclr/inc/corhlprpriv.h index f000e7334c58a..9cbdaa4a3ea85 100644 --- a/src/coreclr/inc/corhlprpriv.h +++ b/src/coreclr/inc/corhlprpriv.h @@ -81,7 +81,7 @@ class CQuickMemoryBase template void *_Alloc(SIZE_T iItems) { -#if defined(_BLD_CLR) && defined(_DEBUG) +#if defined(_DEBUG) { // Exercise heap for OOM-fault injection purposes BYTE * pb = NSQuickBytesHelper::_AllocBytes::Invoke(iItems); _ASSERTE(!bThrow || pb != NULL); // _AllocBytes would have thrown if bThrow == TRUE diff --git a/src/coreclr/inc/ex.h b/src/coreclr/inc/ex.h index 5496c999e0391..50949d62f0138 100644 --- a/src/coreclr/inc/ex.h +++ b/src/coreclr/inc/ex.h @@ -827,6 +827,15 @@ Exception *ExThrowWithInnerHelper(Exception *inner); } \ SCAN_EHMARKER_END_TRY(); \ } \ + PAL_CPP_CATCH_NON_DERIVED_NOARG (const std::bad_alloc&) \ + { \ + SCAN_EHMARKER_CATCH(); \ + __state.SetCaughtCxx(); \ + __state.m_pExceptionPtr = Exception::GetOOMException(); \ + SCAN_EHMARKER_END_CATCH(); \ + SCAN_IGNORE_THROW_MARKER; \ + ThrowOutOfMemory(); \ + } \ PAL_CPP_CATCH_DERIVED (DerivedExceptionClass, __pExceptionRaw) \ { \ SCAN_EHMARKER_CATCH(); \ @@ -862,18 +871,34 @@ Exception *ExThrowWithInnerHelper(Exception *inner); PAL_CPP_TRY \ { \ SCAN_EHMARKER_TRY(); \ - CAutoTryCleanup __autoCleanupTry(__state); \ - /* prevent annotations from being dropped by optimizations in debug */ \ - INDEBUG(static bool __alwayszero;) \ - INDEBUG(VolatileLoad(&__alwayszero);) \ + SCAN_EHMARKER(); \ + PAL_CPP_TRY \ { \ - /* Disallow returns to make exception handling work. */ \ - /* Some work is done after the catch, see EX_ENDTRY. */ \ - DEBUG_ASSURE_NO_RETURN_BEGIN(EX_TRY) \ + SCAN_EHMARKER_TRY(); \ + CAutoTryCleanup __autoCleanupTry(__state); \ + /* prevent annotations from being dropped by optimizations in debug */ \ + INDEBUG(static bool __alwayszero;) \ + INDEBUG(VolatileLoad(&__alwayszero);) \ + { \ + /* Disallow returns to make exception handling work. */ \ + /* Some work is done after the catch, see EX_ENDTRY. */ \ + DEBUG_ASSURE_NO_RETURN_BEGIN(EX_TRY) \ #define EX_CATCH_IMPL_CPP_ONLY \ - DEBUG_ASSURE_NO_RETURN_END(EX_TRY) \ + DEBUG_ASSURE_NO_RETURN_END(EX_TRY) \ + } \ + SCAN_EHMARKER_END_TRY(); \ + } \ + PAL_CPP_CATCH_NON_DERIVED_NOARG (const std::bad_alloc&) \ + { \ + SCAN_EHMARKER_CATCH(); \ + __state.SetCaughtCxx(); \ + __state.m_pExceptionPtr = Exception::GetOOMException(); \ + SCAN_EHMARKER_END_CATCH(); \ + SCAN_IGNORE_THROW_MARKER; \ + ThrowOutOfMemory(); \ } \ + PAL_CPP_ENDTRY \ SCAN_EHMARKER_END_TRY(); \ } \ PAL_CPP_CATCH_DERIVED (Exception, __pExceptionRaw) \ diff --git a/src/coreclr/inc/new.hpp b/src/coreclr/inc/new.hpp deleted file mode 100644 index 037ceb3c667c2..0000000000000 --- a/src/coreclr/inc/new.hpp +++ /dev/null @@ -1,18 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// - -// - -#ifndef __new__hpp -#define __new__hpp - -#include - -using std::nothrow; - -#ifdef _DEBUG -void DisableThrowCheck(); -#endif - -#endif diff --git a/src/coreclr/inc/palclr.h b/src/coreclr/inc/palclr.h index 40fe2d1d3a2d1..98321ba87f838 100644 --- a/src/coreclr/inc/palclr.h +++ b/src/coreclr/inc/palclr.h @@ -481,6 +481,8 @@ #define PAL_CPP_THROW(type, obj) do { SCAN_THROW_MARKER; throw obj; } while (false) #define PAL_CPP_RETHROW do { SCAN_THROW_MARKER; throw; } while (false) #define PAL_CPP_CATCH_DERIVED(type, obj) catch (type * obj) +#define PAL_CPP_CATCH_NON_DERIVED(type, obj) catch (type obj) +#define PAL_CPP_CATCH_NON_DERIVED_NOARG(type) catch (type) #define PAL_CPP_CATCH_ALL catch (...) #define PAL_CPP_CATCH_EXCEPTION_NOARG catch (Exception *) diff --git a/src/coreclr/inc/utilcode.h b/src/coreclr/inc/utilcode.h index ca71af48a977d..53e4b2bb0fe97 100644 --- a/src/coreclr/inc/utilcode.h +++ b/src/coreclr/inc/utilcode.h @@ -14,6 +14,9 @@ #include #include #include +#include + +using std::nothrow; #include "crtwrap.h" #include "winwrap.h" @@ -29,7 +32,6 @@ #include "corhlprpriv.h" #include "check.h" #include "safemath.h" -#include "new.hpp" #include "contract.h" diff --git a/src/coreclr/md/external.h b/src/coreclr/md/external.h index 990ef4a2829e9..0fdc88e930d90 100644 --- a/src/coreclr/md/external.h +++ b/src/coreclr/md/external.h @@ -13,4 +13,6 @@ #include #include -#include +#include + +using std::nothrow; diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index 4a27787794939..08bca424cb24c 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -4418,6 +4418,8 @@ class NativeExceptionHolderFactory #define PAL_CPP_CATCH_EXCEPTION(ident) } catch (Exception *ident) { #define PAL_CPP_CATCH_EXCEPTION_NOARG } catch (Exception *) { #define PAL_CPP_CATCH_DERIVED(type, ident) } catch (type *ident) { +#define PAL_CPP_CATCH_NON_DERIVED(type, ident) } catch (type ident) { +#define PAL_CPP_CATCH_NON_DERIVED_NOARG(type) } catch (type) { #define PAL_CPP_CATCH_ALL } catch (...) { \ try { throw; } \ catch (PAL_SEHException& ex) { ex.SecondPassDone(); } \ diff --git a/src/coreclr/utilcode/clrhost_nodependencies.cpp b/src/coreclr/utilcode/clrhost_nodependencies.cpp index a3d1cfb3e12f6..766081eab8136 100644 --- a/src/coreclr/utilcode/clrhost_nodependencies.cpp +++ b/src/coreclr/utilcode/clrhost_nodependencies.cpp @@ -12,44 +12,6 @@ #include "clrnt.h" #include "contract.h" -#ifdef _DEBUG_IMPL - -// -// I'd very much like for this to go away. Its used to disable all THROWS contracts within whatever DLL this -// function is called from. That's obviously very, very bad, since there's no validation of those macros. But it -// can be difficult to remove this without actually fixing every violation at the same time. -// -// When this flag is finally removed, remove RealCLRThrowsExceptionWorker() too and put CONTRACT_THROWS() in place -// of it. -// -// -static BOOL dbg_fDisableThrowCheck = FALSE; - -void DisableThrowCheck() -{ - LIMITED_METHOD_CONTRACT; - - dbg_fDisableThrowCheck = TRUE; -} - -#define CLRThrowsExceptionWorker() RealCLRThrowsExceptionWorker(__FUNCTION__, __FILE__, __LINE__) - -static void RealCLRThrowsExceptionWorker(_In_z_ const char *szFunction, - _In_z_ const char *szFile, - int lineNum) -{ - WRAPPER_NO_CONTRACT; - - if (dbg_fDisableThrowCheck) - { - return; - } - - CONTRACT_THROWSEX(szFunction, szFile, lineNum); -} - -#endif //_DEBUG_IMPL - #if defined(_DEBUG_IMPL) && defined(ENABLE_CONTRACTS_IMPL) thread_local ClrDebugState* t_pClrDebugState; @@ -202,102 +164,6 @@ ClrDebugState *CLRInitDebugState() #endif //defined(_DEBUG_IMPL) && defined(ENABLE_CONTRACTS_IMPL) -// use standard heap functions for AddressSanitizer and for the DAC build. -#if !defined(HAS_ADDRESS_SANITIZER) && !defined(DACCESS_COMPILE) - -#if defined(SELF_NO_HOST) -namespace -{ - void ReportOOM(size_t size) - { - // With no host, we have nowhere to report the OOM. - } -} -#else -// If we have the CLR host, we should report OOMs to the StressLog. -namespace -{ - FORCEINLINE void ReportOOM(size_t size) - { - STATIC_CONTRACT_NOTHROW; - // If we have not created StressLog ring buffer, we should not try to use it. - // StressLog is going to do a memory allocation. We may enter an endless loop. - if (StressLog::t_pCurrentThreadLog != NULL) - { - STRESS_LOG_OOM_STACK(size); - } - } -} -#endif - -void * __cdecl -operator new(size_t n) -{ -#ifdef _DEBUG_IMPL - CLRThrowsExceptionWorker(); -#endif - - STATIC_CONTRACT_THROWS; - STATIC_CONTRACT_GC_NOTRIGGER; - STATIC_CONTRACT_FAULT; - STATIC_CONTRACT_SUPPORTS_DAC_HOST_ONLY; - - void* result = malloc(n == 0 ? 1 : n); - if (result == NULL) { - ReportOOM(n); - ThrowOutOfMemory(); - } - TRASH_LASTERROR; - return result; -} - -void * __cdecl -operator new[](size_t n) -{ - return ::operator new(n); -} - -void* __cdecl operator new(size_t size, const std::nothrow_t&) noexcept -{ - STATIC_CONTRACT_NOTHROW; - STATIC_CONTRACT_GC_NOTRIGGER; - STATIC_CONTRACT_FAULT; - STATIC_CONTRACT_SUPPORTS_DAC_HOST_ONLY; - - INCONTRACT(_ASSERTE(!ARE_FAULTS_FORBIDDEN())); - - void* result = malloc(size == 0 ? 1 : size); - if (result == nullptr) - { - ReportOOM(size); - } - TRASH_LASTERROR; - return result; -} - -void* __cdecl operator new[](size_t size, const std::nothrow_t&) noexcept -{ - return ::operator new(size, std::nothrow); -} - -void __cdecl operator delete(void* p) noexcept -{ - STATIC_CONTRACT_NOTHROW; - STATIC_CONTRACT_GC_NOTRIGGER; - STATIC_CONTRACT_SUPPORTS_DAC_HOST_ONLY; - - free(p); - - TRASH_LASTERROR; -} - -void __cdecl operator delete[](void* p) noexcept -{ - ::operator delete(p); -} - -#endif // !HAS_ADDRESS_SANITIZER && !DACCESS_COMPILE - #ifdef _DEBUG // This is a DEBUG routing to verify that a memory region complies with executable requirements diff --git a/src/coreclr/utilcode/stresslog.cpp b/src/coreclr/utilcode/stresslog.cpp index bc300ba131268..2f647f3c41208 100644 --- a/src/coreclr/utilcode/stresslog.cpp +++ b/src/coreclr/utilcode/stresslog.cpp @@ -483,6 +483,12 @@ ThreadStressLog* StressLog::CreateThreadStressLog() { // we fail. t_pCurrentThreadLog = NULL; } +#pragma warning(suppress: 4101) + PAL_CPP_CATCH_NON_DERIVED(const std::bad_alloc&, obj) + { + // Just leave on any exception. Note: can't goto or return from within EX_CATCH... + noFLSNow = TRUE; + } #pragma warning(suppress: 4101) PAL_CPP_CATCH_DERIVED(OutOfMemoryException, obj) { diff --git a/src/coreclr/vm/.vscode/c_cpp_properties.json b/src/coreclr/vm/.vscode/c_cpp_properties.json index 4192e236e3548..ae1a019f9c05f 100644 --- a/src/coreclr/vm/.vscode/c_cpp_properties.json +++ b/src/coreclr/vm/.vscode/c_cpp_properties.json @@ -22,7 +22,6 @@ ], "defines": [ "HOST_AMD64", - "_BLD_CLR", "_CRT_SECURE_NO_WARNINGS", "_DBG", "_SECURE_SCL=0", diff --git a/src/coreclr/vm/exceptmacros.h b/src/coreclr/vm/exceptmacros.h index d19966c609d32..9b7137ca90868 100644 --- a/src/coreclr/vm/exceptmacros.h +++ b/src/coreclr/vm/exceptmacros.h @@ -353,6 +353,14 @@ VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHar DEBUG_ASSURE_NO_RETURN_END(IUACH) \ SCAN_EHMARKER_END_TRY(); \ } \ + PAL_CPP_CATCH_NON_DERIVED_NOARG (const std::bad_alloc&) \ + { \ + SCAN_EHMARKER_CATCH(); \ + __pUnCException = Exception::GetOOMException(); \ + UnwindAndContinueRethrowHelperInsideCatch(__pUnCEntryFrame, __pUnCException); \ + __fExceptionCaught = true; \ + SCAN_EHMARKER_END_CATCH(); \ + } \ PAL_CPP_CATCH_DERIVED (Exception, __pException) \ { \ SCAN_EHMARKER_CATCH(); \ diff --git a/src/tests/profiler/native/CMakeLists.txt b/src/tests/profiler/native/CMakeLists.txt index 8d915a9d75033..0e5765fc0c114 100644 --- a/src/tests/profiler/native/CMakeLists.txt +++ b/src/tests/profiler/native/CMakeLists.txt @@ -32,9 +32,10 @@ set(SOURCES include_directories(../../../coreclr/pal/prebuilt/inc) +add_compile_definitions(SOS_INCLUDE) + if(NOT WIN32) include_directories(../../../coreclr/pal/inc/rt ../../../coreclr/pal/inc ../../../coreclr/inc) - add_compile_options(-DPAL_STDCPP_COMPAT) if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-Wno-null-arithmetic) else(CMAKE_CXX_COMPILER_ID MATCHES "Clang") diff --git a/src/tests/profiler/native/rejitprofiler/ilrewriter.cpp b/src/tests/profiler/native/rejitprofiler/ilrewriter.cpp index 371bac8f125a1..c145a49aa03ab 100644 --- a/src/tests/profiler/native/rejitprofiler/ilrewriter.cpp +++ b/src/tests/profiler/native/rejitprofiler/ilrewriter.cpp @@ -4,6 +4,8 @@ #define _ASSERTE(e) ((void)0) #include +#include +#include #include #include "ilrewriter.h" #include "sigparse.h"