From 9df652854d147fa0ed7b086707a61a83ee35375a Mon Sep 17 00:00:00 2001 From: youknowdot Date: Thu, 15 Oct 2026 16:21:37 +0900 Subject: [PATCH] Align mmap operations and capability reporting with Python 3.15 Separate raw descriptor inspection from owned tracking, update flush and naming behavior, and expose resize only on implemented backends. Migrate canonical mmap tests using update_lib while preserving existing RustPython guards. Assisted-by: OpenAI Codex:model version unavailable --- Lib/test/test_mmap.py | 330 +++++++++++++++++++++++------------- crates/host_env/src/mmap.rs | 139 ++++++++++++--- crates/stdlib/src/mmap.rs | 250 +++++++++++++++++---------- 3 files changed, 482 insertions(+), 237 deletions(-) diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 309c0d8c933..38ee32db947 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -1,3 +1,4 @@ +from test import support from test.support import ( requires, _2G, _4G, gc_collect, cpython_only, is_emscripten, is_apple, in_systemd_nspawn_sync_suppressed, @@ -5,8 +6,8 @@ from test.support.import_helper import import_module from test.support.os_helper import TESTFN, unlink from test.support.script_helper import assert_python_ok -import unittest import errno +import unittest import os import re import itertools @@ -58,6 +59,7 @@ def test_basic(self): f.write(b'\0'* (PAGESIZE-3) ) f.flush() m = mmap.mmap(f.fileno(), 2 * PAGESIZE) + self.addCleanup(m.close) finally: f.close() @@ -115,31 +117,28 @@ def test_basic(self): # Try to seek to negative position... self.assertRaises(ValueError, m.seek, -len(m)-1, 2) + @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') + def test_resize(self): + # Create a file to be mmap'ed. + with open(TESTFN, 'bw+') as f: + # Write 2 pages worth of data to the file + f.write(b'\0'* 2 * PAGESIZE) + f.flush() + m = mmap.mmap(f.fileno(), 2 * PAGESIZE) + self.addCleanup(m.close) + # Try resizing map - try: - m.resize(512) - except SystemError: - # resize() not supported - # No messages are printed, since the output of this test suite - # would then be different across platforms. - pass - else: - # resize() is supported - self.assertEqual(len(m), 512) - # Check that we can no longer seek beyond the new size. - self.assertRaises(ValueError, m.seek, 513, 0) - - # Check that the underlying file is truncated too - # (bug #728515) - f = open(TESTFN, 'rb') - try: - f.seek(0, 2) - self.assertEqual(f.tell(), 512) - finally: - f.close() - self.assertEqual(m.size(), 512) + m.resize(512) + self.assertEqual(len(m), 512) + # Check that we can no longer seek beyond the new size. + self.assertRaises(ValueError, m.seek, 513, 0) - m.close() + # Check that the underlying file is truncated too + # (bug #728515) + with open(TESTFN, 'rb') as f: + f.seek(0, 2) + self.assertEqual(f.tell(), 512) + self.assertEqual(m.size(), 512) def test_access_parameter(self): # Test for "access" keyword parameter @@ -184,15 +183,10 @@ def test_access_parameter(self): else: self.fail("Able to write to readonly memory map") - # Ensuring that readonly mmap can't be resized - try: - m.resize(2*mapsize) - except SystemError: # resize is not universally supported - pass - except TypeError: - pass - else: - self.fail("Able to resize readonly memory map") + if hasattr(m, 'resize'): + # Ensuring that readonly mmap can't be resized + with self.assertRaises(TypeError): + m.resize(2 * mapsize) with open(TESTFN, "rb") as fp: self.assertEqual(fp.read(), b'a'*mapsize, "Readonly memory map data file was modified") @@ -243,8 +237,9 @@ def test_access_parameter(self): with open(TESTFN, "rb") as fp: self.assertEqual(fp.read(), b'c'*mapsize, "Copy-on-write test data file should not be modified.") - # Ensuring copy-on-write maps cannot be resized - self.assertRaises(TypeError, m.resize, 2*mapsize) + if hasattr(m, 'resize'): + # Ensuring copy-on-write maps cannot be resized + self.assertRaises(TypeError, m.resize, 2 * mapsize) m.close() # Ensuring invalid access parameter raises exception @@ -271,62 +266,57 @@ def test_access_parameter(self): self.assertRaises(TypeError, m.write_byte, 0) m.close() - @unittest.skipIf(os.name == 'nt', 'trackfd not present on Windows') - def test_trackfd_parameter(self): + @support.subTests('close_original_fd', (True, False)) + def test_trackfd_parameter(self, close_original_fd): size = 64 with open(TESTFN, "wb") as f: f.write(b"a"*size) - for close_original_fd in True, False: - with self.subTest(close_original_fd=close_original_fd): - with open(TESTFN, "r+b") as f: - with mmap.mmap(f.fileno(), size, trackfd=False) as m: - if close_original_fd: - f.close() - self.assertEqual(len(m), size) - with self.assertRaises(OSError) as err_cm: - m.size() - self.assertEqual(err_cm.exception.errno, errno.EBADF) - with self.assertRaises(ValueError): - m.resize(size * 2) - with self.assertRaises(ValueError): - m.resize(size // 2) - self.assertEqual(m.closed, False) - - # Smoke-test other API - m.write_byte(ord('X')) - m[2] = ord('Y') - m.flush() - with open(TESTFN, "rb") as f: - self.assertEqual(f.read(4), b'XaYa') - self.assertEqual(m.tell(), 1) - m.seek(0) - self.assertEqual(m.tell(), 0) - self.assertEqual(m.read_byte(), ord('X')) - - self.assertEqual(m.closed, True) - self.assertEqual(os.stat(TESTFN).st_size, size) - - @unittest.skipIf(os.name == 'nt', 'trackfd not present on Windows') + with open(TESTFN, "r+b") as f: + with mmap.mmap(f.fileno(), size, trackfd=False) as m: + if close_original_fd: + f.close() + self.assertEqual(len(m), size) + with self.assertRaises(ValueError): + m.size() + if hasattr(m, 'resize'): + with self.assertRaises(ValueError): + m.resize(size * 2) + with self.assertRaises(ValueError): + m.resize(size // 2) + self.assertIs(m.closed, False) + + # Smoke-test other API + m.write_byte(ord('X')) + m[2] = ord('Y') + m.flush() + with open(TESTFN, "rb") as f: + self.assertEqual(f.read(4), b'XaYa') + self.assertEqual(m.tell(), 1) + m.seek(0) + self.assertEqual(m.tell(), 0) + self.assertEqual(m.read_byte(), ord('X')) + + if os.name == 'nt' and not close_original_fd: + self.assertRaises(PermissionError, os.rename, TESTFN, TESTFN+'1') + else: + os.rename(TESTFN, TESTFN+'1') + os.rename(TESTFN+'1', TESTFN) + + self.assertIs(m.closed, True) + self.assertEqual(os.stat(TESTFN).st_size, size) + def test_trackfd_neg1(self): size = 64 with mmap.mmap(-1, size, trackfd=False) as m: - with self.assertRaises(OSError): - m.size() with self.assertRaises(ValueError): - m.resize(size // 2) + m.size() + if hasattr(m, 'resize'): + with self.assertRaises(ValueError): + m.resize(size // 2) self.assertEqual(len(m), size) m[0] = ord('a') assert m[0] == ord('a') - @unittest.skipIf(os.name != 'nt', 'trackfd only fails on Windows') - def test_no_trackfd_parameter_on_windows(self): - # 'trackffd' is an invalid keyword argument for this function - size = 64 - with self.assertRaises(TypeError): - mmap.mmap(-1, size, trackfd=True) - with self.assertRaises(TypeError): - mmap.mmap(-1, size, trackfd=False) - def test_bad_file_desc(self): # Try opening a bad file descriptor... self.assertRaises(OSError, mmap.mmap, -2, 4096) @@ -367,6 +357,8 @@ def test_find_end(self): self.assertEqual(m.find(b'one', 1, -1), 8) self.assertEqual(m.find(b'one', 1, -2), -1) self.assertEqual(m.find(bytearray(b'one')), 0) + self.assertEqual(m.find(b'', n + 1), -1) + self.assertEqual(m.rfind(b'', n + 1), -1) for i in range(-n-1, n+1): for j in range(-n-1, n+1): @@ -510,6 +502,7 @@ def test_anonymous(self): b = x & 0xff m[x] = b self.assertEqual(m[x], b) + self.assertEqual(m.size(), PAGESIZE) @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON") def test_read_all(self): @@ -621,13 +614,9 @@ def test_offset (self): self.assertEqual(m[0:3], b'foo') f.close() - # Try resizing map - try: + if hasattr(m, 'resize'): + # Try resizing map m.resize(512) - except SystemError: - pass - else: - # resize() is supported self.assertEqual(len(m), 512) # Check that we can no longer seek beyond the new size. self.assertRaises(ValueError, m.seek, 513, 0) @@ -825,14 +814,12 @@ def test_write_returning_the_number_of_bytes_written(self): self.assertEqual(mm.write(b"python"), 6) @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON") + @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') def test_resize_past_pos(self): m = mmap.mmap(-1, 8192) self.addCleanup(m.close) m.read(5000) - try: - m.resize(4096) - except SystemError: - self.skipTest("resizing not supported") + m.resize(4096) self.assertEqual(m.read(14), b'') self.assertRaises(ValueError, m.read_byte) self.assertRaises(ValueError, m.write_byte, 42) @@ -924,6 +911,7 @@ def __index__(self): self.assertEqual(m.madvise(mmap.MADV_NORMAL, 0, Number()), None) self.assertEqual(m.madvise(mmap.MADV_NORMAL, 0, size), None) + @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') def test_resize_up_anonymous_mapping(self): """If the mmap is backed by the pagefile ensure a resize up can happen and that the original data is still in place @@ -934,23 +922,21 @@ def test_resize_up_anonymous_mapping(self): with mmap.mmap(-1, start_size) as m: m[:] = data - if sys.platform.startswith(('linux', 'android')): - # Can't expand a shared anonymous mapping on Linux. - # See https://bugzilla.kernel.org/show_bug.cgi?id=8691 + if sys.platform.startswith(('linux', 'android', 'netbsd')): + # Can't expand a shared anonymous mapping on Linux + # (see https://bugzilla.kernel.org/show_bug.cgi?id=8691) + # or NetBSD. with self.assertRaises(ValueError): m.resize(new_size) else: - try: - m.resize(new_size) - except SystemError: - pass - else: - self.assertEqual(len(m), new_size) - self.assertEqual(m[:start_size], data) - self.assertEqual(m[start_size:], b'\0' * (new_size - start_size)) + m.resize(new_size) + self.assertEqual(len(m), new_size) + self.assertEqual(m[:start_size], data) + self.assertEqual(m[start_size:], b'\0' * (new_size - start_size)) @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON") @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') def test_resize_up_private_anonymous_mapping(self): start_size = PAGESIZE new_size = 2 * start_size @@ -958,16 +944,13 @@ def test_resize_up_private_anonymous_mapping(self): with mmap.mmap(-1, start_size, flags=mmap.MAP_PRIVATE) as m: m[:] = data - try: - m.resize(new_size) - except SystemError: - pass - else: - self.assertEqual(len(m), new_size) - self.assertEqual(m[:start_size], data) - self.assertEqual(m[start_size:], b'\0' * (new_size - start_size)) + m.resize(new_size) + self.assertEqual(len(m), new_size) + self.assertEqual(m[:start_size], data) + self.assertEqual(m[start_size:], b'\0' * (new_size - start_size)) @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON") + @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') def test_resize_down_anonymous_mapping(self): """If the mmap is backed by the pagefile ensure a resize down up can happen and that a truncated form of the original data is still in place @@ -978,17 +961,56 @@ def test_resize_down_anonymous_mapping(self): with mmap.mmap(-1, start_size) as m: m[:] = data - try: - m.resize(new_size) - except SystemError: - pass - else: - self.assertEqual(len(m), new_size) - self.assertEqual(m[:], data[:new_size]) - if sys.platform.startswith(('linux', 'android')): - # Can't expand to its original size. - with self.assertRaises(ValueError): - m.resize(start_size) + m.resize(new_size) + self.assertEqual(len(m), new_size) + self.assertEqual(m[:], data[:new_size]) + if sys.platform.startswith(('linux', 'android')): + # Can't expand to its original size. + with self.assertRaises(ValueError): + m.resize(start_size) + + @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') + def test_setitem_resize_reentrancy(self): + """Resizing the mmap from inside __index__ while assigning to a + single item must not access memory past the new bounds (gh-157335). + """ + size = 2 * PAGESIZE + new_size = PAGESIZE + + class ResizeOnIndex: + def __init__(self, m): + self.m = m + def __index__(self): + self.m.resize(new_size) + return 0 + + with mmap.mmap(-1, size) as m: + with self.assertRaises(IndexError): + m[size - 1] = ResizeOnIndex(m) + self.assertEqual(len(m), new_size) + + @unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize') + def test_setitem_slice_resize_reentrancy(self): + """Resizing the mmap from inside a value's buffer-protocol + callback while assigning to a slice must not access memory past + the new bounds (gh-157335). + """ + size = 2 * PAGESIZE + new_size = PAGESIZE + + class ResizeOnBuffer: + def __init__(self, m, data): + self.m = m + self.data = data + def __buffer__(self, flags): + self.m.resize(new_size) + return memoryview(self.data) + + with mmap.mmap(-1, size) as m: + value = ResizeOnBuffer(m, bytes(size)) + with self.assertRaises(IndexError): + m[0:size] = value + self.assertEqual(len(m), new_size) @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON") @unittest.skipUnless(os.name == 'nt', 'requires Windows') @@ -1197,6 +1219,74 @@ def test_access_violations(self): self.assertEqual(stdout.strip(), b'') self.assertEqual(stderr.strip(), b'') + def test_flush_parameters(self): + with open(TESTFN, 'wb+') as f: + f.write(b'x' * PAGESIZE * 3) + f.flush() + + m = mmap.mmap(f.fileno(), PAGESIZE * 3) + self.addCleanup(m.close) + + m.flush() + m.flush(PAGESIZE) + m.flush(PAGESIZE, PAGESIZE) + + if hasattr(mmap, 'MS_SYNC'): + m.flush(0, PAGESIZE, flags=mmap.MS_SYNC) + if hasattr(mmap, 'MS_ASYNC'): + m.flush(flags=mmap.MS_ASYNC) + if hasattr(mmap, 'MS_INVALIDATE'): + m.flush(PAGESIZE * 2, flags=mmap.MS_INVALIDATE) + if hasattr(mmap, 'MS_ASYNC') and hasattr(mmap, 'MS_INVALIDATE'): + if sys.platform.startswith(('freebsd', 'dragonfly')): + # FreeBSD and DragonFly don't support this combination + with self.assertRaises(OSError) as cm: + m.flush(0, PAGESIZE, flags=mmap.MS_ASYNC | mmap.MS_INVALIDATE) + self.assertEqual(cm.exception.errno, errno.EINVAL) + else: + m.flush(0, PAGESIZE, flags=mmap.MS_ASYNC | mmap.MS_INVALIDATE) + + @unittest.skipUnless(sys.platform == 'linux', 'Linux only') + @support.requires_linux_version(5, 17, 0) + @unittest.skipIf(support.linked_to_musl(), "musl libc issue, gh-143632") + def test_set_name(self): + # Test setting name on anonymous mmap + m = mmap.mmap(-1, PAGESIZE) + self.addCleanup(m.close) + try: + result = m.set_name('test_mapping') + except OSError as exc: + if exc.errno == errno.EINVAL: + # gh-142419: On Fedora, prctl(PR_SET_VMA_ANON_NAME) fails with + # EINVAL because the kernel option CONFIG_ANON_VMA_NAME is + # disabled. + # See: https://bugzilla.redhat.com/show_bug.cgi?id=2302746 + self.skipTest("prctl() failed with EINVAL") + else: + raise + self.assertIsNone(result) + + # Test name length limit (80 chars including prefix "cpython:mmap:" and '\0') + # Prefix is 13 chars, so max name is 66 chars + long_name = 'x' * 66 + result = m.set_name(long_name) + self.assertIsNone(result) + + # Test name too long + too_long_name = 'x' * 67 + with self.assertRaises(ValueError): + m.set_name(too_long_name) + + # Test that file-backed mmap raises error + with open(TESTFN, 'wb+') as f: + f.write(b'x' * PAGESIZE) + f.flush() + m2 = mmap.mmap(f.fileno(), PAGESIZE) + self.addCleanup(m2.close) + + with self.assertRaises(ValueError): + m2.set_name('should_fail') + class LargeMmapTests(unittest.TestCase): diff --git a/crates/host_env/src/mmap.rs b/crates/host_env/src/mmap.rs index 976d78e2c38..4ce4c52a40d 100644 --- a/crates/host_env/src/mmap.rs +++ b/crates/host_env/src/mmap.rs @@ -7,8 +7,8 @@ use std::io; #[cfg(unix)] pub use libc::{ - MADV_DONTNEED, MADV_NORMAL, MADV_RANDOM, MADV_SEQUENTIAL, MADV_WILLNEED, PROT_EXEC, PROT_READ, - PROT_WRITE, + MADV_DONTNEED, MADV_NORMAL, MADV_RANDOM, MADV_SEQUENTIAL, MADV_WILLNEED, MS_ASYNC, + MS_INVALIDATE, MS_SYNC, PROT_EXEC, PROT_READ, PROT_WRITE, }; #[cfg(unix)] @@ -202,10 +202,43 @@ impl MappedFile { } } - pub fn flush_range(&self, offset: usize, size: usize) -> io::Result<()> { + pub fn flush_range(&self, offset: usize, size: usize, flags: i32) -> io::Result<()> { + #[cfg(unix)] + { + if offset > self.as_slice().len() || size > self.as_slice().len() - offset { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } + let ptr = unsafe { self.as_ptr().add(offset) }; + let flags = if flags == 0 { MS_SYNC } else { flags }; + if unsafe { libc::msync(ptr.cast_mut().cast(), size, flags) } == -1 { + return Err(io::Error::last_os_error()); + } + Ok(()) + } + #[cfg(not(unix))] + { + let _ = flags; + match self { + Self::Read(_) => Ok(()), + Self::Write(mmap) => mmap.flush_range(offset, size), + } + } + } + + /// # Safety + /// The backing file must cover the mapping offset plus `new_size`, and no + /// exported pointers may remain live while the mapping is moved or shrunk. + #[cfg(target_os = "linux")] + pub unsafe fn remap(&mut self, new_size: usize) -> io::Result<()> { + // memmap2 permits a zero-length mapping, whereas mmap.resize(0) must + // preserve mremap's EINVAL result. + if new_size == 0 { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } + let options = memmap2::RemapOptions::new().may_move(true); match self { - Self::Read(_) => Ok(()), - Self::Write(mmap) => mmap.flush_range(offset, size), + Self::Read(mmap) => unsafe { mmap.remap(new_size, options) }, + Self::Write(mmap) => unsafe { mmap.remap(new_size, options) }, } } @@ -216,6 +249,23 @@ impl MappedFile { } } +#[cfg(target_os = "linux")] +pub fn set_mapping_name(address: usize, size: usize, name: &core::ffi::CStr) -> io::Result<()> { + if unsafe { + libc::prctl( + libc::PR_SET_VMA, + libc::PR_SET_VMA_ANON_NAME, + address, + size, + name.as_ptr(), + ) + } == -1 + { + return Err(io::Error::last_os_error()); + } + Ok(()) +} + #[cfg(windows)] unsafe impl Send for NamedMmap {} #[cfg(windows)] @@ -293,16 +343,40 @@ pub fn file_len(fd: crt_fd::Borrowed<'_>) -> io::Result { Ok(fileutils::fstat(fd)?.st_size.into()) } +/// Inspect Python's raw descriptor without assuming that it is a valid Rust fd. +/// CPython ignores failed fstat calls and only bounds regular-file mappings. #[cfg(unix)] -pub fn prepare_file_mapping(fd: crt_fd::Borrowed<'_>) { - #[cfg(target_os = "macos")] - { - let _ = posix::full_fsync(fd.into()); +pub fn regular_file_len(fd: i32) -> Option { + // Use the same target selection as memmap2's mmap64/off64_t aliases. + #[cfg(not(any( + target_os = "android", + all(target_os = "linux", not(target_env = "musl")) + )))] + use libc::{fstat, stat as Stat}; + #[cfg(any( + target_os = "android", + all(target_os = "linux", not(target_env = "musl")) + ))] + use libc::{fstat64 as fstat, stat64 as Stat}; + + let mut status = core::mem::MaybeUninit::::uninit(); + // SAFETY: fstat accepts an arbitrary integer and initializes status only + // on success. No borrowed/owned descriptor is constructed from that input. + if unsafe { fstat(fd, status.as_mut_ptr()) } != 0 { + return None; } - #[cfg(not(target_os = "macos"))] - { - let _ = fd; + let status = unsafe { status.assume_init() }; + if status.st_mode & libc::S_IFMT != libc::S_IFREG { + return None; } + #[allow(clippy::useless_conversion, reason = "needed for 32-bit platforms")] + i64::try_from(status.st_size).ok() +} + +#[cfg(target_os = "macos")] +pub fn prepare_file_mapping(fd: i32) { + // CPython intentionally ignores F_FULLFSYNC errors, including invalid fds. + let _ = unsafe { libc::fcntl(fd, libc::F_FULLFSYNC) }; } #[cfg(windows)] @@ -409,25 +483,44 @@ pub fn map_anon(size: usize) -> io::Result { } #[cfg(unix)] -pub fn map_file( - fd: crt_fd::Borrowed<'_>, - offset: i64, - size: usize, - access: AccessMode, -) -> io::Result<(crt_fd::Owned, MappedFile)> { - let new_fd: crt_fd::Owned = posix::dup_noninheritable(fd.into())?.into(); +pub fn duplicate_descriptor(fd: i32) -> io::Result { + // Python supplies an arbitrary integer, so validate it through fcntl before + // constructing a Rust descriptor. Only the successful duplicate is owned. + let new_fd = unsafe { libc::fcntl(fd, libc::F_DUPFD_CLOEXEC, 0) }; + if new_fd != -1 { + return Ok(unsafe { crt_fd::Owned::from_raw(new_fd) }); + } + let err = io::Error::last_os_error(); + if err.raw_os_error() != Some(libc::EINVAL) { + return Err(err); + } + // Older kernels may not support F_DUPFD_CLOEXEC. + let new_fd = unsafe { libc::dup(fd) }; + if new_fd == -1 { + return Err(io::Error::last_os_error()); + } + let new_fd = unsafe { crt_fd::Owned::from_raw(new_fd) }; + posix::set_inheritable(new_fd.borrow().into(), false)?; + Ok(new_fd) +} + +#[cfg(unix)] +pub fn map_file(fd: i32, offset: i64, size: usize, access: AccessMode) -> io::Result { let mut mmap_opt = MmapOptions::new(); let mmap_opt = mmap_opt.offset(offset as u64).len(size); + // len is always explicit, so memmap2 never wraps this raw fd in File to + // query metadata. Its mmap syscall rejects invalid descriptors directly. + // The caller retains any owned tracking descriptor through this call. let mapped = match access { AccessMode::Default | AccessMode::Write => { - unsafe { mmap_opt.map_mut(&new_fd) }.map(MappedFile::Write)? + unsafe { mmap_opt.map_mut(fd) }.map(MappedFile::Write)? } - AccessMode::Read => unsafe { mmap_opt.map(&new_fd) }.map(MappedFile::Read)?, - AccessMode::Copy => unsafe { mmap_opt.map_copy(&new_fd) }.map(MappedFile::Write)?, + AccessMode::Read => unsafe { mmap_opt.map(fd) }.map(MappedFile::Read)?, + AccessMode::Copy => unsafe { mmap_opt.map_copy(fd) }.map(MappedFile::Write)?, }; - Ok((new_fd, mapped)) + Ok(mapped) } #[cfg(all(unix, not(target_os = "redox")))] diff --git a/crates/stdlib/src/mmap.rs b/crates/stdlib/src/mmap.rs index 790a7bc0fef..1064678a8f2 100644 --- a/crates/stdlib/src/mmap.rs +++ b/crates/stdlib/src/mmap.rs @@ -11,7 +11,7 @@ mod mmap { use crate::vm::{ AsObject, FromArgs, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromBorrowedObject, VirtualMachine, atomic_func, - builtins::{PyBytes, PyBytesRef, PyInt, PyIntRef, PyType, PyTypeRef}, + builtins::{PyBytes, PyBytesRef, PyInt, PyIntRef, PyType, PyTypeRef, PyUtf8StrRef}, byte::{bytes_from_object, value_from_object}, convert::ToPyException, function::{ArgBytesLike, ArgIndex}, @@ -72,7 +72,8 @@ mod mmap { #[pyattr] use host_mmap::{ MADV_DONTNEED, MADV_NORMAL, MADV_RANDOM, MADV_SEQUENTIAL, MADV_WILLNEED, MAP_ANON, - MAP_ANONYMOUS, MAP_PRIVATE, MAP_SHARED, PROT_EXEC, PROT_READ, PROT_WRITE, + MAP_ANONYMOUS, MAP_PRIVATE, MAP_SHARED, MS_ASYNC, MS_INVALIDATE, MS_SYNC, PROT_EXEC, + PROT_READ, PROT_WRITE, }; #[cfg(target_os = "macos")] @@ -191,9 +192,6 @@ mod mmap { mmap: PyMutex>, #[cfg(unix)] fd: AtomicCell, - // only read by the linux/netbsd mremap expansion check - #[cfg(any(target_os = "linux", target_os = "netbsd"))] - flags: core::ffi::c_int, #[cfg(windows)] handle: AtomicCell, // host_mmap::Handle is isize on Windows offset: i64, @@ -203,6 +201,10 @@ mod mmap { access: AccessMode, #[cfg(unix)] trackfd: bool, + #[cfg(target_os = "linux")] + anonymous: bool, + #[cfg(target_os = "linux")] + private: bool, } impl PyMmap { @@ -296,6 +298,8 @@ mod mmap { offset: isize, #[pyarg(positional, optional)] size: Option, + #[pyarg(named, default)] + flags: core::ffi::c_int, } impl FlushOptions { @@ -304,14 +308,15 @@ mod mmap { return None; } let offset = self.offset as usize; + let remaining = len.checked_sub(offset)?; let size = match self.size { + None | Some(-1) => remaining, Some(s) if s < 0 => return None, Some(s) => s as usize, - None => len, }; - if len.checked_sub(offset)? < size { + if remaining < size { return None; } @@ -408,10 +413,7 @@ mod mmap { } // TODO: memmap2 doesn't support mapping with prot and flags right now - #[cfg_attr( - not(any(target_os = "linux", target_os = "netbsd")), - allow(unused_variables) - )] + #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] let (flags, _prot, access) = match access { AccessMode::Read => (MAP_SHARED, PROT_READ, access), AccessMode::Write => (MAP_SHARED, PROT_READ | PROT_WRITE, access), @@ -428,24 +430,23 @@ mod mmap { } }; - let fd = unsafe { crt_fd::Borrowed::try_borrow_raw(fd) }; + let anonymous = fd == -1 || flags & host_mmap::MAP_ANONYMOUS != 0; - // macOS: Issue #11277: fsync(2) is not enough on OS X - a special, OS X specific - // fcntl(2) is necessary to force DISKSYNC and get around mmap(2) bug + // Inspect the supplied descriptor before acquiring ownership, as + // CPython does. Invalid and non-regular fds do not impose file bounds. #[cfg(target_os = "macos")] - if let Ok(fd) = fd { + if fd != -1 { host_mmap::prepare_file_mapping(fd); } - - if let Ok(fd) = fd { - let file_len = host_mmap::file_len(fd).map_err(|err| err.to_pyexception(vm))?; - + if fd != -1 + && let Some(file_len) = host_mmap::regular_file_len(fd) + { if map_size == 0 { if file_len == 0 { return Err(vm.new_value_error("cannot mmap an empty file")); } - if offset > file_len { + if offset >= file_len { return Err(vm.new_value_error("mmap offset is greater than file size")); } @@ -457,37 +458,40 @@ mod mmap { } } - let (fd, mmap) = || -> std::io::Result<_> { - if let Ok(fd) = fd { - let (new_fd, mmap) = host_mmap::map_file( - fd, - offset, - map_size, - match access { - AccessMode::Default => host_mmap::AccessMode::Default, - AccessMode::Read => host_mmap::AccessMode::Read, - AccessMode::Write => host_mmap::AccessMode::Write, - AccessMode::Copy => host_mmap::AccessMode::Copy, - }, - )?; - Ok((Some(new_fd), mmap)) - } else { - let mmap = host_mmap::map_anon(map_size)?; - Ok((None, mmap)) - } - }() + let retained_fd = if fd != -1 && trackfd { + Some(host_mmap::duplicate_descriptor(fd).map_err(|err| err.to_pyexception(vm))?) + } else { + None + }; + let mmap = if anonymous { + host_mmap::map_anon(map_size) + } else { + // Keep an owned tracking fd alive through mmap when requested. + // Otherwise use the raw integer directly, without fabricating a + // BorrowedFd or creating a transient duplicate that can hit EMFILE. + let mapping_fd = retained_fd.as_ref().map_or(fd, |owned| owned.as_raw()); + host_mmap::map_file( + mapping_fd, + offset, + map_size, + match access { + AccessMode::Default => host_mmap::AccessMode::Default, + AccessMode::Read => host_mmap::AccessMode::Read, + AccessMode::Write => host_mmap::AccessMode::Write, + AccessMode::Copy => host_mmap::AccessMode::Copy, + }, + ) + } .map_err(|e| e.to_pyexception(vm))?; Ok(Self { closed: AtomicCell::new(false), mmap: PyMutex::new(Some(MmapObj::Mapped(mmap))), - fd: AtomicCell::new(if trackfd { - fd.map_or(-1, |owned| owned.into_raw()) - } else { - -1 - }), - #[cfg(any(target_os = "linux", target_os = "netbsd"))] - flags, + #[cfg(target_os = "linux")] + anonymous, + #[cfg(target_os = "linux")] + private: flags & MAP_PRIVATE != 0, + fd: AtomicCell::new(retained_fd.map_or(-1, |owned| owned.into_raw())), offset, size: AtomicCell::new(map_size), pos: AtomicCell::new(0), @@ -793,8 +797,7 @@ mod mmap { Ok(m) } - /// TODO: impl resize - #[allow(dead_code)] + #[cfg(any(target_os = "linux", windows, all(test, feature = "threading")))] fn check_resizeable( &self, vm: &VirtualMachine, @@ -913,6 +916,8 @@ mod mmap { #[pymethod] fn flush(zelf: &Py, options: FlushOptions, vm: &VirtualMachine) -> PyResult<()> { + let mmap = zelf.check_valid(vm)?; + let flags = options.flags; let (offset, size) = options .values(zelf.__len__()) .ok_or_else(|| vm.new_value_error("flush values out of range"))?; @@ -921,9 +926,9 @@ mod mmap { return Ok(()); } - match zelf.check_valid(vm)?.deref().as_ref().unwrap() { + match mmap.deref().as_ref().unwrap() { MmapObj::Mapped(mmap) => { - mmap.flush_range(offset, size) + mmap.flush_range(offset, size, flags) .map_err(|e| e.to_pyexception(vm))?; } #[cfg(windows)] @@ -937,15 +942,54 @@ mod mmap { Ok(()) } + #[pymethod] + fn set_name(zelf: &Py, name: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult<()> { + let name = + alloc::ffi::CString::new(name.as_str()).map_err(|err| err.to_pyexception(vm))?; + #[cfg(target_os = "linux")] + { + const PREFIX: &str = "cpython:mmap:"; + if name.as_bytes().len() > 79 - PREFIX.len() { + return Err(vm.new_value_error("name is too long")); + } + if !zelf.anonymous { + return Err( + vm.new_value_error("Cannot set annotation on non-anonymous mappings") + ); + } + // As in CPython's release build, kernel annotations are enabled + // only in development mode. + if vm.state.config.settings.dev_mode { + let annotation = + alloc::ffi::CString::new([PREFIX.as_bytes(), name.as_bytes()].concat()) + .expect("the name was checked for embedded NUL characters"); + let mmap = zelf.mmap.lock(); + let address = mmap + .as_ref() + .map_or(0, |mapping| mapping.as_slice().as_ptr() as usize); + host_mmap::set_mapping_name(address, zelf.__len__(), &annotation) + .map_err(|err| err.to_pyexception(vm))?; + } + Ok(()) + } + #[cfg(not(target_os = "linux"))] + { + let _ = (zelf, name); + Err(vm.new_not_implemented_error( + "Annotation of mmap is not supported on this platform", + )) + } + } + #[cfg(all(unix, not(target_os = "redox")))] #[pymethod] fn madvise(zelf: &Py, options: AdviseOptions, vm: &VirtualMachine) -> PyResult<()> { + let guard = zelf.check_valid(vm)?; let (option, start, length) = options.values(zelf.__len__(), vm)?; if !host_mmap::validate_advice(option) { return Err(vm.new_value_error("Not a valid Advice value")); } - let guard = zelf.check_valid(vm)?; let mmap = guard.deref().as_ref().unwrap(); match mmap { MmapObj::Mapped(m) => m.madvise_range(start, length, option), @@ -987,14 +1031,13 @@ mod mmap { Some((dest, src, cnt)) } - let size = zelf.__len__(); - let (dest, src, cnt) = args(dest.as_ref(), src.as_ref(), count.as_ref(), size, vm) - .ok_or_else(|| vm.new_value_error("source, destination, or count out of range"))?; - - let dest_end = dest + cnt; - let src_end = src + cnt; - zelf.try_writable(vm, |mmap| { + let (dest, src, cnt) = + args(dest.as_ref(), src.as_ref(), count.as_ref(), mmap.len(), vm).ok_or_else( + || vm.new_value_error("source, destination, or count out of range"), + )?; + let dest_end = dest + cnt; + let src_end = src + cnt; let src_buf = mmap[src..src_end].to_vec(); (&mut mmap[dest..dest_end]) .write(&src_buf) @@ -1025,6 +1068,9 @@ mod mmap { .filter(|&n| n >= 0 && (n as usize) <= remaining) .map_or(remaining, |n| n as usize); + if num_bytes == 0 { + return Ok(PyBytes::from(vec![]).into_ref(&vm.ctx)); + } let end_pos = pos + num_bytes; let bytes = mmap.deref().as_ref().unwrap().as_slice()[pos..end_pos].to_vec(); @@ -1037,12 +1083,13 @@ mod mmap { #[pymethod] fn read_byte(zelf: &Py, vm: &VirtualMachine) -> PyResult { + let mmap = zelf.check_valid(vm)?; let pos = zelf.pos(); if pos >= zelf.__len__() { return Err(vm.new_value_error("read byte out of range")); } - let b = zelf.check_valid(vm)?.deref().as_ref().unwrap().as_slice()[pos]; + let b = mmap.deref().as_ref().unwrap().as_slice()[pos]; zelf.advance_pos(1); @@ -1077,29 +1124,38 @@ mod mmap { Ok(result) } - #[cfg(unix)] + #[cfg(target_os = "linux")] #[pymethod] fn resize(zelf: &Py, newsize: PyIntRef, vm: &VirtualMachine) -> PyResult<()> { - drop(zelf.check_resizeable(vm)?); - - let new_size: isize = newsize.try_to_primitive(vm).map_err(|_| { + let newsize: isize = newsize.try_to_primitive(vm).map_err(|_| { vm.new_overflow_error("Python int too large to convert to C ssize_t") })?; - - // Linux mremap() refuses to grow a shared anonymous mapping, and NetBSD - // mremap() returns a mapping whose grown region is not backed. - #[cfg(any(target_os = "linux", target_os = "netbsd"))] - if zelf.fd.load() == -1 - && zelf.flags & host_mmap::MAP_PRIVATE == 0 - && new_size > zelf.size.load() as isize - { + // Reject exports before taking the data lock, and retain the guard + // through truncation and remapping so a new export cannot race us. + let mut mmap = zelf.check_resizeable(vm)?; + let file_size = i128::from(zelf.offset) + newsize as i128; + if newsize < 0 || file_size > isize::MAX as i128 { + return Err(vm.new_value_error("new size out of range")); + } + let newsize = newsize as usize; + if zelf.anonymous && !zelf.private && newsize > zelf.__len__() { return Err(vm.new_value_error("mmap: can't expand a shared anonymous mapping")); } - - // TODO: implement using mremap on Linux - #[cfg(not(any(target_os = "linux", target_os = "netbsd")))] - let _ = new_size; - Err(vm.new_system_error("mmap: resizing not available--no mremap()")) + let fd = zelf.fd.load(); + if fd != -1 { + let fd = unsafe { crt_fd::Borrowed::try_borrow_raw(fd) } + .map_err(|err| err.to_pyexception(vm))?; + let file_size = file_size + .try_into() + .map_err(|_| vm.new_value_error("new size out of range"))?; + crt_fd::ftruncate(fd, file_size).map_err(|err| err.to_pyexception(vm))?; + } + let MmapObj::Mapped(mapping) = mmap.as_mut().unwrap(); + // The mapping is locked, exported buffers are prohibited, and its + // backing file has already been resized to cover the new mapping. + unsafe { mapping.remap(newsize) }.map_err(|err| err.to_pyexception(vm))?; + zelf.size.store(newsize); + Ok(()) } #[cfg(windows)] @@ -1237,15 +1293,25 @@ mod mmap { #[cfg(unix)] #[pymethod] - fn size(zelf: &Py, vm: &VirtualMachine) -> std::io::Result { - let fd = unsafe { crt_fd::Borrowed::try_borrow_raw(zelf.fd.load())? }; - let file_len = host_mmap::file_len(fd)?; + fn size(zelf: &Py, vm: &VirtualMachine) -> PyResult { + let _mmap = zelf.check_valid(vm)?; + if !zelf.trackfd { + return Err(vm.new_value_error("can't get size with trackfd=False")); + } + let fd = zelf.fd.load(); + if fd == -1 { + return Ok(PyInt::from(zelf.__len__()).into_ref(&vm.ctx)); + } + let fd = unsafe { crt_fd::Borrowed::try_borrow_raw(fd) } + .map_err(|err| err.to_pyexception(vm))?; + let file_len = host_mmap::file_len(fd).map_err(|err| err.to_pyexception(vm))?; Ok(PyInt::from(file_len).into_ref(&vm.ctx)) } #[cfg(windows)] #[pymethod] fn size(zelf: &Py, vm: &VirtualMachine) -> PyResult { + let _mmap = zelf.check_valid(vm)?; let handle = zelf.handle.load(); if host_mmap::is_invalid_handle_value(handle) { // Anonymous mapping, return the mmap size @@ -1271,7 +1337,6 @@ mod mmap { fn write(zelf: &Py, bytes: ArgBytesLike, vm: &VirtualMachine) -> PyResult { let self_ = &**zelf; let pos = self_.pos(); - let size = self_.__len__(); // Writing locks the map, and reading a source that views this same // map locks it too, so such a source is copied out first. @@ -1285,11 +1350,10 @@ mod mmap { &borrowed }; - if pos > size || size - pos < data.len() { - return Err(vm.new_value_error("data out of range")); - } - let len = self_.try_writable(vm, |mmap| { + if pos > mmap.len() || mmap.len() - pos < data.len() { + return Err(vm.new_value_error("data out of range")); + } (&mut mmap[pos..(pos + data.len())]) .write(data) .map_err(|err| err.to_pyexception(vm))?; @@ -1306,15 +1370,13 @@ mod mmap { let b = value_from_object(vm, &byte)?; let pos = zelf.pos(); - let size = zelf.__len__(); - - if pos >= size { - return Err(vm.new_value_error("write byte out of range")); - } - zelf.try_writable(vm, |mmap| { + if pos >= mmap.len() { + return Err(vm.new_value_error("write byte out of range")); + } mmap[pos] = b; - })?; + Ok(()) + })??; zelf.advance_pos(1); @@ -1397,11 +1459,12 @@ mod mmap { } fn getitem_by_index(&self, i: isize, vm: &VirtualMachine) -> PyResult { + let mmap = self.check_valid(vm)?; let i = i .wrapped_at(self.__len__()) .ok_or_else(|| vm.new_index_error("mmap index out of range"))?; - let b = self.check_valid(vm)?.deref().as_ref().unwrap().as_slice()[i]; + let b = mmap.deref().as_ref().unwrap().as_slice()[i]; Ok(PyInt::from(b).into_ref(&vm.ctx).into()) } @@ -1411,9 +1474,8 @@ mod mmap { slice: &SaturatedSlice, vm: &VirtualMachine, ) -> PyResult { - let (range, step, slice_len) = slice.adjust_indices(self.__len__()); - let mmap = self.check_valid(vm)?; + let (range, step, slice_len) = slice.adjust_indices(self.__len__()); let slice_data = mmap.deref().as_ref().unwrap().as_slice(); if slice_len == 0 {