Skip to content

Commit

Permalink
deref shim arguments with actual ty instead of declared ty
Browse files Browse the repository at this point in the history
  • Loading branch information
beepster4096 committed Jun 2, 2023
1 parent 507055b commit 13c20f2
Show file tree
Hide file tree
Showing 14 changed files with 215 additions and 126 deletions.
6 changes: 5 additions & 1 deletion src/tools/miri/src/concurrency/init_once.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::VecDeque;
use std::num::NonZeroU32;

use rustc_index::Idx;
use rustc_middle::ty::layout::TyAndLayout;

use super::sync::EvalContextExtPriv as _;
use super::thread::MachineCallback;
Expand Down Expand Up @@ -94,10 +95,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn init_once_get_or_create_id(
&mut self,
lock_op: &OpTy<'tcx, Provenance>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
) -> InterpResult<'tcx, InitOnceId> {
let this = self.eval_context_mut();
this.init_once_get_or_create(|ecx, next_id| ecx.get_or_create_id(next_id, lock_op, offset))
this.init_once_get_or_create(|ecx, next_id| {
ecx.get_or_create_id(next_id, lock_op, lock_layout, offset)
})
}

/// Provides the closure with the next InitOnceId. Creates that InitOnce if the closure returns None,
Expand Down
19 changes: 15 additions & 4 deletions src/tools/miri/src/concurrency/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use log::trace;

use rustc_data_structures::fx::FxHashMap;
use rustc_index::{Idx, IndexVec};
use rustc_middle::ty::layout::TyAndLayout;

use super::init_once::InitOnce;
use super::vector_clock::VClock;
Expand Down Expand Up @@ -200,11 +201,12 @@ pub(super) trait EvalContextExtPriv<'mir, 'tcx: 'mir>:
&mut self,
next_id: Id,
lock_op: &OpTy<'tcx, Provenance>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
) -> InterpResult<'tcx, Option<Id>> {
let this = self.eval_context_mut();
let value_place =
this.deref_operand_and_offset(lock_op, offset, this.machine.layouts.u32)?;
this.deref_operand_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?;

// Since we are lazy, this update has to be atomic.
let (old, success) = this
Expand Down Expand Up @@ -278,28 +280,37 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn mutex_get_or_create_id(
&mut self,
lock_op: &OpTy<'tcx, Provenance>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
) -> InterpResult<'tcx, MutexId> {
let this = self.eval_context_mut();
this.mutex_get_or_create(|ecx, next_id| ecx.get_or_create_id(next_id, lock_op, offset))
this.mutex_get_or_create(|ecx, next_id| {
ecx.get_or_create_id(next_id, lock_op, lock_layout, offset)
})
}

fn rwlock_get_or_create_id(
&mut self,
lock_op: &OpTy<'tcx, Provenance>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
) -> InterpResult<'tcx, RwLockId> {
let this = self.eval_context_mut();
this.rwlock_get_or_create(|ecx, next_id| ecx.get_or_create_id(next_id, lock_op, offset))
this.rwlock_get_or_create(|ecx, next_id| {
ecx.get_or_create_id(next_id, lock_op, lock_layout, offset)
})
}

fn condvar_get_or_create_id(
&mut self,
lock_op: &OpTy<'tcx, Provenance>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
) -> InterpResult<'tcx, CondvarId> {
let this = self.eval_context_mut();
this.condvar_get_or_create(|ecx, next_id| ecx.get_or_create_id(next_id, lock_op, offset))
this.condvar_get_or_create(|ecx, next_id| {
ecx.get_or_create_id(next_id, lock_op, lock_layout, offset)
})
}

#[inline]
Expand Down
35 changes: 27 additions & 8 deletions src/tools/miri/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,31 +730,49 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
}

/// Dereference a pointer operand to a place using `layout` instead of the pointer's declared type
fn deref_operand_as(
&self,
op: &OpTy<'tcx, Provenance>,
layout: TyAndLayout<'tcx>,
) -> InterpResult<'tcx, MPlaceTy<'tcx, Provenance>> {
let this = self.eval_context_ref();
let ptr = this.read_pointer(op)?;

let mplace = MPlaceTy::from_aligned_ptr(ptr, layout);

this.check_mplace(mplace)?;

Ok(mplace)
}

/// Calculates the MPlaceTy given the offset and layout of an access on an operand
fn deref_operand_and_offset(
&self,
op: &OpTy<'tcx, Provenance>,
offset: u64,
layout: TyAndLayout<'tcx>,
base_layout: TyAndLayout<'tcx>,
value_layout: TyAndLayout<'tcx>,
) -> InterpResult<'tcx, MPlaceTy<'tcx, Provenance>> {
let this = self.eval_context_ref();
let op_place = this.deref_operand(op)?; // FIXME: we still deref with the original type!
let op_place = this.deref_operand_as(op, base_layout)?;
let offset = Size::from_bytes(offset);

// Ensure that the access is within bounds.
assert!(op_place.layout.size >= offset + layout.size);
let value_place = op_place.offset(offset, layout, this)?;
assert!(base_layout.size >= offset + value_layout.size);
let value_place = op_place.offset(offset, value_layout, this)?;
Ok(value_place)
}

fn read_scalar_at_offset(
&self,
op: &OpTy<'tcx, Provenance>,
offset: u64,
layout: TyAndLayout<'tcx>,
base_layout: TyAndLayout<'tcx>,
value_layout: TyAndLayout<'tcx>,
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_ref();
let value_place = this.deref_operand_and_offset(op, offset, layout)?;
let value_place = this.deref_operand_and_offset(op, offset, base_layout, value_layout)?;
this.read_scalar(&value_place.into())
}

Expand All @@ -763,10 +781,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
op: &OpTy<'tcx, Provenance>,
offset: u64,
value: impl Into<Scalar<Provenance>>,
layout: TyAndLayout<'tcx>,
base_layout: TyAndLayout<'tcx>,
value_layout: TyAndLayout<'tcx>,
) -> InterpResult<'tcx, ()> {
let this = self.eval_context_mut();
let value_place = this.deref_operand_and_offset(op, offset, layout)?;
let value_place = this.deref_operand_and_offset(op, offset, base_layout, value_layout)?;
this.write_scalar(value, &value_place.into())
}

Expand Down
6 changes: 4 additions & 2 deletions src/tools/miri/src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,14 +409,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// &mut self,
// arg1: &OpTy<'tcx, Provenance>,
// arg2: &OpTy<'tcx, Provenance>,
// arg3: &OpTy<'tcx, Provenance>)
// arg3: &OpTy<'tcx, Provenance>,
// arg4: &OpTy<'tcx, Provenance>)
// -> InterpResult<'tcx, Scalar<Provenance>> {
// let this = self.eval_context_mut();
//
// // First thing: load all the arguments. Details depend on the shim.
// let arg1 = this.read_scalar(arg1)?.to_u32()?;
// let arg2 = this.read_pointer(arg2)?; // when you need to work with the pointer directly
// let arg3 = this.deref_operand(arg3)?; // when you want to load/store through the pointer at its declared type
// let arg3 = this.deref_operand(arg3)?; // when you want to load/store through the pointer
// let arg4 = this.deref_operand_as(arg4, this.libc_ty_layout("some_libc_struct")?)
//
// // ...
//
Expand Down
22 changes: 13 additions & 9 deletions src/tools/miri/src/shims/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.assert_target_os_is_unix("clock_gettime");

let clk_id = this.read_scalar(clk_id_op)?.to_i32()?;
let tp = this.deref_operand_as(tp_op, this.libc_ty_layout("timespec"))?;

let absolute_clocks;
let mut relative_clocks;
Expand Down Expand Up @@ -76,7 +77,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let tv_sec = duration.as_secs();
let tv_nsec = duration.subsec_nanos();

this.write_int_fields(&[tv_sec.into(), tv_nsec.into()], &this.deref_operand(tp_op)?)?;
this.write_int_fields(&[tv_sec.into(), tv_nsec.into()], &tp)?;

Ok(Scalar::from_i32(0))
}
Expand All @@ -91,6 +92,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.assert_target_os_is_unix("gettimeofday");
this.check_no_isolation("`gettimeofday`")?;

let tv = this.deref_operand_as(tv_op, this.libc_ty_layout("timeval"))?;

// Using tz is obsolete and should always be null
let tz = this.read_pointer(tz_op)?;
if !this.ptr_is_null(tz)? {
Expand All @@ -103,7 +106,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let tv_sec = duration.as_secs();
let tv_usec = duration.subsec_micros();

this.write_int_fields(&[tv_sec.into(), tv_usec.into()], &this.deref_operand(tv_op)?)?;
this.write_int_fields(&[tv_sec.into(), tv_usec.into()], &tv)?;

Ok(0)
}
Expand All @@ -118,6 +121,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.assert_target_os("windows", "GetSystemTimeAsFileTime");
this.check_no_isolation("`GetSystemTimeAsFileTime`")?;

let filetime = this.deref_operand_as(LPFILETIME_op, this.windows_ty_layout("FILETIME"))?;

let NANOS_PER_SEC = this.eval_windows_u64("time", "NANOS_PER_SEC");
let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC");
let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH");
Expand All @@ -131,10 +136,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

let dwLowDateTime = u32::try_from(duration_ticks & 0x00000000FFFFFFFF).unwrap();
let dwHighDateTime = u32::try_from((duration_ticks & 0xFFFFFFFF00000000) >> 32).unwrap();
this.write_int_fields(
&[dwLowDateTime.into(), dwHighDateTime.into()],
&this.deref_operand(LPFILETIME_op)?,
)?;
this.write_int_fields(&[dwLowDateTime.into(), dwHighDateTime.into()], &filetime)?;

Ok(())
}
Expand Down Expand Up @@ -177,7 +179,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// and thus 10^9 counts per second.
this.write_scalar(
Scalar::from_i64(1_000_000_000),
&this.deref_operand(lpFrequency_op)?.into(),
&this.deref_operand_as(lpFrequency_op, this.machine.layouts.u64)?.into(),
)?;
Ok(Scalar::from_i32(-1)) // Return non-zero on success
}
Expand All @@ -204,7 +206,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

this.assert_target_os("macos", "mach_timebase_info");

let info = this.deref_operand(info_op)?;
let info = this.deref_operand_as(info_op, this.libc_ty_layout("mach_timebase_info"))?;

// Since our emulated ticks in `mach_absolute_time` *are* nanoseconds,
// no scaling needs to happen.
Expand All @@ -223,7 +225,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

this.assert_target_os_is_unix("nanosleep");

let duration = match this.read_timespec(&this.deref_operand(req_op)?)? {
let req = this.deref_operand_as(req_op, this.libc_ty_layout("timespec"))?;

let duration = match this.read_timespec(&req)? {
Some(duration) => duration,
None => {
let einval = this.eval_libc("EINVAL");
Expand Down
6 changes: 3 additions & 3 deletions src/tools/miri/src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Thread-local storage
"pthread_key_create" => {
let [key, dtor] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let key_place = this.deref_operand(key)?;
let key_place = this.deref_operand_as(key, this.libc_ty_layout("pthread_key_t"))?;
let dtor = this.read_pointer(dtor)?;

// Extract the function type out of the signature (that seems easier than constructing it ourselves).
Expand Down Expand Up @@ -520,7 +520,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Hence we can mostly ignore the input `attr_place`.
let [attr_place, addr_place, size_place] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let _attr_place = this.deref_operand(attr_place)?;
let _attr_place = this.deref_operand_as(attr_place, this.libc_ty_layout("pthread_attr_t"))?;
let addr_place = this.deref_operand(addr_place)?;
let size_place = this.deref_operand(size_place)?;

Expand Down Expand Up @@ -563,7 +563,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.check_no_isolation("`getpwuid_r`")?;

let uid = this.read_scalar(uid)?.to_u32()?;
let pwd = this.deref_operand(pwd)?;
let pwd = this.deref_operand_as(pwd, this.libc_ty_layout("passwd"))?;
let buf = this.read_pointer(buf)?;
let buflen = this.read_target_usize(buflen)?;
let result = this.deref_operand(result)?;
Expand Down
17 changes: 4 additions & 13 deletions src/tools/miri/src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,8 @@ trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx
let (created_sec, created_nsec) = metadata.created.unwrap_or((0, 0));
let (modified_sec, modified_nsec) = metadata.modified.unwrap_or((0, 0));

let buf = this.deref_operand(buf_op)?;
let buf = this.deref_operand_as(buf_op, this.libc_ty_layout("stat"))?;

this.write_int_fields_named(
&[
("st_dev", 0),
Expand Down Expand Up @@ -1014,15 +1015,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
return Ok(-1);
}

// Under normal circumstances, we would use `deref_operand(statxbuf_op)` to produce a
// proper `MemPlace` and then write the results of this function to it. However, the
// `syscall` function is untyped. This means that all the `statx` parameters are provided
// as `isize`s instead of having the proper types. Thus, we have to recover the layout of
// `statxbuf_op` by using the `libc::statx` struct type.
let statxbuf = {
let statx_layout = this.libc_ty_layout("statx");
MPlaceTy::from_aligned_ptr(statxbuf_ptr, statx_layout)
};
let statxbuf = this.deref_operand_as(statxbuf_op, this.libc_ty_layout("statx"))?;

let path = this.read_path_from_c_str(pathname_ptr)?.into_owned();
// See <https://github.com/rust-lang/rust/pull/79196> for a discussion of argument sizes.
Expand Down Expand Up @@ -1428,7 +1421,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// pub d_name: [c_char; 1024],
// }

let entry_place = this.deref_operand(entry_op)?;
let entry_place = this.deref_operand_as(entry_op, this.libc_ty_layout("dirent"))?;
let name_place = this.mplace_field(&entry_place, 5)?;

let file_name = dir_entry.file_name(); // not a Path as there are no separators!
Expand All @@ -1444,8 +1437,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
);
}

let entry_place = this.deref_operand(entry_op)?;

// If the host is a Unix system, fill in the inode number with its real value.
// If not, use 0 as a fallback value.
#[cfg(unix)]
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/linux/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let epoll_ctl_del = this.eval_libc_i32("EPOLL_CTL_DEL");

if op == epoll_ctl_add || op == epoll_ctl_mod {
let event = this.deref_operand(event)?;
let event = this.deref_operand_as(event, this.libc_ty_layout("epoll_event"))?;

let events = this.mplace_field(&event, 0)?;
let events = this.read_scalar(&events.into())?.to_u32()?;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/linux/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
this.read_scalar(pid)?.to_i32()?;
this.read_target_usize(cpusetsize)?;
this.deref_operand(mask)?;
this.deref_operand_as(mask, this.libc_ty_layout("cpu_set_t"))?;
// FIXME: we just return an error; `num_cpus` then falls back to `sysconf`.
let einval = this.eval_libc("EINVAL");
this.set_last_error(einval)?;
Expand Down
6 changes: 3 additions & 3 deletions src/tools/miri/src/shims/unix/linux/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,8 @@ pub fn futex<'tcx>(
return Ok(());
}

// `deref_operand` but not actually dereferencing the ptr yet (it might be NULL!).
let timeout = this.ref_to_mplace(&this.read_immediate(&args[3])?)?;
let timeout_time = if this.ptr_is_null(timeout.ptr)? {
let timeout_op = &args[3];
let timeout_time = if this.ptr_is_null(this.read_pointer(timeout_op)?)? {
None
} else {
let realtime = op & futex_realtime == futex_realtime;
Expand All @@ -96,6 +95,7 @@ pub fn futex<'tcx>(
"`futex` syscall with `op=FUTEX_WAIT` and non-null timeout with `FUTEX_CLOCK_REALTIME`",
)?;
}
let timeout = this.deref_operand_as(timeout_op, this.libc_ty_layout("timespec"))?;
let duration = match this.read_timespec(&timeout)? {
Some(duration) => duration,
None => {
Expand Down
Loading

0 comments on commit 13c20f2

Please sign in to comment.