Skip to content

Dereference pointers in shims as correct types #2661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion 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/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
49 changes: 41 additions & 8 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,31 +730,63 @@ 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)
}

fn deref_pointer_as(
&self,
val: &ImmTy<'tcx, Provenance>,
layout: TyAndLayout<'tcx>,
) -> InterpResult<'tcx, MPlaceTy<'tcx, Provenance>> {
let this = self.eval_context_ref();
let mut mplace = this.ref_to_mplace(val)?;

mplace.layout = layout;
mplace.align = layout.align.abi;

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 +795,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/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/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/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/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/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/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: 4 additions & 2 deletions src/shims/unix/linux/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ 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 = this.deref_pointer_as(
&this.read_immediate(&args[3])?,
this.libc_ty_layout("timespec"),
)?;
let timeout_time = if this.ptr_is_null(timeout.ptr)? {
None
} else {
Expand Down
Loading