Skip to content

Commit

Permalink
Allow FPEs to be caught as C++ exceptions
Browse files Browse the repository at this point in the history
This mostly worked on clang already, but gcc needed a command line
flag.
  • Loading branch information
wthrowe committed Aug 10, 2023
1 parent 9376847 commit d83fc0d
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 41 deletions.
10 changes: 8 additions & 2 deletions cmake/SetupCxxFlags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/DataStructures/Tensor/Structure.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion src/Utilities/ErrorHandling/FloatingPointExceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand Down
73 changes: 35 additions & 38 deletions tests/Unit/ErrorHandling/Test_FloatingPointExceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <cmath>
#include <limits>
#include <stdexcept>

#include "Utilities/ErrorHandling/FloatingPointExceptions.hpp"

Expand All @@ -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",
Expand Down

0 comments on commit d83fc0d

Please sign in to comment.