Skip to content
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

[libc][NFC] Adjust use of off_t internally #68269

Merged
merged 1 commit into from
Jul 8, 2024
Merged
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
12 changes: 5 additions & 7 deletions libc/src/__support/File/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,23 +305,21 @@ ErrorOr<int> File::seek(long offset, int whence) {
auto result = platform_seek(this, offset, whence);
if (!result.has_value())
return Error(result.error());
else
return 0;
return 0;
}

ErrorOr<long> File::tell() {
ErrorOr<off_t> File::tell() {
FileLock lock(this);
auto seek_target = eof ? SEEK_END : SEEK_CUR;
auto result = platform_seek(this, 0, seek_target);
if (!result.has_value() || result.value() < 0)
return Error(result.error());
long platform_offset = result.value();
off_t platform_offset = result.value();
if (prev_op == FileOp::READ)
return platform_offset - (read_limit - pos);
else if (prev_op == FileOp::WRITE)
if (prev_op == FileOp::WRITE)
return platform_offset + pos;
else
return platform_offset;
return platform_offset;
}

int File::flush_unlocked() {
Expand Down
5 changes: 3 additions & 2 deletions libc/src/__support/File/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <stddef.h>
#include <stdint.h>
#include <unistd.h> // For off_t.

namespace LIBC_NAMESPACE {

Expand Down Expand Up @@ -45,7 +46,7 @@ class File {
using ReadFunc = FileIOResult(File *, void *, size_t);
// The SeekFunc is expected to return the current offset of the external
// file position indicator.
using SeekFunc = ErrorOr<long>(File *, long, int);
using SeekFunc = ErrorOr<off_t>(File *, off_t, int);
using CloseFunc = int(File *);

using ModeFlags = uint32_t;
Expand Down Expand Up @@ -182,7 +183,7 @@ class File {

ErrorOr<int> seek(long offset, int whence);

ErrorOr<long> tell();
ErrorOr<off_t> tell();

// If buffer has data written to it, flush it out. Does nothing if the
// buffer is currently being used as a read buffer.
Expand Down
2 changes: 1 addition & 1 deletion libc/src/__support/File/linux/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ FileIOResult linux_file_read(File *f, void *buf, size_t size) {
return ret;
}

ErrorOr<long> linux_file_seek(File *f, long offset, int whence) {
ErrorOr<off_t> linux_file_seek(File *f, off_t offset, int whence) {
auto *lf = reinterpret_cast<LinuxFile *>(f);
auto result = internal::lseekimpl(lf->get_fd(), offset, whence);
if (!result.has_value())
Expand Down
2 changes: 1 addition & 1 deletion libc/src/__support/File/linux/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace LIBC_NAMESPACE {

FileIOResult linux_file_write(File *, const void *, size_t);
FileIOResult linux_file_read(File *, void *, size_t);
ErrorOr<long> linux_file_seek(File *, long, int);
ErrorOr<off_t> linux_file_seek(File *, off_t, int);
int linux_file_close(File *);

class LinuxFile : public File {
Expand Down
7 changes: 3 additions & 4 deletions libc/src/stdio/fopencookie.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class CookieFile : public LIBC_NAMESPACE::File {

static FileIOResult cookie_write(File *f, const void *data, size_t size);
static FileIOResult cookie_read(File *f, void *data, size_t size);
static ErrorOr<long> cookie_seek(File *f, long offset, int whence);
static ErrorOr<off_t> cookie_seek(File *f, off_t offset, int whence);
static int cookie_close(File *f);

public:
Expand Down Expand Up @@ -52,7 +52,7 @@ FileIOResult CookieFile::cookie_read(File *f, void *data, size_t size) {
reinterpret_cast<char *>(data), size);
}

ErrorOr<long> CookieFile::cookie_seek(File *f, long offset, int whence) {
ErrorOr<off_t> CookieFile::cookie_seek(File *f, off_t offset, int whence) {
auto cookie_file = reinterpret_cast<CookieFile *>(f);
if (cookie_file->ops.seek == nullptr) {
return Error(EINVAL);
Expand All @@ -61,8 +61,7 @@ ErrorOr<long> CookieFile::cookie_seek(File *f, long offset, int whence) {
int result = cookie_file->ops.seek(cookie_file->cookie, &offset64, whence);
if (result == 0)
return offset64;
else
return -1;
return -1;
}

int CookieFile::cookie_close(File *f) {
Expand Down
6 changes: 5 additions & 1 deletion libc/src/stdio/generic/ftell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ LLVM_LIBC_FUNCTION(long, ftell, (::FILE * stream)) {
libc_errno = result.error();
return -1;
}
return result.value();
// tell() returns an off_t (64-bit signed integer), but this function returns
// a long (32-bit signed integer in 32-bit systems). We add a cast here to
// silence a "implicit conversion loses integer precision" warning when
// compiling for 32-bit systems.
return static_cast<long>(result.value());
}

} // namespace LIBC_NAMESPACE
5 changes: 4 additions & 1 deletion libc/src/sys/mman/linux/mmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ LLVM_LIBC_FUNCTION(void *, mmap,
#error "mmap or mmap2 syscalls not available."
#endif

// We add an explicit cast to silence a "implicit conversion loses integer
// precision" warning when compiling for 32-bit systems.
long mmap_offset = static_cast<long>(offset);
long ret =
LIBC_NAMESPACE::syscall_impl(syscall_number, reinterpret_cast<long>(addr),
size, prot, flags, fd, offset);
size, prot, flags, fd, mmap_offset);

// The mmap/mmap2 syscalls return negative values on error. These negative
// values are actually the negative values of the error codes. So, fix them
Expand Down
24 changes: 15 additions & 9 deletions libc/src/unistd/linux/pread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,21 @@ namespace LIBC_NAMESPACE {

LLVM_LIBC_FUNCTION(ssize_t, pread,
(int fd, void *buf, size_t count, off_t offset)) {
#ifdef LIBC_TARGET_ARCH_IS_RISCV32
static_assert(sizeof(off_t) == 8);
ssize_t ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(
SYS_pread64, fd, buf, count, (long)offset,
(long)(((uint64_t)(offset)) >> 32));
#else
ssize_t ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(SYS_pread64, fd, buf,
count, offset);
#endif
ssize_t ret;
if constexpr (sizeof(long) == sizeof(uint32_t) &&
sizeof(off_t) == sizeof(uint64_t)) {
// This is a 32-bit system with a 64-bit offset, offset must be split.
const uint64_t bits = cpp::bit_cast<uint64_t>(offset);
const uint32_t lo = bits & UINT32_MAX;
const uint32_t hi = bits >> 32;
const long offset_low = cpp::bit_cast<long>(static_cast<long>(lo));
const long offset_high = cpp::bit_cast<long>(static_cast<long>(hi));
ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(SYS_pread64, fd, buf, count,
offset_low, offset_high);
} else {
ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(SYS_pread64, fd, buf, count,
offset);
}
// The cast is important since there is a check that dereferences the pointer
// which fails on void*.
MSAN_UNPOISON(reinterpret_cast<char *>(buf), count);
Expand Down
26 changes: 17 additions & 9 deletions libc/src/unistd/linux/pwrite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,23 @@ namespace LIBC_NAMESPACE {

LLVM_LIBC_FUNCTION(ssize_t, pwrite,
(int fd, const void *buf, size_t count, off_t offset)) {
#ifdef LIBC_TARGET_ARCH_IS_RISCV32
static_assert(sizeof(off_t) == 8);
ssize_t ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(
SYS_pwrite64, fd, buf, count, (long)offset,
(long)(((uint64_t)(offset)) >> 32));
#else
ssize_t ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(SYS_pwrite64, fd, buf,
count, offset);
#endif

ssize_t ret;
if constexpr (sizeof(long) == sizeof(uint32_t) &&
sizeof(off_t) == sizeof(uint64_t)) {
// This is a 32-bit system with a 64-bit offset, offset must be split.
const uint64_t bits = cpp::bit_cast<uint64_t>(offset);
const uint32_t lo = bits & UINT32_MAX;
const uint32_t hi = bits >> 32;
const long offset_low = cpp::bit_cast<long>(static_cast<long>(lo));
const long offset_high = cpp::bit_cast<long>(static_cast<long>(hi));
ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(SYS_pwrite64, fd, buf, count,
offset_low, offset_high);
} else {
ret = LIBC_NAMESPACE::syscall_impl<ssize_t>(SYS_pwrite64, fd, buf, count,
offset);
}

if (ret < 0) {
libc_errno = static_cast<int>(-ret);
return -1;
Expand Down
8 changes: 4 additions & 4 deletions libc/test/src/__support/File/file_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ class StringFile : public File {
static FileIOResult str_read(LIBC_NAMESPACE::File *f, void *data, size_t len);
static FileIOResult str_write(LIBC_NAMESPACE::File *f, const void *data,
size_t len);
static ErrorOr<long> str_seek(LIBC_NAMESPACE::File *f, long offset,
int whence);
static ErrorOr<off_t> str_seek(LIBC_NAMESPACE::File *f, off_t offset,
int whence);
static int str_close(LIBC_NAMESPACE::File *f) {
delete reinterpret_cast<StringFile *>(f);
return 0;
Expand Down Expand Up @@ -93,8 +93,8 @@ FileIOResult StringFile::str_write(LIBC_NAMESPACE::File *f, const void *data,
return i;
}

ErrorOr<long> StringFile::str_seek(LIBC_NAMESPACE::File *f, long offset,
int whence) {
ErrorOr<off_t> StringFile::str_seek(LIBC_NAMESPACE::File *f, off_t offset,
int whence) {
StringFile *sf = static_cast<StringFile *>(f);
if (whence == SEEK_SET)
sf->pos = offset;
Expand Down
Loading