From 40a8a2b50dcf147c2623c71c9c925143f45c399c Mon Sep 17 00:00:00 2001 From: Jackson Stogel Date: Wed, 5 Nov 2025 23:51:31 +0000 Subject: [PATCH 1/4] Reapply "[libc] Return errno from OFD failure paths in fcntl." (#166658) This reverts commit 597cd767d6ad2cca0d8676888c40cbc5700db1ca. --- libc/src/__support/OSUtil/linux/fcntl.cpp | 2 +- libc/test/src/fcntl/fcntl_test.cpp | 165 ++++++++++++---------- 2 files changed, 94 insertions(+), 73 deletions(-) diff --git a/libc/src/__support/OSUtil/linux/fcntl.cpp b/libc/src/__support/OSUtil/linux/fcntl.cpp index bb76eee90efd2..08db4859c6417 100644 --- a/libc/src/__support/OSUtil/linux/fcntl.cpp +++ b/libc/src/__support/OSUtil/linux/fcntl.cpp @@ -66,7 +66,7 @@ ErrorOr fcntl(int fd, int cmd, void *arg) { LIBC_NAMESPACE::syscall_impl(FCNTL_SYSCALL_ID, fd, cmd, &flk64); // On failure, return if (ret < 0) - return Error(-1); + return Error(-ret); // Check for overflow, i.e. the offsets are not the same when cast // to off_t from off64_t. if (static_cast(flk64.l_len) != flk64.l_len || diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp index 84feb34e537a0..d6aaccddd76ee 100644 --- a/libc/test/src/fcntl/fcntl_test.cpp +++ b/libc/test/src/fcntl/fcntl_test.cpp @@ -94,78 +94,99 @@ TEST_F(LlvmLibcFcntlTest, FcntlSetFl) { ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); } -TEST_F(LlvmLibcFcntlTest, FcntlGetLkRead) { - using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; - constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkread.test"; - auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME); - - struct flock flk, svflk; - int retVal; - int fd = - LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY, S_IRWXU); - ASSERT_ERRNO_SUCCESS(); - ASSERT_GT(fd, 0); - - flk.l_type = F_RDLCK; - flk.l_start = 0; - flk.l_whence = SEEK_SET; - flk.l_len = 50; - - // copy flk into svflk - svflk = flk; - - retVal = LIBC_NAMESPACE::fcntl(fd, F_GETLK, &svflk); - ASSERT_ERRNO_SUCCESS(); - ASSERT_GT(retVal, -1); - ASSERT_NE((int)flk.l_type, F_WRLCK); // File should not be write locked. - - retVal = LIBC_NAMESPACE::fcntl(fd, F_SETLK, &svflk); - ASSERT_ERRNO_SUCCESS(); - ASSERT_GT(retVal, -1); - - ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); -} - -TEST_F(LlvmLibcFcntlTest, FcntlGetLkWrite) { - using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; - constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkwrite.test"; - auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME); - - struct flock flk, svflk; - int retVal; - int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU); - ASSERT_ERRNO_SUCCESS(); - ASSERT_GT(fd, 0); - - flk.l_type = F_WRLCK; - flk.l_start = 0; - flk.l_whence = SEEK_SET; - flk.l_len = 0; - - // copy flk into svflk - svflk = flk; - - retVal = LIBC_NAMESPACE::fcntl(fd, F_GETLK, &svflk); - ASSERT_ERRNO_SUCCESS(); - ASSERT_GT(retVal, -1); - ASSERT_NE((int)flk.l_type, F_RDLCK); // File should not be read locked. - - retVal = LIBC_NAMESPACE::fcntl(fd, F_SETLK, &svflk); - ASSERT_ERRNO_SUCCESS(); - ASSERT_GT(retVal, -1); - - ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); -} - -TEST_F(LlvmLibcFcntlTest, UseAfterClose) { - using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; - constexpr const char *TEST_FILE_NAME = "testdata/fcntl_use_after_close.test"; - auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME); - int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU); - ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); - ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, F_GETFL)); - ASSERT_ERRNO_EQ(EBADF); -} +/* Tests that are common between OFD and traditional variants of fcntl locks. */ +template +class LibcFcntlCommonLockTests : public LlvmLibcFcntlTest { +public: + void GetLkRead() { + using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; + constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkread.test"; + const auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME); + + struct flock flk = {}; + struct flock svflk = {}; + int retVal; + int fd = + LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY, S_IRWXU); + ASSERT_ERRNO_SUCCESS(); + ASSERT_GT(fd, 0); + + flk.l_type = F_RDLCK; + flk.l_start = 0; + flk.l_whence = SEEK_SET; + flk.l_len = 50; + + // copy flk into svflk + svflk = flk; + + retVal = LIBC_NAMESPACE::fcntl(fd, GETLK_CMD, &svflk); + ASSERT_ERRNO_SUCCESS(); + ASSERT_GT(retVal, -1); + ASSERT_NE((int)svflk.l_type, F_WRLCK); // File should not be write locked. + + retVal = LIBC_NAMESPACE::fcntl(fd, SETLK_CMD, &svflk); + ASSERT_ERRNO_SUCCESS(); + ASSERT_GT(retVal, -1); + + ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); + } + + void GetLkWrite() { + using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; + constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkwrite.test"; + const auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME); + + struct flock flk = {}; + struct flock svflk = {}; + int retVal; + int fd = + LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU); + ASSERT_ERRNO_SUCCESS(); + ASSERT_GT(fd, 0); + + flk.l_type = F_WRLCK; + flk.l_start = 0; + flk.l_whence = SEEK_SET; + flk.l_len = 0; + + // copy flk into svflk + svflk = flk; + + retVal = LIBC_NAMESPACE::fcntl(fd, GETLK_CMD, &svflk); + ASSERT_ERRNO_SUCCESS(); + ASSERT_GT(retVal, -1); + ASSERT_NE((int)svflk.l_type, F_RDLCK); // File should not be read locked. + + retVal = LIBC_NAMESPACE::fcntl(fd, SETLK_CMD, &svflk); + ASSERT_ERRNO_SUCCESS(); + ASSERT_GT(retVal, -1); + + ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); + } + + void UseAfterClose() { + using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; + constexpr const char *TEST_FILE_NAME = + "testdata/fcntl_use_after_close.test"; + const auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME); + int fd = + LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU); + ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); + ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, GETLK_CMD)); + ASSERT_ERRNO_EQ(EBADF); + } +}; + +#define COMMON_LOCK_TESTS(NAME, GETLK, SETLK) \ + using NAME = LibcFcntlCommonLockTests; \ + TEST_F(NAME, GetLkRead) { GetLkRead(); } \ + TEST_F(NAME, GetLkWrite) { GetLkWrite(); } \ + TEST_F(NAME, UseAfterClose) { UseAfterClose(); } \ + static_assert(true, "Require semicolon.") + +COMMON_LOCK_TESTS(LlvmLibcFcntlProcessAssociatedLockTest, F_GETLK, F_SETLK); +COMMON_LOCK_TESTS(LlvmLibcFcntlOpenFileDescriptionLockTest, F_OFD_GETLK, + F_OFD_SETLK); TEST_F(LlvmLibcFcntlTest, SetGetOwnerTest) { using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; From 86f4811624f3b8e278c958ed4b60ed1948ec3157 Mon Sep 17 00:00:00 2001 From: Jackson Stogel Date: Thu, 6 Nov 2025 01:41:22 +0000 Subject: [PATCH 2/4] Pass a struct flock into fcntl F_OFD_GETLK. --- libc/test/src/fcntl/fcntl_test.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp index d6aaccddd76ee..41a9893d4d902 100644 --- a/libc/test/src/fcntl/fcntl_test.cpp +++ b/libc/test/src/fcntl/fcntl_test.cpp @@ -172,13 +172,19 @@ class LibcFcntlCommonLockTests : public LlvmLibcFcntlTest { int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU); ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); - ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, GETLK_CMD)); + + struct flock flk = {}; + flk.l_type = F_RDLCK; + flk.l_start = 0; + flk.l_whence = SEEK_SET; + flk.l_len = 50; + ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, GETLK_CMD, &flk)); ASSERT_ERRNO_EQ(EBADF); } }; -#define COMMON_LOCK_TESTS(NAME, GETLK, SETLK) \ - using NAME = LibcFcntlCommonLockTests; \ +#define COMMON_LOCK_TESTS(NAME, GETLK_CMD, SETLK_CMD) \ + using NAME = LibcFcntlCommonLockTests; \ TEST_F(NAME, GetLkRead) { GetLkRead(); } \ TEST_F(NAME, GetLkWrite) { GetLkWrite(); } \ TEST_F(NAME, UseAfterClose) { UseAfterClose(); } \ @@ -188,6 +194,16 @@ COMMON_LOCK_TESTS(LlvmLibcFcntlProcessAssociatedLockTest, F_GETLK, F_SETLK); COMMON_LOCK_TESTS(LlvmLibcFcntlOpenFileDescriptionLockTest, F_OFD_GETLK, F_OFD_SETLK); +TEST_F(LlvmLibcFcntlTest, UseAfterClose) { + using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; + constexpr const char *TEST_FILE_NAME = "testdata/fcntl_use_after_close.test"; + auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME); + int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU); + ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); + ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, F_GETFL)); + ASSERT_ERRNO_EQ(EBADF); +} + TEST_F(LlvmLibcFcntlTest, SetGetOwnerTest) { using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds; pid_t pid = LIBC_NAMESPACE::getpid(); From af9df0c654a74a30b6be3b2177a68675c496d366 Mon Sep 17 00:00:00 2001 From: Jackson Stogel Date: Thu, 6 Nov 2025 21:32:34 +0000 Subject: [PATCH 3/4] Fix typo. --- libc/test/src/fcntl/fcntl_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp index 41a9893d4d902..6e789b72cf637 100644 --- a/libc/test/src/fcntl/fcntl_test.cpp +++ b/libc/test/src/fcntl/fcntl_test.cpp @@ -184,7 +184,7 @@ class LibcFcntlCommonLockTests : public LlvmLibcFcntlTest { }; #define COMMON_LOCK_TESTS(NAME, GETLK_CMD, SETLK_CMD) \ - using NAME = LibcFcntlCommonLockTests; \ + using NAME = LibcFcntlCommonLockTests; \ TEST_F(NAME, GetLkRead) { GetLkRead(); } \ TEST_F(NAME, GetLkWrite) { GetLkWrite(); } \ TEST_F(NAME, UseAfterClose) { UseAfterClose(); } \ From ca1a3e92323107b1dfdcf67c028e5fe415f70c39 Mon Sep 17 00:00:00 2001 From: Jackson Stogel Date: Mon, 10 Nov 2025 21:04:51 +0000 Subject: [PATCH 4/4] Remove unnecessary struct qualifier. --- libc/test/src/fcntl/fcntl_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp index 6e789b72cf637..d008aea54b425 100644 --- a/libc/test/src/fcntl/fcntl_test.cpp +++ b/libc/test/src/fcntl/fcntl_test.cpp @@ -173,7 +173,7 @@ class LibcFcntlCommonLockTests : public LlvmLibcFcntlTest { LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU); ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0)); - struct flock flk = {}; + flock flk = {}; flk.l_type = F_RDLCK; flk.l_start = 0; flk.l_whence = SEEK_SET;