Skip to content

Commit

Permalink
Try storing Mappings in a RangeMap
Browse files Browse the repository at this point in the history
  • Loading branch information
@saethlin
saethlin committed Oct 12, 2022
1 parent 2ceb7b9 commit c8fa2f9
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 104 deletions.
18 changes: 13 additions & 5 deletions src/intptrcast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,11 @@ impl<'mir, 'tcx> GlobalStateInner {
}

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

Expand Down Expand Up @@ -226,8 +228,14 @@ impl<'mir, 'tcx> GlobalStateInner {
let base_addr = ecx
.machine
.mappings
.get(offset)
.map(|mapping| mapping.range.start.bytes())
.all_memory
.iter_all()
.find_map(|(map_start, map)| {
let Some(m) = map else {
return None;
};
if m.alloc_id == alloc_id { Some(map_start.bytes()) } else { None }
})
.or_else(|| ecx.machine.mappings.pending.as_ref().map(|r| r.start.bytes()))
.unwrap_or_else(|| GlobalStateInner::alloc_base_addr(ecx, alloc_id));

Expand Down
59 changes: 11 additions & 48 deletions src/machine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use std::borrow::Cow;
use std::cell::RefCell;
use std::cmp::Ordering;
use std::fmt;
use std::ops::Range;

Expand Down Expand Up @@ -313,62 +312,22 @@ impl<'mir, 'tcx: 'mir> PrimitiveLayouts<'tcx> {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Mapping {
pub range: Range<Size>,
pub alloc_id: AllocId,
pub can_read: bool,
pub can_write: bool,
}

impl Mapping {
pub fn contains(&self, addr: Size) -> bool {
self.range.contains(&addr)
}
}

#[derive(Default, Debug)]
#[derive(Debug)]
pub struct Mappings {
v: Vec<Mapping>,
pub all_memory: RangeMap<Option<Mapping>>,
pub pending: Option<Range<Size>>,
}

impl Mappings {
pub fn iter(&self) -> impl Iterator<Item = &Mapping> {
self.v.iter()
}

fn binary_search(&self, addr: Size) -> Result<usize, usize> {
self.v.binary_search_by(|map| {
if map.range.start > addr {
Ordering::Greater
} else if map.range.end < addr {
Ordering::Less
} else {
Ordering::Equal
}
})
}

pub fn insert(&mut self, new: Mapping) {
let idx = self.binary_search(new.range.start).unwrap_err();
self.v.insert(idx, new);
}

pub fn remove(&mut self, addr: Size) -> Option<Mapping> {
if !self.v.get(0)?.contains(addr) && !self.v.last()?.contains(addr) {
return None;
}
let idx = self.binary_search(addr).ok()?;
Some(self.v.remove(idx))
}

pub fn get(&self, addr: Size) -> Option<&Mapping> {
if !self.v.get(0)?.contains(addr) && !self.v.last()?.contains(addr) {
return None;
}
let idx = self.binary_search(addr).ok()?;
self.v.get(idx)
fn new() -> Self {
Mappings { all_memory: RangeMap::new(Size::from_bytes(u64::MAX), None), pending: None }
}
}

Expand Down Expand Up @@ -529,7 +488,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
argv: None,
cmd_line: None,
tls: TlsData::default(),
mappings: Default::default(),
mappings: Mappings::new(),
isolated_op: config.isolated_op,
validate: config.validate,
enforce_abi: config.check_abi,
Expand Down Expand Up @@ -684,7 +643,11 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
}

pub(crate) fn get_mapping(&self, alloc_id: AllocId) -> Option<&Mapping> {
self.mappings.v.iter().find(|m| m.alloc_id == alloc_id)
self.mappings
.all_memory
.iter_all()
.filter_map(|(_offset, map)| map.as_ref())
.find(|m| m.alloc_id == alloc_id)
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/range_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ impl<T> RangeMap<T> {
self.v.iter_mut().map(|elem| &mut elem.data)
}

pub fn iter_all(&self) -> impl Iterator<Item = (Size, &T)> {
self.v.iter().map(|elem| (Size::from_bytes(elem.range.start), &elem.data))
}

// Splits the element situated at the given `index`, such that the 2nd one starts at offset
// `split_offset`. Do nothing if the element already starts there.
// Returns whether a split was necessary.
Expand Down
126 changes: 75 additions & 51 deletions src/shims/unix/mem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
unreachable!("allocate_ptr should not return a Wildcard pointer"),
};

this.machine.mappings.insert(Mapping {
range: mapped_addr..mapped_addr + Size::from_bytes(map_length),
alloc_id,
can_read: true,
can_write: true,
});
this.machine
.mappings
.all_memory
.iter_mut(mapped_addr, Size::from_bytes(map_length))
.for_each(|m| {
*m.1 = Some(Mapping { alloc_id, can_read: true, can_write: true });
});

this.machine.mappings.pending = None;

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

let mut mapping = this.machine.mappings.remove(mapped_addr).unwrap();
mapping.can_read = prot & prot_read > 0;
mapping.can_write = prot & prot_write > 0;
this.machine.mappings.insert(mapping);
this.machine
.mappings
.all_memory
.iter_mut(mapped_addr, Size::from_bytes(map_length))
.for_each(|(_offset, map)| {
if let Some(m) = map {
m.can_read = prot & prot_read > 0;
m.can_write = prot & prot_write > 0;
}
});

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

let old_address = this.read_pointer(old_address)?;
let _old_size = this.read_scalar(old_size)?.to_machine_usize(this)?;
// This is actually a pointer but we really do not care about its provenance
let old_address = this.read_scalar(old_address)?.to_machine_usize(this)?;
let old_address = Size::from_bytes(old_address);

let old_size = this.read_scalar(old_size)?.to_machine_usize(this)?;
let new_size = this.read_scalar(new_size)?.to_machine_usize(this)?;
let flags = this.read_scalar(flags)?.to_i32()?;

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

let (_, offset) = old_address.into_parts();
// Remove all mappings associated with the range specified by old_address and old_size
this.machine
.mappings
.all_memory
.iter_mut(old_address, Size::from_bytes(old_size))
.for_each(|(_offset, map)| {
*map = None;
});

if let Some(mut map) = this.machine.mappings.remove(offset) {
let pointer = this.realloc(old_address, new_size, MiriMemoryKind::Mmap)?;
let (_, offset) = pointer.into_parts();
map.range = offset..offset + Size::from_bytes(new_size);
this.machine.mappings.insert(map);
Ok(pointer)
} else {
// This isn't a previous mapping
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")?))?;
Ok(Pointer::null())
}
let pointer =
this.realloc(Pointer::new(None, old_address), new_size, MiriMemoryKind::Mmap)?;

let (prov, offset) = pointer.into_parts();
let alloc_id = match prov {
Some(Provenance::Concrete { alloc_id, sb: _ }) => alloc_id,
_ => panic!("realloc should return a pointer with provenance"),
};
let start =
intptrcast::GlobalStateInner::rel_ptr_to_addr(this, Pointer::new(alloc_id, offset));
this.machine
.mappings
.all_memory
.iter_mut(Size::from_bytes(start), Size::from_bytes(new_size))
.take(1) // "an existing mapping" meaning only one mapping starting at old_address?
.for_each(|(_offset, map)| {
if let Some(m) = map {
m.alloc_id = alloc_id;
}
});

Ok(pointer)
}

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

let addr = this.read_pointer(addr)?;
let addr = this.read_scalar(addr)?.to_machine_usize(this)?;
let length = this.read_scalar(length)?.to_machine_usize(this)?;

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

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

let (_, offset) = addr.into_parts();

if let Some(map) = this.machine.mappings.get(offset) {
if map.range == (offset..offset + Size::from_bytes(length)) {
this.machine.mappings.remove(offset);
this.free(addr, MiriMemoryKind::Mmap)?;
} else {
throw_unsup_format!("Miri does not support partial munmap");
}
for (_offset, map) in this
.machine
.mappings
.all_memory
.iter_mut(Size::from_bytes(addr), Size::from_bytes(length))
{
*map = None;
}
Ok(0)
// It is not an error if the indicated range does not contain any mapped pages.
}

fn mprotect(
Expand All @@ -204,28 +228,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
prot: &OpTy<'tcx, Provenance>,
) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let addr = this.read_pointer(addr)?;
let addr = this.read_scalar(addr)?.to_machine_usize(this)?;
let length = this.read_scalar(length)?.to_machine_usize(this)?;
let prot = this.read_scalar(prot)?.to_i32()?;

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

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

if let Some(mut map) = this.machine.mappings.remove(offset) {
if map.range == (offset..offset + Size::from_bytes(length)) {
map.can_read = prot & prot_read > 0;
map.can_write = prot & prot_write > 0;
} else {
throw_unsup_format!("Miri does not support partial mprotect");
for (_offset, map) in this
.machine
.mappings
.all_memory
.iter_mut(Size::from_bytes(addr), Size::from_bytes(length))
{
if let Some(m) = map {
m.can_read = prot & prot_read > 0;
m.can_write = prot & prot_write > 0;
}
this.machine.mappings.insert(map);
Ok(0)
} else {
let einval = this.eval_libc("EINVAL")?;
this.set_last_error(einval)?;
Ok(-1)
}

Ok(0)
}
}

0 comments on commit c8fa2f9

Please sign in to comment.