Try reading a smaller chunk when (p)read fails

[JDevlieghere]

Summary

On macOS, the read and pread calls return EINVAL when the number of bytes exceeds INT32_MAX: https://github.com/apple/darwin-xnu/blob/a449c6a3b8014d9406c2ddbdc81795da24aa7443/bsd/kern/sys_generic.c#L355

I found out when passing a 2.5G Mach-O to llvm-dwarfdump. Normally we would mmap the file, but this particular file happens to be the exact multiple of the page size (2628505600 bytes) in which case llvm prefers not to mmap (as to not waste a whole page for the terminating NULL byte).

rdar://68751407

[JDevlieghere]

Seems like the issue might be specific to macOS (with a fixed max value of INT32_MAX).

[thegameg]

llvm/lib/Support/Unix/Path.inc

1079

= std::min(Buf.size(), INT32_MAX) ?

[thegameg]

llvm/lib/Support/Unix/Path.inc

1068

Actually, is this also affected?

[JDevlieghere]

Big thanks to Francis who discovered the max value is INT32_MAX. I had only tried UIN32_MAX.

[labath]

There's also a similar piece of code in raw_ostream.cpp (raw_fd_ostream::write_impl), only it hard-limits the size to 2G everywhere, and additionally to 1G on linux. It would probably be a good idea to introduce a helper function for this functionality.

this particular file happens to be the exact multiple of the page size (2628505600 bytes) in which case llvm prefers not to mmap (as to not waste a whole page for the terminating NULL byte).

Do we really need the terminating nul byte? It should be possible to avoid this behavior by passing RequiresNullTerminator=false to the relevant MemoryBuffer method...

[rnk]

... in which case llvm prefers not to mmap (as to not waste a whole page for the terminating NULL byte)

We should really re-evaluate this behavior at some point. It is quite surprising.

llvm/lib/Support/Unix/Path.inc

1069

You know, reading maximum 2GB size chunks seems like reasonable behavior for any OS. We do it for Windows too:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Windows/Path.inc#L1235

IMO we should be brave, remove the ifdef, and do 2GB reads on all OSs.

[efriedma]

In D90201#2357105, @rnk wrote:

... in which case llvm prefers not to mmap (as to not waste a whole page for the terminating NULL byte)

We should really re-evaluate this behavior at some point. It is quite surprising.

I think the problem is that if the filesize is exactly a multiple of the page size, there isn't any simple way to allocate a null at the end of the file. Certain uses (for example, clang's lexer) require null termination. Most uses probably shouldn't be requesting null termination.

[joerg]

You can do an anon-mapping with size+pagesize in that case and map over it with MAP_FIXED.

llvm/lib/Support/Unix/Path.inc

     return errorCodeToError(EC);
   return ResultFD;
 }
 
 file_t getStdinHandle() { return 0; }
 file_t getStdoutHandle() { return 1; }
 file_t getStderrHandle() { return 2; }
 
 Expected<size_t> readNativeFile(file_t FD, MutableArrayRef<char> Buf) {
+#if defined(__APPLE__)
+  size_t Size = std::min<size_t>(Buf.size(), INT32_MAX);
+#else
+  size_t Size = Buf.size();
+#endif
   ssize_t NumRead =
-      sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Buf.size());
+      sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Size);
   if (ssize_t(NumRead) == -1)
     return errorCodeToError(std::error_code(errno, std::generic_category()));
   return NumRead;
 }
 
 Expected<size_t> readNativeFileSlice(file_t FD, MutableArrayRef<char> Buf,
                                      uint64_t Offset) {
+#if defined(__APPLE__)
+  size_t Size = std::min<size_t>(Buf.size(), INT32_MAX);
+#else
+  size_t Size = Buf.size();
+#endif
 #ifdef HAVE_PREAD
   ssize_t NumRead =
-      sys::RetryAfterSignal(-1, ::pread, FD, Buf.data(), Buf.size(), Offset);
+      sys::RetryAfterSignal(-1, ::pread, FD, Buf.data(), Size, Offset);
 #else
   if (lseek(FD, Offset, SEEK_SET) == -1)
     return errorCodeToError(std::error_code(errno, std::generic_category()));
   ssize_t NumRead =
-      sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Buf.size());
+      sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Size);
 #endif
   if (NumRead == -1)
     return errorCodeToError(std::error_code(errno, std::generic_category()));
   return NumRead;
 }
 
 std::error_code tryLockFile(int FD, std::chrono::milliseconds Timeout) {
   auto Start = std::chrono::steady_clock::now();