Skip to content

Commit 9a90021

Browse files
authored
Add a type-level distinction between aligned and possibly-unaligned Mmaps (#9639)
In the wasmtime codebase there are two kinds of mmaps: those that are always backed by anonymous memory and are always aligned, and those that are possibly file-backed and so the length might be unaligned. In #9620 I accidentally mixed the two up in a way that was a ticking time bomb. To resolve this, add a type parameter to `Mmap` to distinguish between the cases. All of wasmtime's users are clearly either one or the other, so it's quite simple in the end.
1 parent e32292c commit 9a90021

File tree

9 files changed

+134
-56
lines changed

9 files changed

+134
-56
lines changed

crates/wasmtime/src/runtime/vm/gc/enabled/drc.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ use super::{VMArrayRef, VMGcObjectDataMut, VMStructRef};
4646
use crate::hash_set::HashSet;
4747
use crate::prelude::*;
4848
use crate::runtime::vm::{
49-
ExternRefHostDataId, ExternRefHostDataTable, GarbageCollection, GcHeap, GcHeapObject,
50-
GcProgress, GcRootsIter, GcRuntime, Mmap, TypedGcRef, VMExternRef, VMGcHeader, VMGcRef,
49+
mmap::AlignedLength, ExternRefHostDataId, ExternRefHostDataTable, GarbageCollection, GcHeap,
50+
GcHeapObject, GcProgress, GcRootsIter, GcRuntime, Mmap, TypedGcRef, VMExternRef, VMGcHeader,
51+
VMGcRef,
5152
};
5253
use core::ops::{Deref, DerefMut, Range};
5354
use core::{
@@ -90,7 +91,7 @@ struct DrcHeap {
9091
// NB: this box shouldn't be strictly necessary, but it makes upholding the
9192
// safety invariants of the `vmctx_gc_heap_data` more obviously correct.
9293
activations_table: Box<VMGcRefActivationsTable>,
93-
heap: Mmap,
94+
heap: Mmap<AlignedLength>,
9495
free_list: FreeList,
9596
}
9697

crates/wasmtime/src/runtime/vm/gc/enabled/null.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ use super::*;
88
use crate::{
99
prelude::*,
1010
vm::{
11-
ExternRefHostDataId, ExternRefHostDataTable, GarbageCollection, GcHeap, GcHeapObject,
12-
GcProgress, GcRootsIter, Mmap, SendSyncUnsafeCell, TypedGcRef, VMGcHeader, VMGcRef,
11+
mmap::AlignedLength, ExternRefHostDataId, ExternRefHostDataTable, GarbageCollection,
12+
GcHeap, GcHeapObject, GcProgress, GcRootsIter, Mmap, SendSyncUnsafeCell, TypedGcRef,
13+
VMGcHeader, VMGcRef,
1314
},
1415
GcHeapOutOfMemory,
1516
};
@@ -54,7 +55,7 @@ struct NullHeap {
5455
no_gc_count: usize,
5556

5657
/// The actual GC heap.
57-
heap: Mmap,
58+
heap: Mmap<AlignedLength>,
5859
}
5960

6061
/// The common header for all arrays in the null collector.

crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ use super::{
5656
};
5757
use crate::runtime::vm::mpk::{self, ProtectionKey, ProtectionMask};
5858
use crate::runtime::vm::{
59-
CompiledModuleId, InstanceAllocationRequest, InstanceLimits, Memory, MemoryImageSlot, Mmap,
60-
MpkEnabled, PoolingInstanceAllocatorConfig,
59+
mmap::AlignedLength, CompiledModuleId, InstanceAllocationRequest, InstanceLimits, Memory,
60+
MemoryImageSlot, Mmap, MpkEnabled, PoolingInstanceAllocatorConfig,
6161
};
6262
use crate::{prelude::*, vm::round_usize_up_to_host_pages};
6363
use std::ffi::c_void;
@@ -101,7 +101,7 @@ struct Stripe {
101101
/// ```
102102
#[derive(Debug)]
103103
pub struct MemoryPool {
104-
mapping: Mmap,
104+
mapping: Mmap<AlignedLength>,
105105
/// This memory pool is stripe-aware. If using memory protection keys, this
106106
/// will contain one stripe per available key; otherwise, a single stripe
107107
/// with an empty key.

crates/wasmtime/src/runtime/vm/instance/allocator/pooling/table_pool.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ use super::{
44
};
55
use crate::prelude::*;
66
use crate::runtime::vm::{
7-
InstanceAllocationRequest, Mmap, PoolingInstanceAllocatorConfig, SendSyncPtr, Table,
7+
mmap::AlignedLength, InstanceAllocationRequest, Mmap, PoolingInstanceAllocatorConfig,
8+
SendSyncPtr, Table,
89
};
910
use crate::{runtime::vm::sys::vm::commit_pages, vm::round_usize_up_to_host_pages};
1011
use std::mem;
@@ -18,7 +19,7 @@ use wasmtime_environ::{Module, Tunables};
1819
#[derive(Debug)]
1920
pub struct TablePool {
2021
index_allocator: SimpleIndexAllocator,
21-
mapping: Mmap,
22+
mapping: Mmap<AlignedLength>,
2223
table_size: usize,
2324
max_total_tables: usize,
2425
tables_per_instance: usize,

crates/wasmtime/src/runtime/vm/instance/allocator/pooling/unix_stack_pool.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ use super::{
66
};
77
use crate::prelude::*;
88
use crate::runtime::vm::sys::vm::commit_pages;
9-
use crate::runtime::vm::{round_usize_up_to_host_pages, Mmap, PoolingInstanceAllocatorConfig};
9+
use crate::runtime::vm::{
10+
mmap::AlignedLength, round_usize_up_to_host_pages, Mmap, PoolingInstanceAllocatorConfig,
11+
};
1012

1113
/// Represents a pool of execution stacks (used for the async fiber implementation).
1214
///
@@ -20,7 +22,7 @@ use crate::runtime::vm::{round_usize_up_to_host_pages, Mmap, PoolingInstanceAllo
2022
/// from the pool.
2123
#[derive(Debug)]
2224
pub struct StackPool {
23-
mapping: Mmap,
25+
mapping: Mmap<AlignedLength>,
2426
stack_size: usize,
2527
max_stacks: usize,
2628
page_size: usize,

crates/wasmtime/src/runtime/vm/memory/mmap.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
44
use crate::prelude::*;
55
use crate::runtime::vm::memory::RuntimeLinearMemory;
6-
use crate::runtime::vm::mmap::Mmap;
6+
use crate::runtime::vm::mmap::{AlignedLength, Mmap};
77
use crate::runtime::vm::{round_usize_up_to_host_pages, usize_is_multiple_of_host_page_size};
88
use wasmtime_environ::Tunables;
99

1010
/// A linear memory instance.
1111
#[derive(Debug)]
1212
pub struct MmapMemory {
1313
// The underlying allocation.
14-
mmap: Mmap,
14+
mmap: Mmap<AlignedLength>,
1515

1616
// The current length of this Wasm memory, in bytes.
1717
//

crates/wasmtime/src/runtime/vm/mmap.rs

Lines changed: 100 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,41 +7,65 @@ use core::ops::Range;
77
#[cfg(feature = "std")]
88
use std::{fs::File, sync::Arc};
99

10-
/// A simple struct consisting of a page-aligned pointer to page-aligned
11-
/// and initially-zeroed memory and a length.
12-
#[derive(Debug)]
13-
pub struct Mmap {
14-
sys: mmap::Mmap,
10+
/// A marker type for an [`Mmap`] where both the start address and length are a
11+
/// multiple of the host page size.
12+
///
13+
/// For more information, see the documentation on [`Mmap`].
14+
#[derive(Clone, Debug)]
15+
pub struct AlignedLength {}
16+
17+
/// A type of [`Mmap`] where the start address is host page-aligned, but the
18+
/// length is possibly not a multiple of the host page size.
19+
///
20+
/// For more information, see the documentation on [`Mmap`].
21+
#[derive(Clone, Debug)]
22+
pub struct UnalignedLength {
1523
#[cfg(feature = "std")]
1624
file: Option<Arc<File>>,
1725
}
1826

19-
impl Mmap {
27+
/// A platform-independent abstraction over memory-mapped data.
28+
///
29+
/// The type parameter can be one of:
30+
///
31+
/// * [`AlignedLength`]: Both the start address and length are page-aligned
32+
/// (i.e. a multiple of the host page size). This is always the result of an
33+
/// mmap backed by anonymous memory.
34+
///
35+
/// * [`UnalignedLength`]: The start address is host page-aligned, but the
36+
/// length is not necessarily page-aligned. This is usually backed by a file,
37+
/// but can also be backed by anonymous memory.
38+
///
39+
/// ## Notes
40+
///
41+
/// If the length of a file is not a multiple of the host page size, [POSIX does
42+
/// not specify any semantics][posix-mmap] for the rest of the last page. Linux
43+
/// [does say][linux-mmap] that the rest of the page is reserved and zeroed out,
44+
/// but for portability it's best to not assume anything about the rest of
45+
/// memory. `UnalignedLength` achieves a type-level distinction between an mmap
46+
/// that is backed purely by memory, and one that is possibly backed by a file.
47+
///
48+
/// Currently, the OS-specific `mmap` implementations in this crate do not make
49+
/// this this distinction -- alignment is managed at this platform-independent
50+
/// layer. It might make sense to add this distinction to the OS-specific
51+
/// implementations in the future.
52+
///
53+
/// [posix-mmap]: https://pubs.opengroup.org/onlinepubs/9799919799/functions/mmap.html
54+
/// [linux-mmap]: https://man7.org/linux/man-pages/man2/mmap.2.html#NOTES
55+
#[derive(Debug)]
56+
pub struct Mmap<T> {
57+
sys: mmap::Mmap,
58+
data: T,
59+
}
60+
61+
impl Mmap<AlignedLength> {
2062
/// Create a new `Mmap` pointing to at least `size` bytes of page-aligned
2163
/// accessible memory.
2264
pub fn with_at_least(size: usize) -> Result<Self> {
2365
let rounded_size = crate::runtime::vm::round_usize_up_to_host_pages(size)?;
2466
Self::accessible_reserved(rounded_size, rounded_size)
2567
}
2668

27-
/// Creates a new `Mmap` by opening the file located at `path` and mapping
28-
/// it into memory.
29-
///
30-
/// The memory is mapped in read-only mode for the entire file. If portions
31-
/// of the file need to be modified then the `region` crate can be use to
32-
/// alter permissions of each page.
33-
///
34-
/// The memory mapping and the length of the file within the mapping are
35-
/// returned.
36-
#[cfg(feature = "std")]
37-
pub fn from_file(file: Arc<File>) -> Result<Self> {
38-
let sys = mmap::Mmap::from_file(&file)?;
39-
Ok(Mmap {
40-
sys,
41-
file: Some(file),
42-
})
43-
}
44-
4569
/// Create a new `Mmap` pointing to `accessible_size` bytes of page-aligned
4670
/// accessible memory, within a reserved mapping of `mapping_size` bytes.
4771
/// `accessible_size` and `mapping_size` must be native page-size multiples.
@@ -58,22 +82,19 @@ impl Mmap {
5882
if mapping_size == 0 {
5983
Ok(Mmap {
6084
sys: mmap::Mmap::new_empty(),
61-
#[cfg(feature = "std")]
62-
file: None,
85+
data: AlignedLength {},
6386
})
6487
} else if accessible_size == mapping_size {
6588
Ok(Mmap {
6689
sys: mmap::Mmap::new(mapping_size)
6790
.context(format!("mmap failed to allocate {mapping_size:#x} bytes"))?,
68-
#[cfg(feature = "std")]
69-
file: None,
91+
data: AlignedLength {},
7092
})
7193
} else {
7294
let mut result = Mmap {
7395
sys: mmap::Mmap::reserve(mapping_size)
7496
.context(format!("mmap failed to reserve {mapping_size:#x} bytes"))?,
75-
#[cfg(feature = "std")]
76-
file: None,
97+
data: AlignedLength {},
7798
};
7899
if accessible_size > 0 {
79100
result.make_accessible(0, accessible_size).context(format!(
@@ -84,6 +105,20 @@ impl Mmap {
84105
}
85106
}
86107

108+
/// Converts this `Mmap` into a `Mmap<UnalignedLength>`.
109+
///
110+
/// `UnalignedLength` really means "_possibly_ unaligned length", so it can
111+
/// be freely converted over at the cost of losing the alignment guarantee.
112+
pub fn into_unaligned(self) -> Mmap<UnalignedLength> {
113+
Mmap {
114+
sys: self.sys,
115+
data: UnalignedLength {
116+
#[cfg(feature = "std")]
117+
file: None,
118+
},
119+
}
120+
}
121+
87122
/// Make the memory starting at `start` and extending for `len` bytes
88123
/// accessible. `start` and `len` must be native page-size multiples and
89124
/// describe a range within `self`'s reserved memory.
@@ -101,7 +136,34 @@ impl Mmap {
101136

102137
self.sys.make_accessible(start, len)
103138
}
139+
}
104140

141+
#[cfg(feature = "std")]
142+
impl Mmap<UnalignedLength> {
143+
/// Creates a new `Mmap` by opening the file located at `path` and mapping
144+
/// it into memory.
145+
///
146+
/// The memory is mapped in read-only mode for the entire file. If portions
147+
/// of the file need to be modified then the `region` crate can be use to
148+
/// alter permissions of each page.
149+
///
150+
/// The memory mapping and the length of the file within the mapping are
151+
/// returned.
152+
pub fn from_file(file: Arc<File>) -> Result<Self> {
153+
let sys = mmap::Mmap::from_file(&file)?;
154+
Ok(Mmap {
155+
sys,
156+
data: UnalignedLength { file: Some(file) },
157+
})
158+
}
159+
160+
/// Returns the underlying file that this mmap is mapping, if present.
161+
pub fn original_file(&self) -> Option<&Arc<File>> {
162+
self.data.file.as_ref()
163+
}
164+
}
165+
166+
impl<T> Mmap<T> {
105167
/// Return the allocated memory as a slice of u8.
106168
///
107169
/// # Safety
@@ -198,15 +260,16 @@ impl Mmap {
198260
.make_readonly(range)
199261
.context("failed to make memory readonly")
200262
}
201-
202-
/// Returns the underlying file that this mmap is mapping, if present.
203-
#[cfg(feature = "std")]
204-
pub fn original_file(&self) -> Option<&Arc<File>> {
205-
self.file.as_ref()
206-
}
207263
}
208264

209265
fn _assert() {
210266
fn _assert_send_sync<T: Send + Sync>() {}
211-
_assert_send_sync::<Mmap>();
267+
_assert_send_sync::<Mmap<AlignedLength>>();
268+
_assert_send_sync::<Mmap<UnalignedLength>>();
269+
}
270+
271+
impl From<Mmap<AlignedLength>> for Mmap<UnalignedLength> {
272+
fn from(mmap: Mmap<AlignedLength>) -> Mmap<UnalignedLength> {
273+
mmap.into_unaligned()
274+
}
212275
}

crates/wasmtime/src/runtime/vm/mmap_vec.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::prelude::*;
22
#[cfg(feature = "signals-based-traps")]
3-
use crate::runtime::vm::Mmap;
3+
use crate::runtime::vm::{mmap::UnalignedLength, Mmap};
44
use alloc::sync::Arc;
55
use core::ops::{Deref, Range};
66
#[cfg(feature = "std")]
@@ -29,7 +29,10 @@ pub enum MmapVec {
2929
Vec(Vec<u8>),
3030
#[doc(hidden)]
3131
#[cfg(feature = "signals-based-traps")]
32-
Mmap { mmap: Mmap, len: usize },
32+
Mmap {
33+
mmap: Mmap<UnalignedLength>,
34+
len: usize,
35+
},
3336
}
3437

3538
impl MmapVec {
@@ -39,7 +42,11 @@ impl MmapVec {
3942
/// smaller than the region mapped by the `Mmap`. The returned `MmapVec`
4043
/// will only have at most `size` bytes accessible.
4144
#[cfg(feature = "signals-based-traps")]
42-
fn new_mmap(mmap: Mmap, len: usize) -> MmapVec {
45+
fn new_mmap<M>(mmap: M, len: usize) -> MmapVec
46+
where
47+
M: Into<Mmap<UnalignedLength>>,
48+
{
49+
let mmap = mmap.into();
4350
assert!(len <= mmap.len());
4451
MmapVec::Mmap { mmap, len }
4552
}

crates/wasmtime/src/runtime/vm/sys/unix/mmap.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub struct Mmap {
2020
cfg_if::cfg_if! {
2121
if #[cfg(any(target_os = "illumos", target_os = "linux"))] {
2222
// On illumos, by default, mmap reserves what it calls "swap space" ahead of time, so that
23-
// memory accesses are guaranteed not to fail once mmap succeeds. NORESERVE is for cases
23+
// memory accesses a`re guaranteed not to fail once mmap succeeds. NORESERVE is for cases
2424
// where that memory is never meant to be accessed -- e.g. memory that's used as guard
2525
// pages.
2626
//
@@ -135,6 +135,9 @@ impl Mmap {
135135

136136
#[inline]
137137
pub fn len(&self) -> usize {
138+
// Note: while the start of memory is host page-aligned, the length might
139+
// not be, and in particular is not aligned for file-backed mmaps. Be
140+
// careful!
138141
self.memory.as_ptr().len()
139142
}
140143

0 commit comments

Comments
 (0)