From d83fc0d1273405d97a4da6b4e7539fb995d448a1 Mon Sep 17 00:00:00 2001 From: William Throwe Date: Fri, 21 Jul 2023 18:10:54 -0400 Subject: [PATCH] Allow FPEs to be caught as C++ exceptions This mostly worked on clang already, but gcc needed a command line flag. --- cmake/SetupCxxFlags.cmake | 10 ++- src/DataStructures/Tensor/Structure.hpp | 7 ++ .../ErrorHandling/FloatingPointExceptions.cpp | 13 +++- .../Test_FloatingPointExceptions.cpp | 73 +++++++++---------- 4 files changed, 62 insertions(+), 41 deletions(-) diff --git a/cmake/SetupCxxFlags.cmake b/cmake/SetupCxxFlags.cmake index 9621760f1144..686adbaa57e6 100644 --- a/cmake/SetupCxxFlags.cmake +++ b/cmake/SetupCxxFlags.cmake @@ -122,8 +122,14 @@ set_property(TARGET SpectreFlags $<$:-freciprocal-math> $<$:-freciprocal-math>) -# By default, the LLVM optimizer assumes floating point exceptions are ignored. -create_cxx_flag_target("-ffp-exception-behavior=maytrap" SpectreFpExceptions) +# -ffp-exception-behavior=maytrap - By default, the LLVM optimizer assumes +# floating point exceptions are ignored. +# -fnon-call-exceptions - By default, GCC does not allow signal handlers to +# throw exceptions. +create_cxx_flags_target( + "-ffp-exception-behavior=maytrap;-fnon-call-exceptions" + SpectreFpExceptions + ) target_link_libraries( SpectreFlags INTERFACE diff --git a/src/DataStructures/Tensor/Structure.hpp b/src/DataStructures/Tensor/Structure.hpp index 1301c60fff62..774c9a1a1cea 100644 --- a/src/DataStructures/Tensor/Structure.hpp +++ b/src/DataStructures/Tensor/Structure.hpp @@ -477,12 +477,19 @@ struct Structure { static_assert(sizeof...(Indices) == sizeof...(N), "the number arguments must be equal to rank_"); constexpr auto collapsed_to_storage = collapsed_to_storage_; +#if defined(__GNUC__) and not defined(__clang__) and __GNUC__ >= 10 and __GNUC__ < 12 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif return gsl::at( collapsed_to_storage, compute_collapsed_index( cpp20::array{{static_cast(args)...}}, make_cpp20_array_from_list>())); +#if defined(__GNUC__) and not defined(__clang__) and __GNUC__ >= 10 and __GNUC__ < 12 +#pragma GCC diagnostic pop +#endif } /// \brief Get the storage_index of a tensor_index diff --git a/src/Utilities/ErrorHandling/FloatingPointExceptions.cpp b/src/Utilities/ErrorHandling/FloatingPointExceptions.cpp index 467df743b800..eb4007370a03 100644 --- a/src/Utilities/ErrorHandling/FloatingPointExceptions.cpp +++ b/src/Utilities/ErrorHandling/FloatingPointExceptions.cpp @@ -25,6 +25,11 @@ const int exception_flags = FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW; #endif [[noreturn]] void fpe_signal_handler(int /*signal*/) { +#if SPECTRE_FPE_CSR + _mm_setcsr(exception_flags); +#elif SPECTRE_FPE_FENV + feenableexcept(exception_flags); +#endif ERROR("Floating point exception!"); } } // namespace @@ -35,7 +40,13 @@ void enable_floating_point_exceptions() { #elif SPECTRE_FPE_FENV feenableexcept(exception_flags); #endif - std::signal(SIGFPE, fpe_signal_handler); + + struct sigaction handler {}; + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) + handler.sa_handler = fpe_signal_handler; + handler.sa_flags = SA_NODEFER; + sigemptyset(&handler.sa_mask); + sigaction(SIGFPE, &handler, nullptr); } void disable_floating_point_exceptions() { diff --git a/tests/Unit/ErrorHandling/Test_FloatingPointExceptions.cpp b/tests/Unit/ErrorHandling/Test_FloatingPointExceptions.cpp index dd10e5a33fd5..a84120494f0e 100644 --- a/tests/Unit/ErrorHandling/Test_FloatingPointExceptions.cpp +++ b/tests/Unit/ErrorHandling/Test_FloatingPointExceptions.cpp @@ -5,6 +5,7 @@ #include #include +#include #include "Utilities/ErrorHandling/FloatingPointExceptions.hpp" @@ -18,58 +19,54 @@ // the arm64 architecture, so when building on Apple Silicon, // directly call the fpe_signal_handler in these tests so that they pass. -// [[OutputRegex, Floating point exception!]] -SPECTRE_TEST_CASE("Unit.ErrorHandling.FloatingPointExceptions.Invalid", - "[ErrorHandling][Unit]") { - ERROR_TEST(); - -#ifdef __APPLE__ -#ifdef __arm64__ - ERROR("Floating point exception!"); -#endif -#endif - - enable_floating_point_exceptions(); +namespace { +// Compilers (both gcc and clang) seem prone to deciding that these +// can't actually throw exceptions and then optimizing away the +// try-catch logic (despite explicit requests to treat floating-point +// instructions as potentially throwing). All this nonsense is to +// convince them not to do that. In real code nothing is likely to be +// simple enough that the compiler can "prove" to itself that no +// exception can be thrown, so hopefully this won't be a real issue. +[[noreturn]] __attribute__((noinline)) void throw_invalid() { volatile double x = -1.0; volatile double invalid = sqrt(x); static_cast(invalid); - CHECK(true); + asm(""); + throw std::runtime_error("wrong"); } -// [[OutputRegex, Floating point exception!]] -SPECTRE_TEST_CASE("Unit.ErrorHandling.FloatingPointExceptions.Overflow", - "[ErrorHandling][Unit]") { - ERROR_TEST(); - -#ifdef __APPLE__ -#ifdef __arm64__ - ERROR("Floating point exception!"); -#endif -#endif - - enable_floating_point_exceptions(); +[[noreturn]] __attribute__((noinline)) void throw_overflow() { volatile double overflow = std::numeric_limits::max(); overflow *= 1.0e300; (void)overflow; - CHECK(true); + asm(""); + throw std::runtime_error("wrong"); } -// [[OutputRegex, Floating point exception!]] -SPECTRE_TEST_CASE("Unit.ErrorHandling.FloatingPointExceptions.DivByZero", - "[ErrorHandling][Unit]") { - ERROR_TEST(); - -#ifdef __APPLE__ -#ifdef __arm64__ - ERROR("Floating point exception!"); -#endif -#endif - - enable_floating_point_exceptions(); +[[noreturn]] __attribute__((noinline)) void throw_div_by_zero() { volatile double div_by_zero = 1.0; div_by_zero /= 0.0; (void)div_by_zero; + asm(""); + throw std::runtime_error("wrong"); +} +} // namespace + +SPECTRE_TEST_CASE("Unit.ErrorHandling.FloatingPointExceptions", + "[ErrorHandling][Unit]") { +#if defined(__APPLE__) and defined(__arm64__) CHECK(true); +#else + enable_floating_point_exceptions(); + CHECK_THROWS_WITH(throw_invalid(), Catch::Matchers::ContainsSubstring( + "Floating point exception!")); + + CHECK_THROWS_WITH(throw_overflow(), Catch::Matchers::ContainsSubstring( + "Floating point exception!")); + + CHECK_THROWS_WITH(throw_div_by_zero(), Catch::Matchers::ContainsSubstring( + "Floating point exception!")); +#endif } SPECTRE_TEST_CASE("Unit.ErrorHandling.FloatingPointExceptions.Disable",