Cold-Air-Networks/mirror-llvm-project
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 7

Reapply "[libc] Return errno from OFD failure paths in fcntl." (#166658) (#166846)

Browse source 
The previous implementation in #166252 (rolled back in #166658) caused
buildbot failures due to a bug in an added test. The modified
`UseAfterClose` did not pass a `struct flock` value to `fcntl`:


2d51705941/libc/test/src/fcntl/fcntl_test.cpp (L175)

Which ASAN caught and errored in the `fcntl` implementation when the
unspecified argument was accessed:


c12cb2892c/libc/src/__support/OSUtil/linux/fcntl.cpp (L59)
This commit is contained in:
Jackson Stogel 2025-11-10 13:39:34 -08:00 committed by GitHub
parent fb2fa21bd6
commit 4b9d7e167b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 87 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
libc/src/__support/OSUtil/linux/fcntl.cpp
View file

@ -66,7 +66,7 @@ ErrorOr<int> fcntl(int fd, int cmd, void *arg) {
LIBC_NAMESPACE::syscall_impl<int>(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<off_t>(flk64.l_len) != flk64.l_len ||

135
libc/test/src/fcntl/fcntl_test.cpp
View file

@ -94,68 +94,105 @@ 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);
/* Tests that are common between OFD and traditional variants of fcntl locks. */
template <int GETLK_CMD, int SETLK_CMD>
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, 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);
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;
flk.l_type = F_RDLCK;
flk.l_start = 0;
flk.l_whence = SEEK_SET;
flk.l_len = 50;
// copy flk into svflk
svflk = flk;
// 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, 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, F_SETLK, &svflk);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(retVal, -1);
retVal = LIBC_NAMESPACE::fcntl(fd, SETLK_CMD, &svflk);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(retVal, -1);
ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
}
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);
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, 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);
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;
flk.l_type = F_WRLCK;
flk.l_start = 0;
flk.l_whence = SEEK_SET;
flk.l_len = 0;
// copy flk into svflk
svflk = flk;
// 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, 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, F_SETLK, &svflk);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(retVal, -1);
retVal = LIBC_NAMESPACE::fcntl(fd, SETLK_CMD, &svflk);
ASSERT_ERRNO_SUCCESS();
ASSERT_GT(retVal, -1);
ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
}
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));
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_CMD, SETLK_CMD) \
using NAME = LibcFcntlCommonLockTests<GETLK_CMD, SETLK_CMD>; \
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, UseAfterClose) {
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;