Skip to content

[libc][test] adds errno clearer test fixture, gtest-style errno and fp except assertions #91608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 39 additions & 24 deletions libc/test/UnitTest/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,40 @@ add_header_library(
libc.src.__support.CPP.type_traits
)

add_unittest_framework_library(
LibcFPExceptionHelpers
SRCS
FPExceptMatcher.cpp
HDRS
FPExceptMatcher.h
DEPENDS
LibcTest
libc.src.__support.FPUtil.fp_bits
libc.src.__support.FPUtil.fenv_impl
libc.hdr.types.fenv_t
libc.hdr.fenv_macros
)

add_header_library(
ErrnoSetterMatcher
HDRS
ErrnoSetterMatcher.h
DEPENDS
libc.src.__support.common
libc.src.__support.FPUtil.fp_bits
libc.src.__support.StringUtil.error_to_string
libc.src.errno.errno
)

add_header_library(
ErrnoSafeTest
HDRS
ErrnoSafeTest.h
DEPENDS
libc.src.__support.CPP.utility
libc.src.errno.errno
)

add_unittest_framework_library(
LibcFPTestHelpers
SRCS
Expand All @@ -116,30 +150,22 @@ add_unittest_framework_library(
HDRS
FEnvSafeTest.h
FPMatcher.h
FPTest.h
RoundingModeUtils.h
DEPENDS
LibcTest
ErrnoSafeTest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually add .ErrnoSetterMatcher as relative paths. Is it better to unify the style?

cc @lntue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm down to make the change. Should we change the other names in this file too? lmk

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry for missing the reply. I am fine with unifying all of them at once.

ErrnoSetterMatcher
LibcFPExceptionHelpers
libc.test.UnitTest.string_utils
libc.src.__support.CPP.array
libc.src.__support.CPP.utility
libc.src.__support.FPUtil.fp_bits
libc.src.__support.FPUtil.fpbits_str
libc.src.__support.FPUtil.fenv_impl
libc.src.__support.FPUtil.rounding_mode
)

add_unittest_framework_library(
LibcFPExceptionHelpers
SRCS
FPExceptMatcher.cpp
HDRS
FPExceptMatcher.h
DEPENDS
LibcTest
libc.src.__support.FPUtil.fp_bits
libc.src.__support.FPUtil.fenv_impl
libc.hdr.types.fenv_t
)

add_unittest_framework_library(
LibcMemoryHelpers
SRCS
Expand Down Expand Up @@ -176,14 +202,3 @@ add_unittest_framework_library(
libc.src.stdio.scanf_core.core_structs
libc.test.UnitTest.string_utils
)

add_header_library(
ErrnoSetterMatcher
HDRS
ErrnoSetterMatcher.h
DEPENDS
libc.src.__support.common
libc.src.__support.FPUtil.fp_bits
libc.src.__support.StringUtil.error_to_string
libc.src.errno.errno
)
26 changes: 26 additions & 0 deletions libc/test/UnitTest/ErrnoSafeTest.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//===-- ErrnoSafeTestFixture.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_ERRNOSAFETEST_H
#define LLVM_LIBC_TEST_UNITTEST_ERRNOSAFETEST_H

#include "src/__support/CPP/utility.h"
#include "src/errno/libc_errno.h"
#include "test/UnitTest/Test.h"

namespace LIBC_NAMESPACE::testing {

// This is a test fixture for clearing errno before the start of a test case.
class ErrnoSafeTest : virtual public Test {
public:
void SetUp() override { LIBC_NAMESPACE::libc_errno = 0; }
};

} // namespace LIBC_NAMESPACE::testing

#endif // LLVM_LIBC_TEST_UNITTEST_ERRNOSAFETEST_H
36 changes: 36 additions & 0 deletions libc/test/UnitTest/ErrnoSetterMatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ template <typename T> struct Comparator {
#endif
};

// TODO: this should check errno and not always need a return value to
// also compare against. The FP and non-FP matching is redundant with
// the other matchers pulled in through Test.h and FPMatcher.h.
template <typename T> class ErrnoSetterMatcher : public Matcher<T> {
Comparator<T> return_cmp;
Comparator<int> errno_cmp;
Expand Down Expand Up @@ -184,4 +187,37 @@ static ErrnoSetterMatcherBuilder<RetT> returns(internal::Comparator<RetT> cmp) {
} // namespace testing
} // namespace LIBC_NAMESPACE

// Used to check that `LIBC_NAMESPACE::libc_errno` was 0 or a specific
// errno after executing `expr_or_statement` from a state where
// `LIBC_NAMESPACE::libc_errno` was 0. This is generic, so does not check
// `math_errhandling & MATH_ERRNO` before errno matching, see FPTest.h for
// assertions that check this.
//
// Expects `expected` to be convertible to int type.
//
// Does not return the value of expr_or_statement, i.e., intended usage
// is: `EXPECT_ERRNO(EDOM, EXPECT_EQ(..., ...));` or
// ```
// EXPECT_ERRNO(EDOM, {
// stmt;
// ...
// });
// ```
//
// TODO: this currently uses `ErrnoSetterMatcher` for the nice explanation on
// failed errno matching. `ErrnoSetterMatcher` requires a return value to also
// always check, so this code always checks 0 against 0 for the return value--
// it is not actually checking the value of `expr_or_statement` per above doc
// comments. When `ErrnoSetterMatcher` is changed to not always check return
// values, change this also.
#define EXPECT_ERRNO(expected, expr_or_statement) \
do { \
LIBC_NAMESPACE::libc_errno = 0; \
expr_or_statement; \
EXPECT_THAT( \
0, LIBC_NAMESPACE::testing::internal::ErrnoSetterMatcher<int>( \
LIBC_NAMESPACE::testing::ErrnoSetterMatcher::EQ(0), \
LIBC_NAMESPACE::testing::ErrnoSetterMatcher::EQ((expected)))); \
} while (0)

#endif // LLVM_LIBC_TEST_ERRNOSETTERMATCHER_H
2 changes: 1 addition & 1 deletion libc/test/UnitTest/FEnvSafeTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ 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 {
class FEnvSafeTest : virtual public Test {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this need to be virtual now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to virtual to let FPTest subclass both fixtures:

template <typename T> class FPTest : 
public ErrnoSafeTest, public FEnvSafeTest {...

public:
void TearDown() override;

Expand Down
3 changes: 2 additions & 1 deletion libc/test/UnitTest/FPExceptMatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "test/UnitTest/Test.h"

#include "hdr/fenv_macros.h"
#include "hdr/types/fenv_t.h"
#include "src/__support/FPUtil/FEnvImpl.h"
#include <memory>
Expand All @@ -35,7 +36,7 @@ static void sigfpeHandler(int sig) {
siglongjmp(jumpBuffer, -1);
}

FPExceptMatcher::FPExceptMatcher(FunctionCaller *func) {
FPExceptCallableMatcher::FPExceptCallableMatcher(FunctionCaller *func) {
auto oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
std::unique_ptr<FunctionCaller> funcUP(func);

Expand Down
102 changes: 98 additions & 4 deletions libc/test/UnitTest/FPExceptMatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#ifndef LLVM_LIBC_TEST_UNITTEST_FPEXCEPTMATCHER_H
#define LLVM_LIBC_TEST_UNITTEST_FPEXCEPTMATCHER_H

#include "hdr/fenv_macros.h"
#include "test/UnitTest/Test.h"
#include "test/UnitTest/TestLogger.h"

Expand All @@ -17,9 +18,71 @@
namespace LIBC_NAMESPACE {
namespace testing {

// Used to compare FP exception flag states with nice error printing
class FPExceptMatcher : public Matcher<int> {
const int expected;
int actual;

public:
explicit FPExceptMatcher(int expected) : expected(expected) {}

void explainError() override {
tlog << "Expected floating point exceptions: " << expected << ' ';
printExcepts(expected);
tlog << '\n';

tlog << "Actual floating point exceptions: " << actual << ' ';
printExcepts(actual);
tlog << '\n';
}

bool match(int got) {
actual = got;
return got == expected;
}

private:
void printExcepts(int excepts) {
if (!excepts) {
tlog << "(no exceptions)";
return;
}

bool firstPrinted = false;
auto printWithPipe = [&](const char *name) {
if (firstPrinted)
tlog << " | ";

tlog << name;

firstPrinted = true;
};

tlog << '(';

if (FE_DIVBYZERO & excepts)
printWithPipe("FE_DIVBYZERO");

if (FE_INEXACT & excepts)
printWithPipe("FE_INEXACT");

if (FE_INVALID & excepts)
printWithPipe("FE_INVALID");

if (FE_OVERFLOW & excepts)
printWithPipe("FE_OVERFLOW");

if (FE_UNDERFLOW & excepts)
printWithPipe("FE_UNDERFLOW");

tlog << ')';
}
};

// TODO: Make the matcher match specific exceptions instead of just identifying
// that an exception was raised.
class FPExceptMatcher : public Matcher<bool> {
// Used in death tests for fenv
class FPExceptCallableMatcher : public Matcher<bool> {
bool exceptionRaised;

public:
Expand All @@ -40,7 +103,7 @@ class FPExceptMatcher : public Matcher<bool> {
}

// Takes ownership of func.
explicit FPExceptMatcher(FunctionCaller *func);
explicit FPExceptCallableMatcher(FunctionCaller *func);

bool match(bool unused) { return exceptionRaised; }

Expand All @@ -53,11 +116,42 @@ class FPExceptMatcher : public Matcher<bool> {
} // namespace testing
} // namespace LIBC_NAMESPACE

// Matches on the FP exception flag `expected` being *equal* to FP exception
// flag `actual`
#define EXPECT_FP_EXCEPTION_EQ(expected, actual) \
EXPECT_THAT((actual), LIBC_NAMESPACE::testing::FPExceptMatcher((expected)))

#define ASSERT_FP_EXCEPT_EQ(expected, actual) \
ASSERT_THAT((actual), LIBC_NAMESPACE::testing::FPExceptMatcher((expected)))

#define ASSERT_RAISES_FP_EXCEPT(func) \
ASSERT_THAT( \
true, \
LIBC_NAMESPACE::testing::FPExceptMatcher( \
LIBC_NAMESPACE::testing::FPExceptMatcher::getFunctionCaller(func)))
LIBC_NAMESPACE::testing::FPExceptCallableMatcher( \
LIBC_NAMESPACE::testing::FPExceptCallableMatcher::getFunctionCaller( \
func)))

// Does not return the value of `expr_or_statement`, i.e., intended usage
// is: `EXPECT_FP_EXCEPT(FE_INVALID, EXPECT_FP_EQ(..., ...));` or
// ```
// EXPECT_FP_EXCEPT(FE_ALL_EXCEPT, {
// stmt;
// ...
// });
// ```
// Ensures that fp excepts are cleared before executing `expr_or_statement`
// Checking (expected = 0) should ensure that no exceptions were set
#define EXPECT_FP_EXCEPTION(expected, expr_or_statement) \
do { \
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT); \
expr_or_statement; \
int expected_ = (expected); \
int mask_ = expected_ ? expected_ : FE_ALL_EXCEPT; \
if (math_errhandling & MATH_ERREXCEPT) { \
EXPECT_FP_EXCEPTION_EQ(expected_, \
LIBC_NAMESPACE::fputil::test_except(mask_)); \
} \
} while (0)

#else // !LIBC_TEST_HAS_MATCHERS()

Expand Down
Loading
Loading