[libc-commits] [libc] [libc][NFC] Adjust use of off_t internally (PR #68269)

[Guillaume Chatelet via libc-commits]

================
@@ -19,15 +19,18 @@ 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(off_t) == 8 && sizeof(size_t) == 4) {
+    // This is a 32-bit system with a 64-bit offset.
+    long offset_low = static_cast<long>(offset);
+    long offset_high = static_cast<long>(offset >> 32);
----------------
gchatelet wrote:

> This checks only if we are running in a 32-bit system.

AFAICT what you want to check here is whether `sizeof(off_t) == sizeof(long)` or `sizeof(off_t) == (sizeof(long) * 2)`, You may want to `error` in the case where none of these are true. Whether `sizeof(size_t) == 4` is related to this or not is not entirely clear to me.

> The cast will assign the lowest bits of offset to offset_low...

This is *implementation defined* as well.

The code below would work:
```
// This is a 32-bit system with a 64-bit offset.
static_assert(sizeof(off_t) == sizeof(uint64_t));
static_assert(sizeof(long) == sizeof(uint32_t));
const uint64_t bits = cpp::bit_cast<uint64_t>(offset); // as unsigned
const uint32_t lo = bits & UINT32_MAX;
const uint32_t hi = bits >> 32;
const long offset_low = cpp::bit_cast<long>(lo); // as signed
const long offset_hi = cpp::bit_cast<long>(hi); // as signed
```

 - `cpp::bit_cast` ensures that types have the same width and prevent precision loss.
 - Shift and logical operations are done on unsigned types and are well defined.
 - The intent is also much clearer.

This can be implemented in a zero cost abstraction that is type-safe and well defined.
https://godbolt.org/z/TjoacYnKW

Note that it fails with a clear message if types are not correct
https://godbolt.org/z/94v5cecE3

https://github.com/llvm/llvm-project/pull/68269