[libc] Make fenv and math tests preserve fenv_t state

libc/test/UnitTest/CMakeLists.txt

@@ -111,8 +111,10 @@ add_header_library(
 add_unittest_framework_library(
   LibcFPTestHelpers
   SRCS
+    FEnvSafeTest.cpp
     RoundingModeUtils.cpp
   HDRS
+    FEnvSafeTest.h
     FPMatcher.h
     RoundingModeUtils.h
   DEPENDS

libc/test/UnitTest/FEnvSafeTest.cpp

@@ -0,0 +1,84 @@
+//===-- FEnvSafeTest.cpp ---------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+
+#include "FEnvSafeTest.h"
+
+#include "src/__support/FPUtil/FEnvImpl.h"
+#include "src/__support/macros/properties/architectures.h"
+
+namespace LIBC_NAMESPACE::testing {
+
+void FEnvSafeTest::PreserveFEnv::check() {
+  fenv_t after;
+  test.get_fenv(after);
+  test.expect_fenv_eq(before, after);
+}
+
+void FEnvSafeTest::TearDown() {
+  if (!should_be_unchanged) {
+    restore_fenv();
+  }
+}
+
+void FEnvSafeTest::get_fenv(fenv_t &fenv) {
+  ASSERT_EQ(LIBC_NAMESPACE::fputil::get_env(&fenv), 0);
+}
+
+void FEnvSafeTest::set_fenv(const fenv_t &fenv) {
+  ASSERT_EQ(LIBC_NAMESPACE::fputil::set_env(&fenv), 0);
+}
+
+void FEnvSafeTest::expect_fenv_eq(const fenv_t &before_fenv,
+                                  const fenv_t &after_fenv) {
+#if defined(LIBC_TARGET_ARCH_IS_AARCH64)
+  using LIBC_NAMESPACE::fputil::FEnv::FPState;
+  const FPState &before_state = reinterpret_cast<const FPState &>(before_fenv);
+  const FPState &after_state = reinterpret_cast<const FPState &>(after_fenv);
+
+  EXPECT_EQ(before_state.ControlWord, after_state.ControlWord);
+  EXPECT_EQ(before_state.StatusWord, after_state.StatusWord);
+
+#elif defined(LIBC_TARGET_ARCH_IS_X86) && !defined(__APPLE__)
+  using LIBC_NAMESPACE::fputil::internal::FPState;
+  const FPState &before_state = reinterpret_cast<const FPState &>(before_fenv);
+  const FPState &after_state = reinterpret_cast<const FPState &>(after_fenv);
+
+#if defined(_WIN32)
+  EXPECT_EQ(before_state.control_word, after_state.control_word);
+  EXPECT_EQ(before_state.status_word, after_state.status_word);
+#elif defined(__APPLE__)
+  EXPECT_EQ(before_state.control_word, after_state.control_word);
+  EXPECT_EQ(before_state.status_word, after_state.status_word);
+  EXPECT_EQ(before_state.mxcsr, after_state.mxcsr);
+#else
+  EXPECT_EQ(before_state.x87_status.control_word,
+            after_state.x87_status.control_word);
+  EXPECT_EQ(before_state.x87_status.status_word,
+            after_state.x87_status.status_word);
+  EXPECT_EQ(before_state.mxcsr, after_state.mxcsr);
+#endif
+
+#elif defined(LIBC_TARGET_ARCH_IS_ARM) && defined(__ARM_FP)
+  using LIBC_NAMESPACE::fputil::FEnv;
+  const FEnv &before_state = reinterpret_cast<const FEnv &>(before_fenv);
+  const FEnv &after_state = reinterpret_cast<const FEnv &>(after_fenv);
+
+  EXPECT_EQ(before_state.fpscr, after_state.fpscr);
+
+#elif defined(LIBC_TARGET_ARCH_IS_ANY_RISCV)
+  const uint32_t &before_fcsr = reinterpret_cast<const uint32_t &>(before_fenv);
+  const uint32_t &after_fcsr = reinterpret_cast<const uint32_t &>(after_fenv);
+  EXPECT_EQ(before_fcsr, after_fcsr);
+
+#else
+  // No arch-specific `fenv_t` support, so nothing to compare.
+
+#endif
+}
+
+} // namespace LIBC_NAMESPACE::testing

libc/test/UnitTest/FEnvSafeTest.h

@@ -0,0 +1,101 @@
+//===-- FEnvSafeTest.h -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_TEST_UNITTEST_FPENVSAFE_H
+#define LLVM_LIBC_TEST_UNITTEST_FPENVSAFE_H
+
+#include "hdr/types/fenv_t.h"
+#include "src/__support/CPP/utility.h"
+#include "test/UnitTest/Test.h"
+
+namespace LIBC_NAMESPACE::testing {
+
+// This provides a test fixture (or base class for other test fixtures) that
+// asserts that each test does not leave the FPU state represented by `fenv_t`
+// (aka `FPState`) perturbed from its initial state.
+class FEnvSafeTest : public Test {
+public:
+  void TearDown() override;
+
+protected:
+  // This is an RAII type where `PreserveFEnv preserve{this};` will sample the
+  // `fenv_t` state and restore it when `preserve` goes out of scope.
+  class PreserveFEnv {
+    fenv_t before;
+    FEnvSafeTest &test;
+
+  public:
+    explicit PreserveFEnv(FEnvSafeTest *self) : test{*self} {
+      test.get_fenv(before);
+    }
+
+    // Cause test expectation failures if the current state doesn't match what
+    // was captured in the constructor.
+    void check();
+
+    // Restore the state captured in the constructor.
+    void restore() { test.set_fenv(before); }
+
+    ~PreserveFEnv() { restore(); }
+  };
+
+  // This is an RAII type where `CheckFEnv check{this};` will sample the
+  // `fenv_t` state and require it be the same when `check` goes out of scope.
+  struct CheckFEnv : public PreserveFEnv {
+    using PreserveFEnv::PreserveFEnv;
+
+    ~CheckFEnv() { check(); }
+  };
+
+  // This calls callable() and returns its value, but has EXPECT_* failures if
+  // the `fenv_t` state is not preserved by the call.
+  template <typename T> decltype(auto) check_fenv_preserved(T &&callable) {
+    CheckFEnv check{this};
+    return cpp::forward<T>(callable)();
+  }
+
+  // This calls callable() and returns its value, but saves and restores the
+  // `fenv_t` state around the call.
+  template <typename T>
+  auto with_fenv_preserved(T &&callable)
+      -> decltype(cpp::forward<decltype(callable)>(callable)()) {
+    PreserveFEnv preserve{this};
+    return cpp::forward<T>(callable)();
+  }
+
+  // A test can call these to indicate it will or won't change `fenv_t` state.
+  void will_change_fenv() { should_be_unchanged = false; }
+  void will_not_change_fenv() { should_be_unchanged = true; }
+
+  // This explicitly resets back to the "before" state captured in SetUp().
+  // TearDown() always does this, but should_be_unchanged controls whether
+  // it also causes test failures if a test fails to restore it.
+  void restore_fenv() { check.restore(); }
+
+private:
+  void get_fenv(fenv_t &fenv);
+  void set_fenv(const fenv_t &fenv);
+  void expect_fenv_eq(const fenv_t &before_fenv, const fenv_t &after_fenv);
+
+  CheckFEnv check{this};
+
+  // TODO: Many tests fail if this is true. It needs to be figured out whether
+  // the state should be preserved by each library function under test, and
+  // separately whether each test itself should preserve the state.  It
+  // probably isn't important that tests be explicitly written to preserve the
+  // state, as the fixture can (and does) reset it--the next test can rely on
+  // getting "normal" ambient state initially.  For library functions that
+  // should preserve the state, that should be checked after each call, not
+  // just after the whole test.  So they can use check_fenv_preserved or
+  // with_fenv_preserved as appropriate.
+  bool should_be_unchanged = false;
+};
+
+} // namespace LIBC_NAMESPACE::testing
+
+#endif // LLVM_LIBC_TEST_UNITTEST_FPENVSAFE_H

libc/test/src/fenv/CMakeLists.txt

@@ -9,6 +9,8 @@ add_libc_unittest(
   DEPENDS
     libc.src.fenv.fegetround
     libc.src.fenv.fesetround
+  LINK_LIBRARIES
+    LibcFPTestHelpers
 )
 
 add_libc_unittest(
@@ -23,6 +25,8 @@ add_libc_unittest(
     libc.src.fenv.fesetexcept
     libc.src.fenv.fetestexcept
     libc.src.__support.FPUtil.fenv_impl
+  LINK_LIBRARIES
+    LibcFPTestHelpers
 )
 
 add_libc_unittest(
@@ -37,6 +41,8 @@ add_libc_unittest(
     libc.src.fenv.fesetenv
     libc.src.fenv.fesetround
     libc.src.__support.FPUtil.fenv_impl
+  LINK_LIBRARIES
+    LibcFPTestHelpers
 )
 
 add_libc_unittest(
@@ -50,6 +56,8 @@ add_libc_unittest(
     libc.src.fenv.fesetexceptflag
     libc.src.fenv.fetestexceptflag
     libc.src.__support.FPUtil.fenv_impl
+  LINK_LIBRARIES
+    LibcFPTestHelpers
 )
 
 add_libc_unittest(
@@ -62,6 +70,8 @@ add_libc_unittest(
     libc.include.signal
     libc.src.fenv.feupdateenv
     libc.src.__support.FPUtil.fenv_impl
+  LINK_LIBRARIES
+    LibcFPTestHelpers
 )
 
 add_libc_unittest(
@@ -73,6 +83,8 @@ add_libc_unittest(
   DEPENDS
     libc.src.fenv.feclearexcept
     libc.src.__support.FPUtil.fenv_impl
+  LINK_LIBRARIES
+    LibcFPTestHelpers
 )
 
 add_libc_unittest(
@@ -85,6 +97,8 @@ add_libc_unittest(
     libc.src.fenv.fedisableexcept
     libc.src.fenv.feenableexcept
     libc.src.fenv.fegetexcept
+  LINK_LIBRARIES
+    LibcFPTestHelpers
 )
 
 if (NOT (LLVM_USE_SANITIZER OR (${LIBC_TARGET_OS} STREQUAL "windows")
@@ -109,6 +123,7 @@ if (NOT (LLVM_USE_SANITIZER OR (${LIBC_TARGET_OS} STREQUAL "windows")
       libc.src.__support.FPUtil.fenv_impl
     LINK_LIBRARIES
       LibcFPExceptionHelpers
+      LibcFPTestHelpers
   )
 
   add_fp_unittest(
@@ -124,5 +139,6 @@ if (NOT (LLVM_USE_SANITIZER OR (${LIBC_TARGET_OS} STREQUAL "windows")
       libc.src.__support.FPUtil.fenv_impl
     LINK_LIBRARIES
       LibcFPExceptionHelpers
+      LibcFPTestHelpers
   )
 endif()

libc/test/src/fenv/enabled_exceptions_test.cpp

@@ -12,15 +12,20 @@
 
 #include "src/__support/FPUtil/FEnvImpl.h"
 #include "src/__support/macros/properties/architectures.h"
+#include "test/UnitTest/FEnvSafeTest.h"
 #include "test/UnitTest/FPExceptMatcher.h"
 #include "test/UnitTest/Test.h"
 
 #include "hdr/fenv_macros.h"
 #include <signal.h>
 
+#include "excepts.h"
+
+using LlvmLibcExceptionStatusTest = LIBC_NAMESPACE::testing::FEnvSafeTest;
+
 // This test enables an exception and verifies that raising that exception
 // triggers SIGFPE.
-TEST(LlvmLibcExceptionStatusTest, RaiseAndCrash) {
+TEST_F(LlvmLibcExceptionStatusTest, RaiseAndCrash) {
 #if defined(LIBC_TARGET_ARCH_IS_ANY_ARM) ||                                    \
     defined(LIBC_TARGET_ARCH_IS_ANY_RISCV)
   // Few Arm HW implementations do not trap exceptions. We skip this test
@@ -41,16 +46,7 @@ TEST(LlvmLibcExceptionStatusTest, RaiseAndCrash) {
   // that exception handler, so such a testing can be done after we have
   // longjmp implemented.
 
-  int excepts[] = {FE_DIVBYZERO, FE_INVALID, FE_INEXACT, FE_OVERFLOW,
-                   FE_UNDERFLOW};
-
-  // We '|' the individual exception flags instead of using FE_ALL_EXCEPT
-  // as it can include non-standard extensions. Note that we should be able
-  // to compile this file with headers from other libcs as well.
-  constexpr int ALL_EXCEPTS =
-      FE_DIVBYZERO | FE_INVALID | FE_INEXACT | FE_OVERFLOW | FE_UNDERFLOW;
-
-  for (int e : excepts) {
+  for (int e : EXCEPTS) {
     LIBC_NAMESPACE::fputil::disable_except(FE_ALL_EXCEPT);
     LIBC_NAMESPACE::fputil::enable_except(e);
     ASSERT_EQ(LIBC_NAMESPACE::feclearexcept(FE_ALL_EXCEPT), 0);

libc/test/src/fenv/exception_flags_test.cpp

@@ -12,18 +12,20 @@
 #include "src/fenv/fetestexceptflag.h"
 
 #include "src/__support/FPUtil/FEnvImpl.h"
+#include "test/UnitTest/FEnvSafeTest.h"
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcFenvTest, GetSetTestExceptFlag) {
+#include "excepts.h"
+
+using LlvmLibcFEnvTest = LIBC_NAMESPACE::testing::FEnvSafeTest;
+
+TEST_F(LlvmLibcFEnvTest, GetSetTestExceptFlag) {
   // We will disable all exceptions to prevent invocation of the exception
   // handler.
   LIBC_NAMESPACE::fputil::disable_except(FE_ALL_EXCEPT);
   LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
 
-  int excepts[] = {FE_DIVBYZERO, FE_INVALID, FE_INEXACT, FE_OVERFLOW,
-                   FE_UNDERFLOW};
-
-  for (int e : excepts) {
+  for (int e : EXCEPTS) {
     // The overall idea is to raise an except and save the exception flags.
     // Next, clear the flags and then set the saved exception flags. This
     // should set the flag corresponding to the previously raised exception.