Skip to content

[libc] Properly clear out errno in tests that check its value. #131650

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[libc] Properly clear out errno in tests that check its value. #131650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions libc/test/src/math/smoke/FModTest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

#define TEST_SPECIAL(x, y, expected, dom_err, expected_exception) \
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT); \
LIBC_NAMESPACE::libc_errno = 0; \
EXPECT_FP_EQ(expected, f(x, y)); \
EXPECT_MATH_ERRNO((dom_err) ? EDOM : 0); \
EXPECT_FP_EXCEPTION(expected_exception); \
LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT)
EXPECT_FP_EXCEPTION(expected_exception)

#define TEST_REGULAR(x, y, expected) TEST_SPECIAL(x, y, expected, false, 0)

Expand Down
2 changes: 2 additions & 0 deletions libc/test/src/sys/epoll/linux/epoll_create1_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;

TEST(LlvmLibcEpollCreate1Test, Basic) {
LIBC_NAMESPACE::libc_errno = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

imo we should be resetting errno at the end of tests instead of at the start. Ideally we should assume we start from a clean state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This assumption doesn't hold - we see cases when errno (and previously - fp exceptions) are set to non-zero values before the first test case is being executed -- e.g. by some code that runs pre-main(). Clearing out the state at the beginning of the tests would fix these cases and make the tests more hermetic.

Copy link
Contributor

Choose a reason for hiding this comment

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

in that case this is fine, though I do wonder if we should leave the bazel build using LIBC_ERRNO_MODE_THREAD_LOCAL for test code since that way nothing other than our internal functions should touch that errno value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, so the tradeoff here is "keeping using LIBC_ERRNO_MODE_THREAD_LOCAL as it makes tests more hermeticvs. "switching toLIBC_ERRNO_MODE_SYSTEM` since that's what the overlay production build uses."

I'm kind of ambivalent, and would defer to you and @lntue here, though I would note that attempted switch uncovered some of the problems that we'd better fix anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point. In general I'd prefer to test what we're really going to use, so I'd say go to LIBC_ERRNO_MODE_SYSTEM.

int fd = LIBC_NAMESPACE::epoll_create1(0);
ASSERT_GT(fd, 0);
ASSERT_ERRNO_SUCCESS();
Expand All @@ -23,6 +24,7 @@ TEST(LlvmLibcEpollCreate1Test, Basic) {
}

TEST(LlvmLibcEpollCreate1Test, CloseOnExecute) {
LIBC_NAMESPACE::libc_errno = 0;
int fd = LIBC_NAMESPACE::epoll_create1(EPOLL_CLOEXEC);
ASSERT_GT(fd, 0);
ASSERT_ERRNO_SUCCESS();
Expand Down
1 change: 1 addition & 0 deletions libc/test/src/sys/epoll/linux/epoll_create_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;

TEST(LlvmLibcEpollCreateTest, Basic) {
LIBC_NAMESPACE::libc_errno = 0;
int fd = LIBC_NAMESPACE::epoll_create(1);
ASSERT_GT(fd, 0);
ASSERT_ERRNO_SUCCESS();
Expand Down
1 change: 1 addition & 0 deletions libc/test/src/sys/epoll/linux/epoll_ctl_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;

TEST(LlvmLibcEpollCtlTest, Basic) {
LIBC_NAMESPACE::libc_errno = 0;
int epfd = LIBC_NAMESPACE::epoll_create1(0);
ASSERT_GT(epfd, 0);
ASSERT_ERRNO_SUCCESS();
Expand Down
1 change: 1 addition & 0 deletions libc/test/src/sys/epoll/linux/epoll_pwait2_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;

TEST(LlvmLibcEpollPwaitTest, Basic) {
LIBC_NAMESPACE::libc_errno = 0;
int epfd = LIBC_NAMESPACE::epoll_create1(0);
ASSERT_GT(epfd, 0);
ASSERT_ERRNO_SUCCESS();
Expand Down
1 change: 1 addition & 0 deletions libc/test/src/sys/epoll/linux/epoll_pwait_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;

TEST(LlvmLibcEpollPwaitTest, Basic) {
LIBC_NAMESPACE::libc_errno = 0;
int epfd = LIBC_NAMESPACE::epoll_create1(0);
ASSERT_GT(epfd, 0);
ASSERT_ERRNO_SUCCESS();
Expand Down
1 change: 1 addition & 0 deletions libc/test/src/sys/epoll/linux/epoll_wait_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;

TEST(LlvmLibcEpollWaitTest, Basic) {
LIBC_NAMESPACE::libc_errno = 0;
int epfd = LIBC_NAMESPACE::epoll_create1(0);
ASSERT_GT(epfd, 0);
ASSERT_ERRNO_SUCCESS();
Expand Down
12 changes: 7 additions & 5 deletions libc/test/src/sys/socket/linux/send_recv_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ TEST(LlvmLibcSendRecvTest, SucceedsWithSocketPair) {

int sockpair[2] = {0, 0};

LIBC_NAMESPACE::libc_errno = 0;
int result = LIBC_NAMESPACE::socketpair(AF_UNIX, SOCK_STREAM, 0, sockpair);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();

LIBC_NAMESPACE::libc_errno = 0;
ssize_t send_result =
LIBC_NAMESPACE::send(sockpair[0], TEST_MESSAGE, MESSAGE_LEN, 0);
EXPECT_EQ(send_result, static_cast<ssize_t>(MESSAGE_LEN));
ASSERT_ERRNO_SUCCESS();

LIBC_NAMESPACE::libc_errno = 0;
char buffer[256];

ssize_t recv_result =
LIBC_NAMESPACE::recv(sockpair[1], buffer, sizeof(buffer), 0);
ASSERT_EQ(recv_result, static_cast<ssize_t>(MESSAGE_LEN));
Expand All @@ -42,10 +44,12 @@ TEST(LlvmLibcSendRecvTest, SucceedsWithSocketPair) {
ASSERT_STREQ(buffer, TEST_MESSAGE);

// close both ends of the socket
LIBC_NAMESPACE::libc_errno = 0;
result = LIBC_NAMESPACE::close(sockpair[0]);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();

LIBC_NAMESPACE::libc_errno = 0;
result = LIBC_NAMESPACE::close(sockpair[1]);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();
Expand All @@ -55,19 +59,17 @@ TEST(LlvmLibcSendRecvTest, SendFails) {
const char TEST_MESSAGE[] = "connection terminated";
const size_t MESSAGE_LEN = sizeof(TEST_MESSAGE);

LIBC_NAMESPACE::libc_errno = 0;
ssize_t send_result = LIBC_NAMESPACE::send(-1, TEST_MESSAGE, MESSAGE_LEN, 0);
EXPECT_EQ(send_result, ssize_t(-1));
ASSERT_ERRNO_FAILURE();

LIBC_NAMESPACE::libc_errno = 0; // reset errno to avoid test ordering issues.
}

TEST(LlvmLibcSendRecvTest, RecvFails) {
char buffer[256];

LIBC_NAMESPACE::libc_errno = 0;
ssize_t recv_result = LIBC_NAMESPACE::recv(-1, buffer, sizeof(buffer), 0);
ASSERT_EQ(recv_result, ssize_t(-1));
ASSERT_ERRNO_FAILURE();

LIBC_NAMESPACE::libc_errno = 0; // reset errno to avoid test ordering issues.
}
11 changes: 7 additions & 4 deletions libc/test/src/sys/socket/linux/sendmsg_recvmsg_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ TEST(LlvmLibcSendMsgRecvMsgTest, SucceedsWithSocketPair) {

int sockpair[2] = {0, 0};

LIBC_NAMESPACE::libc_errno = 0;
int result = LIBC_NAMESPACE::socketpair(AF_UNIX, SOCK_STREAM, 0, sockpair);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();
Expand All @@ -41,6 +42,7 @@ TEST(LlvmLibcSendMsgRecvMsgTest, SucceedsWithSocketPair) {
send_message.msg_controllen = 0;
send_message.msg_flags = 0;

LIBC_NAMESPACE::libc_errno = 0;
ssize_t send_result = LIBC_NAMESPACE::sendmsg(sockpair[0], &send_message, 0);
EXPECT_EQ(send_result, static_cast<ssize_t>(MESSAGE_LEN));
ASSERT_ERRNO_SUCCESS();
Expand All @@ -60,17 +62,20 @@ TEST(LlvmLibcSendMsgRecvMsgTest, SucceedsWithSocketPair) {
recv_message.msg_controllen = 0;
recv_message.msg_flags = 0;

LIBC_NAMESPACE::libc_errno = 0;
ssize_t recv_result = LIBC_NAMESPACE::recvmsg(sockpair[1], &recv_message, 0);
ASSERT_EQ(recv_result, static_cast<ssize_t>(MESSAGE_LEN));
ASSERT_ERRNO_SUCCESS();

ASSERT_STREQ(buffer, TEST_MESSAGE);

// close both ends of the socket
LIBC_NAMESPACE::libc_errno = 0;
result = LIBC_NAMESPACE::close(sockpair[0]);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();

LIBC_NAMESPACE::libc_errno = 0;
result = LIBC_NAMESPACE::close(sockpair[1]);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();
Expand All @@ -94,11 +99,10 @@ TEST(LlvmLibcSendMsgRecvMsgTest, SendFails) {
send_message.msg_controllen = 0;
send_message.msg_flags = 0;

LIBC_NAMESPACE::libc_errno = 0;
ssize_t send_result = LIBC_NAMESPACE::sendmsg(-1, &send_message, 0);
EXPECT_EQ(send_result, ssize_t(-1));
ASSERT_ERRNO_FAILURE();

LIBC_NAMESPACE::libc_errno = 0; // reset errno to avoid test ordering issues.
}

TEST(LlvmLibcSendMsgRecvMsgTest, RecvFails) {
Expand All @@ -117,9 +121,8 @@ TEST(LlvmLibcSendMsgRecvMsgTest, RecvFails) {
recv_message.msg_controllen = 0;
recv_message.msg_flags = 0;

LIBC_NAMESPACE::libc_errno = 0;
ssize_t recv_result = LIBC_NAMESPACE::recvmsg(-1, &recv_message, 0);
ASSERT_EQ(recv_result, ssize_t(-1));
ASSERT_ERRNO_FAILURE();

LIBC_NAMESPACE::libc_errno = 0; // reset errno to avoid test ordering issues.
}
11 changes: 7 additions & 4 deletions libc/test/src/sys/socket/linux/sendto_recvfrom_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,20 @@ TEST(LlvmLibcSendToRecvFromTest, SucceedsWithSocketPair) {

int sockpair[2] = {0, 0};

LIBC_NAMESPACE::libc_errno = 0;
int result = LIBC_NAMESPACE::socketpair(AF_UNIX, SOCK_STREAM, 0, sockpair);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();

LIBC_NAMESPACE::libc_errno = 0;
ssize_t send_result = LIBC_NAMESPACE::sendto(sockpair[0], TEST_MESSAGE,
MESSAGE_LEN, 0, nullptr, 0);
EXPECT_EQ(send_result, static_cast<ssize_t>(MESSAGE_LEN));
ASSERT_ERRNO_SUCCESS();

char buffer[256];

LIBC_NAMESPACE::libc_errno = 0;
ssize_t recv_result = LIBC_NAMESPACE::recvfrom(sockpair[1], buffer,
sizeof(buffer), 0, nullptr, 0);
ASSERT_EQ(recv_result, static_cast<ssize_t>(MESSAGE_LEN));
Expand All @@ -42,10 +45,12 @@ TEST(LlvmLibcSendToRecvFromTest, SucceedsWithSocketPair) {
ASSERT_STREQ(buffer, TEST_MESSAGE);

// close both ends of the socket
LIBC_NAMESPACE::libc_errno = 0;
result = LIBC_NAMESPACE::close(sockpair[0]);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();

LIBC_NAMESPACE::libc_errno = 0;
result = LIBC_NAMESPACE::close(sockpair[1]);
ASSERT_EQ(result, 0);
ASSERT_ERRNO_SUCCESS();
Expand All @@ -55,21 +60,19 @@ TEST(LlvmLibcSendToRecvFromTest, SendToFails) {
const char TEST_MESSAGE[] = "connection terminated";
const size_t MESSAGE_LEN = sizeof(TEST_MESSAGE);

LIBC_NAMESPACE::libc_errno = 0;
ssize_t send_result =
LIBC_NAMESPACE::sendto(-1, TEST_MESSAGE, MESSAGE_LEN, 0, nullptr, 0);
EXPECT_EQ(send_result, ssize_t(-1));
ASSERT_ERRNO_FAILURE();

LIBC_NAMESPACE::libc_errno = 0; // reset errno to avoid test ordering issues.
}

TEST(LlvmLibcSendToRecvFromTest, RecvFromFails) {
char buffer[256];

LIBC_NAMESPACE::libc_errno = 0;
ssize_t recv_result =
LIBC_NAMESPACE::recvfrom(-1, buffer, sizeof(buffer), 0, nullptr, 0);
ASSERT_EQ(recv_result, ssize_t(-1));
ASSERT_ERRNO_FAILURE();

LIBC_NAMESPACE::libc_errno = 0; // reset errno to avoid test ordering issues.
}
1 change: 1 addition & 0 deletions libc/test/src/sys/socket/linux/socket_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <sys/socket.h> // For AF_UNIX and SOCK_DGRAM

TEST(LlvmLibcSocketTest, LocalSocket) {
LIBC_NAMESPACE::libc_errno = 0;
int sock = LIBC_NAMESPACE::socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_GE(sock, 0);
ASSERT_ERRNO_SUCCESS();
Expand Down
3 changes: 3 additions & 0 deletions libc/test/src/sys/socket/linux/socketpair_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <sys/socket.h> // For AF_UNIX and SOCK_DGRAM

TEST(LlvmLibcSocketPairTest, LocalSocket) {
LIBC_NAMESPACE::libc_errno = 0;
int sockpair[2] = {-1, -1};
int result = LIBC_NAMESPACE::socketpair(AF_UNIX, SOCK_DGRAM, 0, sockpair);
ASSERT_EQ(result, 0);
Expand All @@ -24,12 +25,14 @@ TEST(LlvmLibcSocketPairTest, LocalSocket) {
ASSERT_GE(sockpair[0], 0);
ASSERT_GE(sockpair[1], 0);

LIBC_NAMESPACE::libc_errno = 0;
LIBC_NAMESPACE::close(sockpair[0]);
LIBC_NAMESPACE::close(sockpair[1]);
ASSERT_ERRNO_SUCCESS();
}

TEST(LlvmLibcSocketPairTest, SocketFails) {
LIBC_NAMESPACE::libc_errno = 0;
int sockpair[2] = {-1, -1};
int result = LIBC_NAMESPACE::socketpair(-1, -1, -1, sockpair);
ASSERT_EQ(result, -1);
Expand Down
3 changes: 3 additions & 0 deletions libc/test/src/unistd/pread_pwrite_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ TEST(LlvmLibcUniStd, PWriteAndPReadBackTest) {

constexpr const char *FILENAME = "pread_pwrite.test";
auto TEST_FILE = libc_make_test_file_path(FILENAME);
LIBC_NAMESPACE::libc_errno = 0;
int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(fd, 0);
Expand All @@ -42,6 +43,7 @@ TEST(LlvmLibcUniStd, PWriteAndPReadBackTest) {
ASSERT_THAT(LIBC_NAMESPACE::fsync(fd), Succeeds(0));
ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));

LIBC_NAMESPACE::libc_errno = 0;
fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(fd, 0);
Expand All @@ -50,6 +52,7 @@ TEST(LlvmLibcUniStd, PWriteAndPReadBackTest) {
ASSERT_THAT(LIBC_NAMESPACE::fsync(fd), Succeeds(0));
ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));

LIBC_NAMESPACE::libc_errno = 0;
fd = LIBC_NAMESPACE::open(TEST_FILE, O_RDONLY);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(fd, 0);
Expand Down
2 changes: 2 additions & 0 deletions libc/test/src/unistd/read_write_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ TEST(LlvmLibcUniStd, WriteAndReadBackTest) {
constexpr const char *FILENAME = "__unistd_read_write.test";
auto TEST_FILE = libc_make_test_file_path(FILENAME);

LIBC_NAMESPACE::libc_errno = 0;
int write_fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(write_fd, 0);
Expand All @@ -33,6 +34,7 @@ TEST(LlvmLibcUniStd, WriteAndReadBackTest) {
ASSERT_THAT(LIBC_NAMESPACE::fsync(write_fd), Succeeds(0));
ASSERT_THAT(LIBC_NAMESPACE::close(write_fd), Succeeds(0));

LIBC_NAMESPACE::libc_errno = 0;
int read_fd = LIBC_NAMESPACE::open(TEST_FILE, O_RDONLY);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(read_fd, 0);
Expand Down
1 change: 1 addition & 0 deletions libc/test/src/unistd/unlink_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ TEST(LlvmLibcUnlinkTest, CreateAndUnlink) {
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
constexpr const char *FILENAME = "unlink.test";
auto TEST_FILE = libc_make_test_file_path(FILENAME);
LIBC_NAMESPACE::libc_errno = 0;
int write_fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(write_fd, 0);
Expand Down
Loading