Merge pull request sxs-collaboration#5324 from wthrowe/fpe_recovery

Allow FPEs to be caught as C++ exceptions

cmake/SetupCxxFlags.cmake

@@ -122,8 +122,14 @@ set_property(TARGET SpectreFlags
   $<$<COMPILE_LANGUAGE:CXX>:-freciprocal-math>
   $<$<COMPILE_LANGUAGE:Fortran>:-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

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<size_t, sizeof...(N)>{{static_cast<size_t>(args)...}},
             make_cpp20_array_from_list<tmpl::conditional_t<
                 0 != sizeof...(Indices), index_list, size_t>>()));
+#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

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() {

tests/Unit/ErrorHandling/Test_FloatingPointExceptions.cpp

@@ -5,6 +5,7 @@
 
 #include <cmath>
 #include <limits>
+#include <stdexcept>
 
 #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<void>(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<double>::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",