[libc] Make fenv and math tests preserve fenv_t state (#89658)

This adds a new test fixture class FEnvSafeTest (usable as a base
class for other fixtures) that ensures each test doesn't perturb
the `fenv_t` state that the next test will start with.  It also
provides types and methods tests can use to explicitly wrap code
under test either to check that it doesn't perturb the state or
to save and restore the state around particular test code.

All the fenv and math tests are updated to use this so that none
can affect another.  Expectations that code under test and/or
tests themselves don't perturb state can be added later.

Author: frobtech

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.