[libc][NFC] Adjust use of off_t internally (llvm#68269)

This patch includes changes related to the use of off_t in libc,
targeted at 32-bit systems: in several places, the offset is used either
as a long or an off_t (64-bit signed int), but in 32-bit systems a long
type is only 32 bits long.

Fix a warning in mmap where a long offset is expected, but we were
passing an off_t. A static_cast and a comment were added to explain
that we know we are ignoring the upper 32-bit of the off_t in 32-bit
systems.
The code in pread and pwrite was slightly improved to remove a
#ifdef LIBC_TARGET_ARCH_IS_RISCV32; we are using an if constexpr now.
The Linux file operations were changed to use off_t instead of a long
where applicable. No changes were made to the standard API, e.g.,
ftell returns the offset as an int so we added a static_cast and a
comment explaining that this will cause a loss of integer precision
in 32-bit systems.

Author: mikhailramalho

libc/src/__support/File/file.cpp

@@ -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() {

libc/src/__support/File/file.h

@@ -16,6 +16,7 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <unistd.h> // For off_t.
 
 namespace LIBC_NAMESPACE {
 
@@ -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;
@@ -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.

libc/src/__support/File/linux/file.cpp

@@ -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())

libc/src/__support/File/linux/file.h

@@ -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 {

libc/src/stdio/fopencookie.cpp

@@ -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:
@@ -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);
@@ -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) {

libc/src/stdio/generic/ftell.cpp

@@ -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

libc/src/sys/mman/linux/mmap.cpp

@@ -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

libc/src/unistd/linux/pread.cpp

@@ -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);

libc/src/unistd/linux/pwrite.cpp

@@ -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;

libc/test/src/__support/File/file_test.cpp

@@ -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;
@@ -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;