[libc][mmap] force offset to long for mmap2

[nickdesaulniers]

llvm-libc's "Large File Support" isn't really complete. We define off_t to be
a uint64_t always, which breaks with convention in an attempt to drop
historical baggage. We might need to revisit that decision in the future, but
for now, force off_t to long when using mmap2.

This avoids having to pass a 64b value to our syscall wrappers for 32b targets,
which we do not support on arm32. Since we're using mmap2 rather than mmap, we
don't need a 64b value anyways.

Our mmap implementation still has further room for improvement...

[llvmbot]

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (paternity leave) (nickdesaulniers)

Changes

llvm-libc's "Large File Support" isn't really complete. We define off_t to be
a uint64_t always, which breaks with convention in an attempt to drop
historical baggage. We might need to revisit that decision in the future, but
for now, force off_t to long when using mmap2.

This avoids having to pass a 64b value to our syscall wrappers for 32b targets,
which we do not support on arm32. Since we're using mmap2 rather than mmap, we
don't need a 64b value anyways.

Our mmap implementation still has further room for improvement...

---

Full diff: https://github.com/llvm/llvm-project/pull/96522.diff

1 Files Affected:

• (modified) libc/src/sys/mman/linux/mmap.cpp (+2-1)

diff --git a/libc/src/sys/mman/linux/mmap.cpp b/libc/src/sys/mman/linux/mmap.cpp
index 16111c66859f5..e280ac42092a8 100644
--- a/libc/src/sys/mman/linux/mmap.cpp
+++ b/libc/src/sys/mman/linux/mmap.cpp
@@ -41,7 +41,8 @@ LLVM_LIBC_FUNCTION(void *, mmap,
 
   long ret =
       LIBC_NAMESPACE::syscall_impl(syscall_number, reinterpret_cast<long>(addr),
-                                   size, prot, flags, fd, offset);
+                                   size, prot, flags, fd,
+                                   static_cast<long>(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

[enh-google]

llvm-libc's "Large File Support" isn't really complete. We define off_t to be
a uint64_t always,

an int64_t, i assume :-)

which breaks with convention in an attempt to drop

eh, iOS/macOS would say "whose convention?". it's only really a historical accident that bionic has different off_t/off64_t in the ILP32 ABI. in 2024, having two off_t types is deep into "historical cruft" territory.

force off_t to long when using mmap2

this sounds worse than what the actual change does --- you're actually only truncating offset >> 12, not the offset.

but why not just do the right thing? it's only a couple of lines. see bionic:

// mmap2(2) is like mmap(2), but the offset is in 4096-byte blocks (regardless
// of page size), not bytes, to enable mapping parts of large files past the
// 4GiB limit but without the inconvenience of dealing with 64-bit values, with
// no down side since mappings need to be page aligned anyway, and the 32-bit
// architectures that support this system call all have 4KiB pages.
extern "C" void* __mmap2(void*, size_t, int, int, int, size_t);

void* mmap64(void* addr, size_t size, int prot, int flags, int fd, off64_t offset) {
  static constexpr size_t MMAP2_SHIFT = 12;

  if (offset < 0 || (offset & ((1UL << MMAP2_SHIFT) - 1)) != 0) {
    errno = EINVAL;
    return MAP_FAILED;
  }

  // Prevent allocations large enough for `end - start` to overflow,
  // to avoid security bugs.
  size_t rounded = __BIONIC_ALIGN(size, page_size());
  if (rounded < size || rounded > PTRDIFF_MAX) {
    errno = ENOMEM;
    return MAP_FAILED;
  }

  return __mmap2(addr, size, prot, flags, fd, offset >> MMAP2_SHIFT);
}

void* mmap(void* addr, size_t size, int prot, int flags, int fd, off_t offset) {
  return mmap64(addr, size, prot, flags, fd, static_cast<off64_t>(offset));
}

(annoyingly there's no named constant in the kernel for that 12 that i've ever found, but if you grep arch/ for mmap2, you find lots of copy & pasted comments reminding the various arch maintainers that it's not the page size :-) )

note that that's only used for ILP32. for LP64 we use the trivial generated assembler syscall stub, which doesn't need any of this.

[nickdesaulniers]

an int64_t, i assume :-)

oops, yep. Will update commit message + description.

in 2024, having two off_t types is deep into "historical cruft" territory.

Right, in terms of https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html#index-_005fFILE_005fOFFSET_005fBITS, I'd rather llvm-libc have the policy that off_t is 64b ALWAYS, regardless of ILP32 or LP64 data models.

this sounds worse than what the actual change does --- you're actually only truncating offset >> 12, not the offset.

Will update the PR description.

but why not just do the right thing? it's only a couple of lines. see bionic:

yeah, I was reading exactly that code. Perhaps it's time to just fix mmap. I don't think we quite need to provide mmap2 or mmap64 as symbols, and I don't think we should attempt to support kernels that lack mmap2. Instead, it's probably just time to rewrite our mmap to not suck so much.

[enh-google]

I don't think we quite need to provide mmap2 or mmap64 as symbols

we only have mmap64() for historical reasons (we had to retrofit _FILE_OFFSET_BITS=64 pretty late), and i don't think anyone exposes mmap2().

(fwiw, musl removed a their _LARGEFILE64_SOURCE junk recently. it didn't cause much trouble that i know of, and means that folks likely to have trouble have already had it :-) )

and I don't think we should attempt to support kernels that lack mmap2

yeah, the toybox guy has a "seven year rule" -- https://landley.net/toybox/faq.html#support_horizon -- that i think is quite reasonable, but Linux 2.4 is way beyond that :-)

[nickdesaulniers]

For Linux kernels, I'll only support kernels that kernel developers do (those listed on kernel.org). Though now I'm confused; does mmap2 exist on 64b targets? That bionic code you cited above looks like it's always doing that shift on the offset. Is that right for 64b targets? I guess the man page for mmap mentions that mmap2 is used under the hood, but I guess I'm surprised to NOT set __NR_mmap2 in /usr/include/asm/unistd_64.h.

[enh-google]

For Linux kernels, I'll only support kernels that kernel developers do (those listed on kernel.org). Though now I'm confused; does mmap2 exist on 64b targets? That bionic code you cited above looks like it's always doing that shift on the offset. Is that right for 64b targets? I guess the man page for mmap mentions that mmap2 is used under the hood, but I guess I'm surprised to NOT set __NR_mmap2 in /usr/include/asm/unistd_64.h.

that bionic code is in a file called legacy_32_bit_support.cpp :-) the LP64 architectures get just get an auto-generated assembler stub, but, yes, on LP64 the syscall is called __NR_mmap:

# (mmap only gets two lines because 32-bit bionic only uses the 64-bit call.)
void* __mmap2:mmap2(void*, size_t, int, int, int, long) lp32
void* mmap|mmap64(void*, size_t, int, int, int, off_t) lp64

(__mmap2:mmap2 means "make a stub function called __mmap2() for the syscall __NR_mmap2"; mmap|mmap64 means "make a stub function called mmap() for the syscall __NR_mmap, but also add an ELF alias called mmap64()".)

[nickdesaulniers]

superseded by #96700