Skip to content

Commit c8fa2f9

Browse files
saethlinsaethlin
committed
Try storing Mappings in a RangeMap
1 parent 2ceb7b9 commit c8fa2f9

File tree

4 files changed

+103
-104
lines changed

4 files changed

+103
-104
lines changed

‎src/intptrcast.rs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,11 @@ impl<'mir, 'tcx> GlobalStateInner {
163163
}
164164

165165
fn alloc_base_addr(ecx: &MiriInterpCx<'mir, 'tcx>, alloc_id: AllocId) -> u64 {
166-
for m in ecx.machine.mappings.iter() {
167-
if m.alloc_id == alloc_id {
168-
return m.range.start.bytes();
166+
for (offset, chunk) in ecx.machine.mappings.all_memory.iter_all() {
167+
if let Some(map) = chunk {
168+
if map.alloc_id == alloc_id {
169+
return offset.bytes();
170+
}
169171
}
170172
}
171173

@@ -226,8 +228,14 @@ impl<'mir, 'tcx> GlobalStateInner {
226228
let base_addr = ecx
227229
.machine
228230
.mappings
229-
.get(offset)
230-
.map(|mapping| mapping.range.start.bytes())
231+
.all_memory
232+
.iter_all()
233+
.find_map(|(map_start, map)| {
234+
let Some(m) = map else {
235+
return None;
236+
};
237+
if m.alloc_id == alloc_id { Some(map_start.bytes()) } else { None }
238+
})
231239
.or_else(|| ecx.machine.mappings.pending.as_ref().map(|r| r.start.bytes()))
232240
.unwrap_or_else(|| GlobalStateInner::alloc_base_addr(ecx, alloc_id));
233241

‎src/machine.rs‎

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
44
use std::borrow::Cow;
55
use std::cell::RefCell;
6-
use std::cmp::Ordering;
76
use std::fmt;
87
use std::ops::Range;
98

@@ -313,62 +312,22 @@ impl<'mir, 'tcx: 'mir> PrimitiveLayouts<'tcx> {
313312
}
314313
}
315314

316-
#[derive(Clone, Debug)]
315+
#[derive(Clone, Debug, PartialEq, Eq)]
317316
pub struct Mapping {
318-
pub range: Range<Size>,
319317
pub alloc_id: AllocId,
320318
pub can_read: bool,
321319
pub can_write: bool,
322320
}
323321

324-
impl Mapping {
325-
pub fn contains(&self, addr: Size) -> bool {
326-
self.range.contains(&addr)
327-
}
328-
}
329-
330-
#[derive(Default, Debug)]
322+
#[derive(Debug)]
331323
pub struct Mappings {
332-
v: Vec<Mapping>,
324+
pub all_memory: RangeMap<Option<Mapping>>,
333325
pub pending: Option<Range<Size>>,
334326
}
335327

336328
impl Mappings {
337-
pub fn iter(&self) -> impl Iterator<Item = &Mapping> {
338-
self.v.iter()
339-
}
340-
341-
fn binary_search(&self, addr: Size) -> Result<usize, usize> {
342-
self.v.binary_search_by(|map| {
343-
if map.range.start > addr {
344-
Ordering::Greater
345-
} else if map.range.end < addr {
346-
Ordering::Less
347-
} else {
348-
Ordering::Equal
349-
}
350-
})
351-
}
352-
353-
pub fn insert(&mut self, new: Mapping) {
354-
let idx = self.binary_search(new.range.start).unwrap_err();
355-
self.v.insert(idx, new);
356-
}
357-
358-
pub fn remove(&mut self, addr: Size) -> Option<Mapping> {
359-
if !self.v.get(0)?.contains(addr) && !self.v.last()?.contains(addr) {
360-
return None;
361-
}
362-
let idx = self.binary_search(addr).ok()?;
363-
Some(self.v.remove(idx))
364-
}
365-
366-
pub fn get(&self, addr: Size) -> Option<&Mapping> {
367-
if !self.v.get(0)?.contains(addr) && !self.v.last()?.contains(addr) {
368-
return None;
369-
}
370-
let idx = self.binary_search(addr).ok()?;
371-
self.v.get(idx)
329+
fn new() -> Self {
330+
Mappings { all_memory: RangeMap::new(Size::from_bytes(u64::MAX), None), pending: None }
372331
}
373332
}
374333

@@ -529,7 +488,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
529488
argv: None,
530489
cmd_line: None,
531490
tls: TlsData::default(),
532-
mappings: Default::default(),
491+
mappings: Mappings::new(),
533492
isolated_op: config.isolated_op,
534493
validate: config.validate,
535494
enforce_abi: config.check_abi,
@@ -684,7 +643,11 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
684643
}
685644

686645
pub(crate) fn get_mapping(&self, alloc_id: AllocId) -> Option<&Mapping> {
687-
self.mappings.v.iter().find(|m| m.alloc_id == alloc_id)
646+
self.mappings
647+
.all_memory
648+
.iter_all()
649+
.filter_map(|(_offset, map)| map.as_ref())
650+
.find(|m| m.alloc_id == alloc_id)
688651
}
689652
}
690653

‎src/range_map.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ impl<T> RangeMap<T> {
9191
self.v.iter_mut().map(|elem| &mut elem.data)
9292
}
9393

94+
pub fn iter_all(&self) -> impl Iterator<Item = (Size, &T)> {
95+
self.v.iter().map(|elem| (Size::from_bytes(elem.range.start), &elem.data))
96+
}
97+
9498
// Splits the element situated at the given `index`, such that the 2nd one starts at offset
9599
// `split_offset`. Do nothing if the element already starts there.
96100
// Returns whether a split was necessary.

‎src/shims/unix/mem.rs‎

Lines changed: 75 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
8888
unreachable!("allocate_ptr should not return a Wildcard pointer"),
8989
};
9090

91-
this.machine.mappings.insert(Mapping {
92-
range: mapped_addr..mapped_addr + Size::from_bytes(map_length),
93-
alloc_id,
94-
can_read: true,
95-
can_write: true,
96-
});
91+
this.machine
92+
.mappings
93+
.all_memory
94+
.iter_mut(mapped_addr, Size::from_bytes(map_length))
95+
.for_each(|m| {
96+
*m.1 = Some(Mapping { alloc_id, can_read: true, can_write: true });
97+
});
9798

9899
this.machine.mappings.pending = None;
99100

@@ -105,10 +106,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
105106
)
106107
.unwrap();
107108

108-
let mut mapping = this.machine.mappings.remove(mapped_addr).unwrap();
109-
mapping.can_read = prot & prot_read > 0;
110-
mapping.can_write = prot & prot_write > 0;
111-
this.machine.mappings.insert(mapping);
109+
this.machine
110+
.mappings
111+
.all_memory
112+
.iter_mut(mapped_addr, Size::from_bytes(map_length))
113+
.for_each(|(_offset, map)| {
114+
if let Some(m) = map {
115+
m.can_read = prot & prot_read > 0;
116+
m.can_write = prot & prot_write > 0;
117+
}
118+
});
112119

113120
Ok(Some(ptr))
114121
} else {
@@ -128,8 +135,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
128135
) -> InterpResult<'tcx, Pointer<Option<Provenance>>> {
129136
let this = self.eval_context_mut();
130137

131-
let old_address = this.read_pointer(old_address)?;
132-
let _old_size = this.read_scalar(old_size)?.to_machine_usize(this)?;
138+
// This is actually a pointer but we really do not care about its provenance
139+
let old_address = this.read_scalar(old_address)?.to_machine_usize(this)?;
140+
let old_address = Size::from_bytes(old_address);
141+
142+
let old_size = this.read_scalar(old_size)?.to_machine_usize(this)?;
133143
let new_size = this.read_scalar(new_size)?.to_machine_usize(this)?;
134144
let flags = this.read_scalar(flags)?.to_i32()?;
135145

@@ -147,19 +157,37 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
147157
return Ok(Pointer::null());
148158
}
149159

150-
let (_, offset) = old_address.into_parts();
160+
// Remove all mappings associated with the range specified by old_address and old_size
161+
this.machine
162+
.mappings
163+
.all_memory
164+
.iter_mut(old_address, Size::from_bytes(old_size))
165+
.for_each(|(_offset, map)| {
166+
*map = None;
167+
});
151168

152-
if let Some(mut map) = this.machine.mappings.remove(offset) {
153-
let pointer = this.realloc(old_address, new_size, MiriMemoryKind::Mmap)?;
154-
let (_, offset) = pointer.into_parts();
155-
map.range = offset..offset + Size::from_bytes(new_size);
156-
this.machine.mappings.insert(map);
157-
Ok(pointer)
158-
} else {
159-
// This isn't a previous mapping
160-
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")?))?;
161-
Ok(Pointer::null())
162-
}
169+
let pointer =
170+
this.realloc(Pointer::new(None, old_address), new_size, MiriMemoryKind::Mmap)?;
171+
172+
let (prov, offset) = pointer.into_parts();
173+
let alloc_id = match prov {
174+
Some(Provenance::Concrete { alloc_id, sb: _ }) => alloc_id,
175+
_ => panic!("realloc should return a pointer with provenance"),
176+
};
177+
let start =
178+
intptrcast::GlobalStateInner::rel_ptr_to_addr(this, Pointer::new(alloc_id, offset));
179+
this.machine
180+
.mappings
181+
.all_memory
182+
.iter_mut(Size::from_bytes(start), Size::from_bytes(new_size))
183+
.take(1) // "an existing mapping" meaning only one mapping starting at old_address?
184+
.for_each(|(_offset, map)| {
185+
if let Some(m) = map {
186+
m.alloc_id = alloc_id;
187+
}
188+
});
189+
190+
Ok(pointer)
163191
}
164192

165193
fn munmap(
@@ -169,32 +197,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
169197
) -> InterpResult<'tcx, i32> {
170198
let this = self.eval_context_mut();
171199

172-
let addr = this.read_pointer(addr)?;
200+
let addr = this.read_scalar(addr)?.to_machine_usize(this)?;
173201
let length = this.read_scalar(length)?.to_machine_usize(this)?;
174202

175203
// The address addr must be a multiple of the page size (but length need not be).
176204
#[allow(clippy::integer_arithmetic)] // PAGE_SIZE is nonzero
177-
if addr.addr().bytes() % PAGE_SIZE != 0 {
205+
if addr % PAGE_SIZE != 0 {
178206
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")?))?;
179207
return Ok(-1);
180208
}
181209

182210
// All pages containing a part of the indicated range are unmapped.
183-
// TODO: That means we can actually alter multiple mappings with munmap :/
184211
let length = round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX);
185212

186-
let (_, offset) = addr.into_parts();
187-
188-
if let Some(map) = this.machine.mappings.get(offset) {
189-
if map.range == (offset..offset + Size::from_bytes(length)) {
190-
this.machine.mappings.remove(offset);
191-
this.free(addr, MiriMemoryKind::Mmap)?;
192-
} else {
193-
throw_unsup_format!("Miri does not support partial munmap");
194-
}
213+
for (_offset, map) in this
214+
.machine
215+
.mappings
216+
.all_memory
217+
.iter_mut(Size::from_bytes(addr), Size::from_bytes(length))
218+
{
219+
*map = None;
195220
}
196221
Ok(0)
197-
// It is not an error if the indicated range does not contain any mapped pages.
198222
}
199223

200224
fn mprotect(
@@ -204,28 +228,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
204228
prot: &OpTy<'tcx, Provenance>,
205229
) -> InterpResult<'tcx, i32> {
206230
let this = self.eval_context_mut();
207-
let addr = this.read_pointer(addr)?;
231+
let addr = this.read_scalar(addr)?.to_machine_usize(this)?;
208232
let length = this.read_scalar(length)?.to_machine_usize(this)?;
209233
let prot = this.read_scalar(prot)?.to_i32()?;
210234

211235
let prot_read = this.eval_libc_i32("PROT_READ")?;
212236
let prot_write = this.eval_libc_i32("PROT_WRITE")?;
213237

214-
let (_, offset) = addr.into_parts();
238+
// All pages containing a part of the indicated range are modified
239+
let length = round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX);
215240

216-
if let Some(mut map) = this.machine.mappings.remove(offset) {
217-
if map.range == (offset..offset + Size::from_bytes(length)) {
218-
map.can_read = prot & prot_read > 0;
219-
map.can_write = prot & prot_write > 0;
220-
} else {
221-
throw_unsup_format!("Miri does not support partial mprotect");
241+
for (_offset, map) in this
242+
.machine
243+
.mappings
244+
.all_memory
245+
.iter_mut(Size::from_bytes(addr), Size::from_bytes(length))
246+
{
247+
if let Some(m) = map {
248+
m.can_read = prot & prot_read > 0;
249+
m.can_write = prot & prot_write > 0;
222250
}
223-
this.machine.mappings.insert(map);
224-
Ok(0)
225-
} else {
226-
let einval = this.eval_libc("EINVAL")?;
227-
this.set_last_error(einval)?;
228-
Ok(-1)
229251
}
252+
253+
Ok(0)
230254
}
231255
}

0 commit comments

Comments
 (0)