Skip to content

Commit 837dab9

Browse files
authored
[libc] Make fenv and math tests preserve fenv_t state (llvm#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.
1 parent 2662bce commit 837dab9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+471
-129
lines changed

libc/test/UnitTest/CMakeLists.txt

+2
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,10 @@ add_header_library(
111111
add_unittest_framework_library(
112112
LibcFPTestHelpers
113113
SRCS
114+
FEnvSafeTest.cpp
114115
RoundingModeUtils.cpp
115116
HDRS
117+
FEnvSafeTest.h
116118
FPMatcher.h
117119
RoundingModeUtils.h
118120
DEPENDS

libc/test/UnitTest/FEnvSafeTest.cpp

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
//===-- FEnvSafeTest.cpp ---------------------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===---------------------------------------------------------------------===//
8+
9+
#include "FEnvSafeTest.h"
10+
11+
#include "src/__support/FPUtil/FEnvImpl.h"
12+
#include "src/__support/macros/properties/architectures.h"
13+
14+
namespace LIBC_NAMESPACE::testing {
15+
16+
void FEnvSafeTest::PreserveFEnv::check() {
17+
fenv_t after;
18+
test.get_fenv(after);
19+
test.expect_fenv_eq(before, after);
20+
}
21+
22+
void FEnvSafeTest::TearDown() {
23+
if (!should_be_unchanged) {
24+
restore_fenv();
25+
}
26+
}
27+
28+
void FEnvSafeTest::get_fenv(fenv_t &fenv) {
29+
ASSERT_EQ(LIBC_NAMESPACE::fputil::get_env(&fenv), 0);
30+
}
31+
32+
void FEnvSafeTest::set_fenv(const fenv_t &fenv) {
33+
ASSERT_EQ(LIBC_NAMESPACE::fputil::set_env(&fenv), 0);
34+
}
35+
36+
void FEnvSafeTest::expect_fenv_eq(const fenv_t &before_fenv,
37+
const fenv_t &after_fenv) {
38+
#if defined(LIBC_TARGET_ARCH_IS_AARCH64)
39+
using LIBC_NAMESPACE::fputil::FEnv::FPState;
40+
const FPState &before_state = reinterpret_cast<const FPState &>(before_fenv);
41+
const FPState &after_state = reinterpret_cast<const FPState &>(after_fenv);
42+
43+
EXPECT_EQ(before_state.ControlWord, after_state.ControlWord);
44+
EXPECT_EQ(before_state.StatusWord, after_state.StatusWord);
45+
46+
#elif defined(LIBC_TARGET_ARCH_IS_X86) && !defined(__APPLE__)
47+
using LIBC_NAMESPACE::fputil::internal::FPState;
48+
const FPState &before_state = reinterpret_cast<const FPState &>(before_fenv);
49+
const FPState &after_state = reinterpret_cast<const FPState &>(after_fenv);
50+
51+
#if defined(_WIN32)
52+
EXPECT_EQ(before_state.control_word, after_state.control_word);
53+
EXPECT_EQ(before_state.status_word, after_state.status_word);
54+
#elif defined(__APPLE__)
55+
EXPECT_EQ(before_state.control_word, after_state.control_word);
56+
EXPECT_EQ(before_state.status_word, after_state.status_word);
57+
EXPECT_EQ(before_state.mxcsr, after_state.mxcsr);
58+
#else
59+
EXPECT_EQ(before_state.x87_status.control_word,
60+
after_state.x87_status.control_word);
61+
EXPECT_EQ(before_state.x87_status.status_word,
62+
after_state.x87_status.status_word);
63+
EXPECT_EQ(before_state.mxcsr, after_state.mxcsr);
64+
#endif
65+
66+
#elif defined(LIBC_TARGET_ARCH_IS_ARM) && defined(__ARM_FP)
67+
using LIBC_NAMESPACE::fputil::FEnv;
68+
const FEnv &before_state = reinterpret_cast<const FEnv &>(before_fenv);
69+
const FEnv &after_state = reinterpret_cast<const FEnv &>(after_fenv);
70+
71+
EXPECT_EQ(before_state.fpscr, after_state.fpscr);
72+
73+
#elif defined(LIBC_TARGET_ARCH_IS_ANY_RISCV)
74+
const uint32_t &before_fcsr = reinterpret_cast<const uint32_t &>(before_fenv);
75+
const uint32_t &after_fcsr = reinterpret_cast<const uint32_t &>(after_fenv);
76+
EXPECT_EQ(before_fcsr, after_fcsr);
77+
78+
#else
79+
// No arch-specific `fenv_t` support, so nothing to compare.
80+
81+
#endif
82+
}
83+
84+
} // namespace LIBC_NAMESPACE::testing

libc/test/UnitTest/FEnvSafeTest.h

+101
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//===-- FEnvSafeTest.h -----------------------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===---------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_LIBC_TEST_UNITTEST_FPENVSAFE_H
10+
#define LLVM_LIBC_TEST_UNITTEST_FPENVSAFE_H
11+
12+
#include "hdr/types/fenv_t.h"
13+
#include "src/__support/CPP/utility.h"
14+
#include "test/UnitTest/Test.h"
15+
16+
namespace LIBC_NAMESPACE::testing {
17+
18+
// This provides a test fixture (or base class for other test fixtures) that
19+
// asserts that each test does not leave the FPU state represented by `fenv_t`
20+
// (aka `FPState`) perturbed from its initial state.
21+
class FEnvSafeTest : public Test {
22+
public:
23+
void TearDown() override;
24+
25+
protected:
26+
// This is an RAII type where `PreserveFEnv preserve{this};` will sample the
27+
// `fenv_t` state and restore it when `preserve` goes out of scope.
28+
class PreserveFEnv {
29+
fenv_t before;
30+
FEnvSafeTest &test;
31+
32+
public:
33+
explicit PreserveFEnv(FEnvSafeTest *self) : test{*self} {
34+
test.get_fenv(before);
35+
}
36+
37+
// Cause test expectation failures if the current state doesn't match what
38+
// was captured in the constructor.
39+
void check();
40+
41+
// Restore the state captured in the constructor.
42+
void restore() { test.set_fenv(before); }
43+
44+
~PreserveFEnv() { restore(); }
45+
};
46+
47+
// This is an RAII type where `CheckFEnv check{this};` will sample the
48+
// `fenv_t` state and require it be the same when `check` goes out of scope.
49+
struct CheckFEnv : public PreserveFEnv {
50+
using PreserveFEnv::PreserveFEnv;
51+
52+
~CheckFEnv() { check(); }
53+
};
54+
55+
// This calls callable() and returns its value, but has EXPECT_* failures if
56+
// the `fenv_t` state is not preserved by the call.
57+
template <typename T> decltype(auto) check_fenv_preserved(T &&callable) {
58+
CheckFEnv check{this};
59+
return cpp::forward<T>(callable)();
60+
}
61+
62+
// This calls callable() and returns its value, but saves and restores the
63+
// `fenv_t` state around the call.
64+
template <typename T>
65+
auto with_fenv_preserved(T &&callable)
66+
-> decltype(cpp::forward<decltype(callable)>(callable)()) {
67+
PreserveFEnv preserve{this};
68+
return cpp::forward<T>(callable)();
69+
}
70+
71+
// A test can call these to indicate it will or won't change `fenv_t` state.
72+
void will_change_fenv() { should_be_unchanged = false; }
73+
void will_not_change_fenv() { should_be_unchanged = true; }
74+
75+
// This explicitly resets back to the "before" state captured in SetUp().
76+
// TearDown() always does this, but should_be_unchanged controls whether
77+
// it also causes test failures if a test fails to restore it.
78+
void restore_fenv() { check.restore(); }
79+
80+
private:
81+
void get_fenv(fenv_t &fenv);
82+
void set_fenv(const fenv_t &fenv);
83+
void expect_fenv_eq(const fenv_t &before_fenv, const fenv_t &after_fenv);
84+
85+
CheckFEnv check{this};
86+
87+
// TODO: Many tests fail if this is true. It needs to be figured out whether
88+
// the state should be preserved by each library function under test, and
89+
// separately whether each test itself should preserve the state. It
90+
// probably isn't important that tests be explicitly written to preserve the
91+
// state, as the fixture can (and does) reset it--the next test can rely on
92+
// getting "normal" ambient state initially. For library functions that
93+
// should preserve the state, that should be checked after each call, not
94+
// just after the whole test. So they can use check_fenv_preserved or
95+
// with_fenv_preserved as appropriate.
96+
bool should_be_unchanged = false;
97+
};
98+
99+
} // namespace LIBC_NAMESPACE::testing
100+
101+
#endif // LLVM_LIBC_TEST_UNITTEST_FPENVSAFE_H

libc/test/src/fenv/CMakeLists.txt

+16
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ add_libc_unittest(
99
DEPENDS
1010
libc.src.fenv.fegetround
1111
libc.src.fenv.fesetround
12+
LINK_LIBRARIES
13+
LibcFPTestHelpers
1214
)
1315

1416
add_libc_unittest(
@@ -23,6 +25,8 @@ add_libc_unittest(
2325
libc.src.fenv.fesetexcept
2426
libc.src.fenv.fetestexcept
2527
libc.src.__support.FPUtil.fenv_impl
28+
LINK_LIBRARIES
29+
LibcFPTestHelpers
2630
)
2731

2832
add_libc_unittest(
@@ -37,6 +41,8 @@ add_libc_unittest(
3741
libc.src.fenv.fesetenv
3842
libc.src.fenv.fesetround
3943
libc.src.__support.FPUtil.fenv_impl
44+
LINK_LIBRARIES
45+
LibcFPTestHelpers
4046
)
4147

4248
add_libc_unittest(
@@ -50,6 +56,8 @@ add_libc_unittest(
5056
libc.src.fenv.fesetexceptflag
5157
libc.src.fenv.fetestexceptflag
5258
libc.src.__support.FPUtil.fenv_impl
59+
LINK_LIBRARIES
60+
LibcFPTestHelpers
5361
)
5462

5563
add_libc_unittest(
@@ -62,6 +70,8 @@ add_libc_unittest(
6270
libc.include.signal
6371
libc.src.fenv.feupdateenv
6472
libc.src.__support.FPUtil.fenv_impl
73+
LINK_LIBRARIES
74+
LibcFPTestHelpers
6575
)
6676

6777
add_libc_unittest(
@@ -73,6 +83,8 @@ add_libc_unittest(
7383
DEPENDS
7484
libc.src.fenv.feclearexcept
7585
libc.src.__support.FPUtil.fenv_impl
86+
LINK_LIBRARIES
87+
LibcFPTestHelpers
7688
)
7789

7890
add_libc_unittest(
@@ -85,6 +97,8 @@ add_libc_unittest(
8597
libc.src.fenv.fedisableexcept
8698
libc.src.fenv.feenableexcept
8799
libc.src.fenv.fegetexcept
100+
LINK_LIBRARIES
101+
LibcFPTestHelpers
88102
)
89103

90104
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")
109123
libc.src.__support.FPUtil.fenv_impl
110124
LINK_LIBRARIES
111125
LibcFPExceptionHelpers
126+
LibcFPTestHelpers
112127
)
113128

114129
add_fp_unittest(
@@ -124,5 +139,6 @@ if (NOT (LLVM_USE_SANITIZER OR (${LIBC_TARGET_OS} STREQUAL "windows")
124139
libc.src.__support.FPUtil.fenv_impl
125140
LINK_LIBRARIES
126141
LibcFPExceptionHelpers
142+
LibcFPTestHelpers
127143
)
128144
endif()

libc/test/src/fenv/enabled_exceptions_test.cpp

+7-11
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,20 @@
1212

1313
#include "src/__support/FPUtil/FEnvImpl.h"
1414
#include "src/__support/macros/properties/architectures.h"
15+
#include "test/UnitTest/FEnvSafeTest.h"
1516
#include "test/UnitTest/FPExceptMatcher.h"
1617
#include "test/UnitTest/Test.h"
1718

1819
#include "hdr/fenv_macros.h"
1920
#include <signal.h>
2021

22+
#include "excepts.h"
23+
24+
using LlvmLibcExceptionStatusTest = LIBC_NAMESPACE::testing::FEnvSafeTest;
25+
2126
// This test enables an exception and verifies that raising that exception
2227
// triggers SIGFPE.
23-
TEST(LlvmLibcExceptionStatusTest, RaiseAndCrash) {
28+
TEST_F(LlvmLibcExceptionStatusTest, RaiseAndCrash) {
2429
#if defined(LIBC_TARGET_ARCH_IS_ANY_ARM) || \
2530
defined(LIBC_TARGET_ARCH_IS_ANY_RISCV)
2631
// Few Arm HW implementations do not trap exceptions. We skip this test
@@ -41,16 +46,7 @@ TEST(LlvmLibcExceptionStatusTest, RaiseAndCrash) {
4146
// that exception handler, so such a testing can be done after we have
4247
// longjmp implemented.
4348

44-
int excepts[] = {FE_DIVBYZERO, FE_INVALID, FE_INEXACT, FE_OVERFLOW,
45-
FE_UNDERFLOW};
46-
47-
// We '|' the individual exception flags instead of using FE_ALL_EXCEPT
48-
// as it can include non-standard extensions. Note that we should be able
49-
// to compile this file with headers from other libcs as well.
50-
constexpr int ALL_EXCEPTS =
51-
FE_DIVBYZERO | FE_INVALID | FE_INEXACT | FE_OVERFLOW | FE_UNDERFLOW;
52-
53-
for (int e : excepts) {
49+
for (int e : EXCEPTS) {
5450
LIBC_NAMESPACE::fputil::disable_except(FE_ALL_EXCEPT);
5551
LIBC_NAMESPACE::fputil::enable_except(e);
5652
ASSERT_EQ(LIBC_NAMESPACE::feclearexcept(FE_ALL_EXCEPT), 0);

libc/test/src/fenv/exception_flags_test.cpp

+7-5
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,20 @@
1212
#include "src/fenv/fetestexceptflag.h"
1313

1414
#include "src/__support/FPUtil/FEnvImpl.h"
15+
#include "test/UnitTest/FEnvSafeTest.h"
1516
#include "test/UnitTest/Test.h"
1617

17-
TEST(LlvmLibcFenvTest, GetSetTestExceptFlag) {
18+
#include "excepts.h"
19+
20+
using LlvmLibcFEnvTest = LIBC_NAMESPACE::testing::FEnvSafeTest;
21+
22+
TEST_F(LlvmLibcFEnvTest, GetSetTestExceptFlag) {
1823
// We will disable all exceptions to prevent invocation of the exception
1924
// handler.
2025
LIBC_NAMESPACE::fputil::disable_except(FE_ALL_EXCEPT);
2126
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
2227

23-
int excepts[] = {FE_DIVBYZERO, FE_INVALID, FE_INEXACT, FE_OVERFLOW,
24-
FE_UNDERFLOW};
25-
26-
for (int e : excepts) {
28+
for (int e : EXCEPTS) {
2729
// The overall idea is to raise an except and save the exception flags.
2830
// Next, clear the flags and then set the saved exception flags. This
2931
// should set the flag corresponding to the previously raised exception.

0 commit comments

Comments
 (0)