Skip to content

Commit

Permalink
Auto merge of #3053 - RalfJung:rustup, r=RalfJung
Browse files Browse the repository at this point in the history
Rustup
  • Loading branch information
bors committed Sep 6, 2023
2 parents 4f5d951 + 9750d69 commit aae7597
Show file tree
Hide file tree
Showing 25 changed files with 93 additions and 103 deletions.
4 changes: 2 additions & 2 deletions miri-script/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ impl Command {
cmd.stdout(process::Stdio::null());
cmd.stderr(process::Stdio::null());
let josh = cmd.spawn().context("failed to start josh-proxy, make sure it is installed")?;
// Give it some time so hopefully the port is open. (10ms was not enough.)
thread::sleep(time::Duration::from_millis(100));
// Give it some time so hopefully the port is open. (100ms was not enough.)
thread::sleep(time::Duration::from_millis(200));

// Create a wrapper that stops it on drop.
struct Josh(process::Child);
Expand Down
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
a989e25f1b87949a886eab3da10324d14189fe95
aeddd2ddfd35bb385dd36476fab326f40d57a131
10 changes: 5 additions & 5 deletions src/borrow_tracker/stacked_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
) -> InterpResult<'tcx, Option<Provenance>> {
let this = self.eval_context_mut();
// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
this.check_ptr_access_align(place.ptr, size, Align::ONE, CheckInAllocMsg::InboundsTest)?;
this.check_ptr_access_align(place.ptr(), size, Align::ONE, CheckInAllocMsg::InboundsTest)?;

// It is crucial that this gets called on all code paths, to ensure we track tag creation.
let log_creation = |this: &MiriInterpCx<'mir, 'tcx>,
Expand Down Expand Up @@ -682,7 +682,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
trace!(
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
new_tag,
place.ptr,
place.ptr(),
place.layout.ty,
);
// Don't update any stacks for a zero-sized access; borrow stacks are per-byte and this
Expand All @@ -692,19 +692,19 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
// attempt to use it for a non-zero-sized access.
// Dangling slices are a common case here; it's valid to get their length but with raw
// pointer tagging for example all calls to get_unchecked on them are invalid.
if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr) {
if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr()) {
log_creation(this, Some((alloc_id, base_offset, orig_tag)))?;
// Still give it the new provenance, it got retagged after all.
return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag }));
} else {
// This pointer doesn't come with an AllocId. :shrug:
log_creation(this, None)?;
// Provenance unchanged.
return Ok(place.ptr.provenance);
return Ok(place.ptr().provenance);
}
}

let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?;
let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr())?;
log_creation(this, Some((alloc_id, base_offset, orig_tag)))?;

trace!(
Expand Down
10 changes: 5 additions & 5 deletions src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
let this = self.eval_context_mut();
// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
this.check_ptr_access_align(
place.ptr,
place.ptr(),
ptr_size,
Align::ONE,
CheckInAllocMsg::InboundsTest,
Expand All @@ -197,7 +197,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
};

trace!("Reborrow of size {:?}", ptr_size);
let (alloc_id, base_offset, parent_prov) = match this.ptr_try_get_alloc_id(place.ptr) {
let (alloc_id, base_offset, parent_prov) = match this.ptr_try_get_alloc_id(place.ptr()) {
Ok(data) => {
// Unlike SB, we *do* a proper retag for size 0 if can identify the allocation.
// After all, the pointer may be lazily initialized outside this initial range.
Expand All @@ -210,18 +210,18 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
trace!(
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
new_tag,
place.ptr,
place.ptr(),
place.layout.ty,
);
log_creation(this, None)?;
// Keep original provenance.
return Ok(place.ptr.provenance);
return Ok(place.ptr().provenance);
}
};
log_creation(this, Some((alloc_id, base_offset, parent_prov)))?;

let orig_tag = match parent_prov {
ProvenanceExtra::Wildcard => return Ok(place.ptr.provenance), // TODO: handle wildcard pointers
ProvenanceExtra::Wildcard => return Ok(place.ptr().provenance), // TODO: handle wildcard pointers
ProvenanceExtra::Concrete(tag) => tag,
};

Expand Down
13 changes: 7 additions & 6 deletions src/concurrency/data_race.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
// be 8-aligned).
let align = Align::from_bytes(place.layout.size.bytes()).unwrap();
this.check_ptr_access_align(
place.ptr,
place.ptr(),
place.layout.size,
align,
CheckInAllocMsg::MemoryAccessTest,
Expand All @@ -1030,8 +1030,9 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
// <https://github.com/rust-lang/miri/pull/2464#discussion_r939636130> for details.
// We avoid `get_ptr_alloc` since we do *not* want to run the access hooks -- the actual
// access will happen later.
let (alloc_id, _offset, _prov) =
this.ptr_try_get_alloc_id(place.ptr).expect("there are no zero-sized atomic accesses");
let (alloc_id, _offset, _prov) = this
.ptr_try_get_alloc_id(place.ptr())
.expect("there are no zero-sized atomic accesses");
if this.get_alloc_mutability(alloc_id)? == Mutability::Not {
// FIXME: make this prettier, once these messages have separate title/span/help messages.
throw_ub_format!(
Expand Down Expand Up @@ -1135,15 +1136,15 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
if let Some(data_race) = &this.machine.data_race {
if data_race.race_detecting() {
let size = place.layout.size;
let (alloc_id, base_offset, _prov) = this.ptr_get_alloc_id(place.ptr)?;
let (alloc_id, base_offset, _prov) = this.ptr_get_alloc_id(place.ptr())?;
// Load and log the atomic operation.
// Note that atomic loads are possible even from read-only allocations, so `get_alloc_extra_mut` is not an option.
let alloc_meta = this.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap();
log::trace!(
"Atomic op({}) with ordering {:?} on {:?} (size={})",
description,
&atomic,
place.ptr,
place.ptr(),
size.bytes()
);

Expand Down Expand Up @@ -1186,7 +1187,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
{
log::trace!(
"Updated atomic memory({:?}, size={}) to {:#?}",
place.ptr,
place.ptr(),
size.bytes(),
mem_clocks.atomic_ops
);
Expand Down
12 changes: 10 additions & 2 deletions src/concurrency/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::sync::atomic::{AtomicBool, Ordering::Relaxed};
use std::task::Poll;
use std::time::{Duration, SystemTime};

use either::Either;
use log::trace;

use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -259,8 +260,15 @@ impl VisitTags for Frame<'_, '_, Provenance, FrameExtra<'_>> {
return_place.visit_tags(visit);
// Locals.
for local in locals.iter() {
if let LocalValue::Live(value) = &local.value {
value.visit_tags(visit);
match local.as_mplace_or_imm() {
None => {}
Some(Either::Left((ptr, meta))) => {
ptr.visit_tags(visit);
meta.visit_tags(visit);
}
Some(Either::Right(imm)) => {
imm.visit_tags(visit);
}
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/concurrency/weak_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
place: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
let this = self.eval_context_ref();
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
if let crate::AllocExtra {
weak_memory: Some(alloc_buffers),
data_race: Some(alloc_clocks),
Expand Down Expand Up @@ -512,7 +512,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
init: Scalar<Provenance>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
if let (
crate::AllocExtra { weak_memory: Some(alloc_buffers), .. },
crate::MiriMachine { data_race: Some(global), threads, .. },
Expand All @@ -539,7 +539,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_ref();
if let Some(global) = &this.machine.data_race {
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() {
if atomic == AtomicReadOrd::SeqCst {
global.sc_read(&this.machine.threads);
Expand Down Expand Up @@ -577,7 +577,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
init: Scalar<Provenance>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr)?;
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr())?;
if let (
crate::AllocExtra { weak_memory: Some(alloc_buffers), .. },
crate::MiriMachine { data_race: Some(global), threads, .. },
Expand Down Expand Up @@ -627,7 +627,7 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
global.sc_read(&this.machine.threads);
}
let size = place.layout.size;
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr)?;
let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?;
if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() {
let buffer = alloc_buffers
.get_or_create_store_buffer(alloc_range(base_offset, size), init)?;
Expand Down
4 changes: 2 additions & 2 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,9 @@ pub fn report_error<'tcx, 'mir>(
for (i, frame) in ecx.active_thread_stack().iter().enumerate() {
trace!("-------------------");
trace!("Frame {}", i);
trace!(" return: {:?}", *frame.return_place);
trace!(" return: {:?}", frame.return_place);
for (i, local) in frame.locals.iter().enumerate() {
trace!(" local {}: {:?}", i, local.value);
trace!(" local {}: {:?}", i, local);
}
}

Expand Down
15 changes: 6 additions & 9 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,7 @@ impl MainThreadState {
},
Done => {
// Figure out exit code.
let ret_place = MPlaceTy::from_aligned_ptr(
this.machine.main_fn_ret_place.unwrap().ptr,
this.machine.layouts.isize,
);
let ret_place = this.machine.main_fn_ret_place.clone().unwrap();
let exit_code = this.read_target_isize(&ret_place)?;
// Need to call this ourselves since we are not going to return to the scheduler
// loop, and we want the main thread TLS to not show up as memory leaks.
Expand Down Expand Up @@ -308,7 +305,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
let arg_type = Ty::new_array(tcx, tcx.types.u8, size);
let arg_place =
ecx.allocate(ecx.layout_of(arg_type)?, MiriMemoryKind::Machine.into())?;
ecx.write_os_str_to_c_str(OsStr::new(arg), arg_place.ptr, size)?;
ecx.write_os_str_to_c_str(OsStr::new(arg), arg_place.ptr(), size)?;
ecx.mark_immutable(&arg_place);
argvs.push(arg_place.to_ref(&ecx));
}
Expand All @@ -332,15 +329,15 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
ecx.write_scalar(argc, &argc_place)?;
ecx.mark_immutable(&argc_place);
ecx.machine.argc = Some(*argc_place);
ecx.machine.argc = Some(argc_place.ptr());

let argv_place = ecx.allocate(
ecx.layout_of(Ty::new_imm_ptr(tcx, tcx.types.unit))?,
MiriMemoryKind::Machine.into(),
)?;
ecx.write_immediate(argv, &argv_place)?;
ecx.mark_immutable(&argv_place);
ecx.machine.argv = Some(*argv_place);
ecx.machine.argv = Some(argv_place.ptr());
}
// Store command line as UTF-16 for Windows `GetCommandLineW`.
{
Expand All @@ -351,7 +348,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
Ty::new_array(tcx, tcx.types.u16, u64::try_from(cmd_utf16.len()).unwrap());
let cmd_place =
ecx.allocate(ecx.layout_of(cmd_type)?, MiriMemoryKind::Machine.into())?;
ecx.machine.cmd_line = Some(*cmd_place);
ecx.machine.cmd_line = Some(cmd_place.ptr());
// Store the UTF-16 string. We just allocated so we know the bounds are fine.
for (idx, &c) in cmd_utf16.iter().enumerate() {
let place = ecx.project_field(&cmd_place, idx)?;
Expand All @@ -364,7 +361,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(

// Return place (in static memory so that it does not count as leak).
let ret_place = ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
ecx.machine.main_fn_ret_place = Some(*ret_place);
ecx.machine.main_fn_ret_place = Some(ret_place.clone());
// Call start function.

match entry_type {
Expand Down
12 changes: 6 additions & 6 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Store how far we proceeded into the place so far. Everything to the left of
// this offset has already been handled, in the sense that the frozen parts
// have had `action` called on them.
let start_addr = place.ptr.addr();
let start_addr = place.ptr().addr();
let mut cur_addr = start_addr;
// Called when we detected an `UnsafeCell` at the given offset and size.
// Calls `action` and advances `cur_ptr`.
Expand Down Expand Up @@ -413,7 +413,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let mut visitor = UnsafeCellVisitor {
ecx: this,
unsafe_cell_action: |place| {
trace!("unsafe_cell_action on {:?}", place.ptr);
trace!("unsafe_cell_action on {:?}", place.ptr());
// We need a size to go on.
let unsafe_cell_size = this
.size_and_align_of_mplace(place)?
Expand All @@ -422,7 +422,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
.unwrap_or_else(|| place.layout.size);
// Now handle this `UnsafeCell`, unless it is empty.
if unsafe_cell_size != Size::ZERO {
unsafe_cell_action(&place.ptr, unsafe_cell_size)
unsafe_cell_action(&place.ptr(), unsafe_cell_size)
} else {
Ok(())
}
Expand All @@ -432,7 +432,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
// The part between the end_ptr and the end of the place is also frozen.
// So pretend there is a 0-sized `UnsafeCell` at the end.
unsafe_cell_action(&place.ptr.offset(size, this)?, Size::ZERO)?;
unsafe_cell_action(&place.ptr().offset(size, this)?, Size::ZERO)?;
// Done!
return Ok(());

Expand Down Expand Up @@ -994,10 +994,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}

/// Mark a machine allocation that was just created as immutable.
fn mark_immutable(&mut self, mplace: &MemPlace<Provenance>) {
fn mark_immutable(&mut self, mplace: &MPlaceTy<'tcx, Provenance>) {
let this = self.eval_context_mut();
// This got just allocated, so there definitely is a pointer here.
let provenance = mplace.ptr.into_pointer_or_addr().unwrap().provenance;
let provenance = mplace.ptr().into_pointer_or_addr().unwrap().provenance;
this.alloc_mark_immutable(provenance.get_alloc_id().unwrap()).unwrap();
}

Expand Down
16 changes: 8 additions & 8 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,14 +396,14 @@ pub struct MiriMachine<'mir, 'tcx> {
pub(crate) env_vars: EnvVars<'tcx>,

/// Return place of the main function.
pub(crate) main_fn_ret_place: Option<MemPlace<Provenance>>,
pub(crate) main_fn_ret_place: Option<MPlaceTy<'tcx, Provenance>>,

/// Program arguments (`Option` because we can only initialize them after creating the ecx).
/// These are *pointers* to argc/argv because macOS.
/// We also need the full command line as one string because of Windows.
pub(crate) argc: Option<MemPlace<Provenance>>,
pub(crate) argv: Option<MemPlace<Provenance>>,
pub(crate) cmd_line: Option<MemPlace<Provenance>>,
pub(crate) argc: Option<Pointer<Option<Provenance>>>,
pub(crate) argv: Option<Pointer<Option<Provenance>>>,
pub(crate) cmd_line: Option<Pointer<Option<Provenance>>>,

/// TLS state.
pub(crate) tls: TlsData<'tcx>,
Expand Down Expand Up @@ -670,7 +670,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
) -> InterpResult<'tcx> {
let place = this.allocate(val.layout, MiriMemoryKind::ExternStatic.into())?;
this.write_immediate(*val, &place)?;
Self::add_extern_static(this, name, place.ptr);
Self::add_extern_static(this, name, place.ptr());
Ok(())
}

Expand All @@ -686,7 +686,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
Self::add_extern_static(
this,
"environ",
this.machine.env_vars.environ.as_ref().unwrap().ptr,
this.machine.env_vars.environ.as_ref().unwrap().ptr(),
);
// A couple zero-initialized pointer-sized extern statics.
// Most of them are for weak symbols, which we all set to null (indicating that the
Expand All @@ -703,7 +703,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
Self::add_extern_static(
this,
"environ",
this.machine.env_vars.environ.as_ref().unwrap().ptr,
this.machine.env_vars.environ.as_ref().unwrap().ptr(),
);
}
"android" => {
Expand Down Expand Up @@ -1415,7 +1415,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
local: mir::Local,
mplace: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
let Some(Provenance::Concrete { alloc_id, .. }) = mplace.ptr.provenance else {
let Some(Provenance::Concrete { alloc_id, .. }) = mplace.ptr().provenance else {
panic!("after_local_allocated should only be called on fresh allocations");
};
let local_decl = &ecx.active_thread_stack()[frame].body.local_decls[local];
Expand Down
Loading

0 comments on commit aae7597

Please sign in to comment.