From 6f1bdfffb77ba1c95f46e16a7eeff3cfaf2f2f1f Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Fri, 15 Feb 2019 14:03:47 -0800 Subject: [PATCH] Disable arm64 contracts. (#22605) * Fix ifdef for `g_DbgSuppressAllocationAsserts`. It needs to be defined under `FEATURE_INTEROP_DEBUGGING`. * Unify contracts disabling for UNIX/ARM. It also disables some parts that were disabled for ARM, but not for UNIX. It fixes the difference that we see between ARM32 and ARM64 Unix Checked. * Disable contracts on arm64. FIxes the difference that we see between windows arm32 and arm64 in checked/debug builds. * Revert CoreFX arm64 timeout change. That is no longer necessary. * Reenable arm64 corefx jobs. They should not fail with timeouts anymore. * Don't allow `g_DbgSuppressAllocationAsserts` defnition in release builds. * delete AssertAllocationAllowed --- configurecompiler.cmake | 6 ++++++ netci.groovy | 15 --------------- src/debug/ee/debugger.h | 2 -- src/debug/ee/rcthread.cpp | 40 ---------------------------------------- src/inc/contract.h | 3 +-- src/inc/palclr.h | 2 +- 6 files changed, 8 insertions(+), 60 deletions(-) diff --git a/configurecompiler.cmake b/configurecompiler.cmake index 709994d..dd0ee96 100644 --- a/configurecompiler.cmake +++ b/configurecompiler.cmake @@ -438,6 +438,7 @@ if (CLR_CMAKE_PLATFORM_UNIX) add_compile_options(-fstack-protector-strong) endif(CLR_CMAKE_PLATFORM_DARWIN) + # Contracts are disabled on UNIX. add_definitions(-DDISABLE_CONTRACTS) if (CLR_CMAKE_WARNINGS_ARE_ERRORS) @@ -563,6 +564,11 @@ if (WIN32) set(CMAKE_ASM_MASM_FLAGS "${CMAKE_ASM_MASM_FLAGS} /ZH:SHA_256") + if (CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARGET_ARCH_ARM64) + # Contracts work too slow on ARM/ARM64 DEBUG/CHECKED. + add_definitions(-DDISABLE_CONTRACTS) + endif (CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARGET_ARCH_ARM64) + endif (WIN32) if(CLR_CMAKE_ENABLE_CODE_COVERAGE) diff --git a/netci.groovy b/netci.groovy index aa61148..16656fc 100755 --- a/netci.groovy +++ b/netci.groovy @@ -940,12 +940,6 @@ def static setJobTimeout(newJob, isPR, architecture, configuration, scenario, is } else if (isCoreFxScenario(scenario)) { timeout = 360 - if (architecture == 'arm64') { - if (configuration == 'Checked' || configuration == 'Debug') { - // ARM64 checked/debug is slow, see #17414. - timeout *= 3; - } - } } else if (isJitStressScenario(scenario)) { timeout = 300 @@ -1568,15 +1562,6 @@ def static addNonPRTriggers(def job, def branch, def isPR, def architecture, def if ((architecture == 'arm64') && isCoreFxScenario(scenario) && !isFlowJob) { break } - // Windows arm64 corefx testing all fails due to time out, partially due to no parallelism - // in the test run harness. So don't create cron jobs for these. We could alternatively - // just increase the timeout, but we don't have enough Windows arm64 machines to - // take so much time running these. We also have Linux/arm64 corefx test coverage. - // It would be best to improve the runtime of the tests. - // See issue https://github.com/dotnet/coreclr/issues/21236. - if ((architecture == 'arm64') && isCoreFxScenario(scenario) && (os == 'Windows_NT')) { - break - } if (jobRequiresLimitedHardware(architecture, os)) { if ((architecture == 'arm64') && (os == 'Ubuntu16.04')) { // These jobs are very fast on Linux/arm64 hardware, so run them daily. diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h index 1f824c7..15299c0 100644 --- a/src/debug/ee/debugger.h +++ b/src/debug/ee/debugger.h @@ -906,8 +906,6 @@ private: // calls operator new, which may occur during shutdown paths. static EEThreadId s_DbgHelperThreadId; - friend void AssertAllocationAllowed(); - public: // The OS ThreadId of the helper as determined from the CreateThread call. DWORD m_DbgHelperThreadOSTid; diff --git a/src/debug/ee/rcthread.cpp b/src/debug/ee/rcthread.cpp index bb653e5..c041831 100644 --- a/src/debug/ee/rcthread.cpp +++ b/src/debug/ee/rcthread.cpp @@ -797,46 +797,6 @@ static LONG _debugFilter(LPEXCEPTION_POINTERS ep, PVOID pv) return EXCEPTION_CONTINUE_SEARCH; } -#ifdef _DEBUG -// Tracking to ensure that we don't call New() for the normal (non interop-safe heap) -// on the helper thread. We also can't do a normal allocation when we have hard -// suspended any other thread (since it could hold the OS heap lock). - -// TODO: this probably belongs in the EE itself, not here in the debugger stuff. - -void AssertAllocationAllowed() -{ -#ifdef USE_INTEROPSAFE_HEAP - // Don't forget to preserve error status! - DWORD err = GetLastError(); - - // We can mark certain - if (g_DbgSuppressAllocationAsserts == 0) - { - - // if we have hard suspended any threads. We want to assert as it could cause deadlock - // since those suspended threads may hold the OS heap lock - if (g_fEEStarted) { - _ASSERTE (!EEAllocationDisallowed()); - } - - // Can't call IsDbgHelperSpecialThread() here b/c that changes program state. - // So we use our - if (DebuggerRCThread::s_DbgHelperThreadId.IsCurrentThread()) - { - // In case assert allocates, bump up the 'OK' counter to avoid an infinite recursion. - SUPPRESS_ALLOCATION_ASSERTS_IN_THIS_SCOPE; - - _ASSERTE(false || !"New called on Helper Thread"); - - } - } - SetLastError(err); -#endif -} -#endif - - //--------------------------------------------------------------------------------------- // // Primary function of the Runtime Controller thread. First, we let diff --git a/src/inc/contract.h b/src/inc/contract.h index a497424..3913e56 100644 --- a/src/inc/contract.h +++ b/src/inc/contract.h @@ -239,8 +239,7 @@ #endif // Also, we won't enable contracts if this is a DAC build. -// @ARMTODO: Disable for ARM for now, contracts slow down the debug build far too much -#if defined(ENABLE_CONTRACTS_DATA) && !defined(DACCESS_COMPILE) && !defined(CROSS_COMPILE) && !defined(_TARGET_ARM_) +#if defined(ENABLE_CONTRACTS_DATA) && !defined(DACCESS_COMPILE) && !defined(CROSS_COMPILE) #define ENABLE_CONTRACTS #endif diff --git a/src/inc/palclr.h b/src/inc/palclr.h index fbb8975..77b547d 100644 --- a/src/inc/palclr.h +++ b/src/inc/palclr.h @@ -503,7 +503,7 @@ #endif -#if defined(_DEBUG_IMPL) && !defined(JIT_BUILD) && !defined(JIT64_BUILD) && !defined(CROSS_COMPILE) && !defined(_TARGET_ARM_) // @ARMTODO: no contracts for speed +#if defined(_DEBUG_IMPL) && !defined(JIT_BUILD) && !defined(JIT64_BUILD) && !defined(CROSS_COMPILE) && !defined(DISABLE_CONTRACTS) #define PAL_TRY_HANDLER_DBG_BEGIN \ BOOL ___oldOkayToThrowValue = FALSE; \ ClrDebugState *___pState = ::GetClrDebugState(); \ -- 2.7.4