From 64018cfae450593e5e0f4077ec49f2d5bd9a47ef Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Tue, 13 Aug 2024 13:18:42 +0200 Subject: [PATCH] Catch floating point exceptions on MSVC for gtest + add cmake `FORCE_DEBUG_ARITHM_MSVC` to enable in release builds too --- cmake/CompilerFlags.cmake | 16 ++++++++++------ src/EnergyPlus/api/EnergyPlusPgm.cc | 27 +++++++++++++++++---------- src/EnergyPlus/main.cc | 8 -------- tst/EnergyPlus/unit/main.cc | 16 ++++++++++++++++ 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/cmake/CompilerFlags.cmake b/cmake/CompilerFlags.cmake index bfc9fb3b8b7..9acc1c3691a 100644 --- a/cmake/CompilerFlags.cmake +++ b/cmake/CompilerFlags.cmake @@ -76,15 +76,19 @@ if(MSVC AND NOT ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Intel")) # Visual C++ (VS target_compile_options(project_options INTERFACE $<$:/Ob0>) # Disable inlining target_compile_options(project_options INTERFACE $<$:/RTCsu>) # Runtime checks target_compile_options(project_fp_options INTERFACE $<$:/fp:strict>) # Floating point model - target_compile_options(project_options INTERFACE $<$:/DMSVC_DEBUG>) # Triggers code in main.cc to catch floating point NaNs - target_compile_options(turn_off_warnings INTERFACE /W0) + option(FORCE_DEBUG_ARITHM_MSVC "Enable trapping floating point exceptions in non Debug mode" OFF) + mark_as_advanced(FORCE_DEBUG_ARITHM_MSVC) + + set(need_arithm_debug_genex "$,$>") + + # in main.cc for E+ (actual: api/EnergyPlusPgm.cc) and gtest: will catch _EM_ZERODIVIDE | _EM_INVALID | _EM_OVERFLOW + target_compile_definitions(project_fp_options INTERFACE $<${need_arithm_debug_genex}:DEBUG_ARITHM_MSVC>) + elseif(CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang") # g++/Clang - # TODO: after we fix all test, enable this by default on Debug builds - # option(FORCE_DEBUG_ARITHM_GCC_OR_CLANG "Enable trapping floating point exceptions in non Debug mode" OFF) - option(FORCE_DEBUG_ARITHM_GCC_OR_CLANG "Enable trapping floating point exceptions" OFF) + option(FORCE_DEBUG_ARITHM_GCC_OR_CLANG "Enable trapping floating point exceptions in non Debug mode" OFF) mark_as_advanced(FORCE_DEBUG_ARITHM_GCC_OR_CLANG) # COMPILER FLAGS @@ -126,7 +130,7 @@ elseif(CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" O set(need_arithm_debug_genex "$,$>") - # in main.cc for E+ and gtest: feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW) + # in main.cc for E+ (actual: api/EnergyPlusPgm.cc) and gtest: feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW) target_compile_definitions(project_fp_options INTERFACE $<${need_arithm_debug_genex}:DEBUG_ARITHM_GCC_OR_CLANG>) include(CheckCXXSymbolExists) check_cxx_symbol_exists(feenableexcept "fenv.h" HAVE_FEENABLEEXCEPT) diff --git a/src/EnergyPlus/api/EnergyPlusPgm.cc b/src/EnergyPlus/api/EnergyPlusPgm.cc index a86e8c1a94e..6c7a375f371 100644 --- a/src/EnergyPlus/api/EnergyPlusPgm.cc +++ b/src/EnergyPlus/api/EnergyPlusPgm.cc @@ -182,10 +182,12 @@ #include #include -#ifndef NDEBUG -#ifdef __unix__ -#include +#ifdef DEBUG_ARITHM_GCC_OR_CLANG +#include #endif + +#ifdef DEBUG_ARITHM_MSVC +#include #endif // ObjexxFCL Headers @@ -248,17 +250,22 @@ void commonInitialize(EnergyPlus::EnergyPlusData &state) // std::cin.tie(nullptr); // Untie cin and cout: Could cause odd behavior for interactive prompts // Enable floating point exceptions -#ifndef NDEBUG -#ifdef __unix__ +#ifdef DEBUG_ARITHM_GCC_OR_CLANG feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW); // These exceptions are enabled (FE_INEXACT and FE_UNDERFLOW will not throw) #endif -#endif -#ifdef MSVC_DEBUG - // the following line enables NaN detection in Visual Studio debug builds. See +#ifdef DEBUG_ARITHM_MSVC + // the following enables NaN detection in Visual Studio debug builds. See // https://github.com/NREL/EnergyPlus/wiki/Debugging-Tips - int fp_control_state = - _controlfp(_EM_INEXACT | _EM_UNDERFLOW, _MCW_EM); // These exceptions are disabled (_EM_INEXACT and _EM_UNDERFLOW will not throw) + + // Note: what you need to pass to the _controlfp_s is actually the opposite + // By default all bits are 1, and the exceptions are turned off, so you need to turn off the bits for the exceptions you want to enable + // > For the _MCW_EM mask, clearing it sets the exception, which allows the hardware exception; setting it hides the exception. + unsigned int fpcntrl = 0; + _controlfp_s(&fpcntrl, 0, 0); + unsigned int new_exceptions = _EM_ZERODIVIDE | _EM_INVALID | _EM_OVERFLOW; + unsigned int new_control = fpcntrl & ~new_exceptions; + _controlfp_s(&fpcntrl, new_control, _MCW_EM); #endif #ifdef _MSC_VER diff --git a/src/EnergyPlus/main.cc b/src/EnergyPlus/main.cc index b7ee17b6659..cffbc573080 100644 --- a/src/EnergyPlus/main.cc +++ b/src/EnergyPlus/main.cc @@ -50,16 +50,8 @@ #include -#ifdef DEBUG_ARITHM_GCC_OR_CLANG -#include -#endif - int main(int argc, char **argv) { -#ifdef DEBUG_ARITHM_GCC_OR_CLANG - feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW); -#endif - #ifdef _WIN32 const std::vector args = CLI::detail::compute_win32_argv(); #else diff --git a/tst/EnergyPlus/unit/main.cc b/tst/EnergyPlus/unit/main.cc index 45d9421cf3b..b46336bf040 100644 --- a/tst/EnergyPlus/unit/main.cc +++ b/tst/EnergyPlus/unit/main.cc @@ -54,6 +54,10 @@ #include #endif +#ifdef DEBUG_ARITHM_MSVC +#include +#endif + // Google Test main int main(int argc, char **argv) { @@ -68,5 +72,17 @@ int main(int argc, char **argv) #ifdef DEBUG_ARITHM_GCC_OR_CLANG feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW); #endif + +#ifdef DEBUG_ARITHM_MSVC + // Note: what you need to pass to the _controlfp_s is actually the opposite + // By default all bits are 1, and the exceptions are turned off, so you need to turn off the bits for the exceptions you want to enable + // > For the _MCW_EM mask, clearing it sets the exception, which allows the hardware exception; setting it hides the exception. + unsigned int fpcntrl = 0; + _controlfp_s(&fpcntrl, 0, 0); + unsigned int new_exceptions = _EM_ZERODIVIDE | _EM_INVALID | _EM_OVERFLOW; + unsigned int new_control = fpcntrl & ~new_exceptions; + _controlfp_s(&fpcntrl, new_control, _MCW_EM); +#endif + return RUN_ALL_TESTS(); }