Skip to content

Commit

Permalink
Auto merge of #127317 - RalfJung:miri-sync, r=RalfJung
Browse files Browse the repository at this point in the history
Miri subtree update

r? `@ghost`
  • Loading branch information
bors committed Jul 4, 2024
2 parents ef8dc07 + 89c5442 commit 2ede4a4
Show file tree
Hide file tree
Showing 15 changed files with 192 additions and 71 deletions.
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
9ed2ab3790ff41bf741dd690befd6a1c1e2b23ca
66b4f0021bfb11a8c20d084c99a40f4a78ce1d38
10 changes: 7 additions & 3 deletions src/borrow_tracker/tree_borrows/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub enum AccessCause {
Explicit(AccessKind),
Reborrow,
Dealloc,
FnExit,
FnExit(AccessKind),
}

impl fmt::Display for AccessCause {
Expand All @@ -28,7 +28,11 @@ impl fmt::Display for AccessCause {
Self::Explicit(kind) => write!(f, "{kind}"),
Self::Reborrow => write!(f, "reborrow"),
Self::Dealloc => write!(f, "deallocation"),
Self::FnExit => write!(f, "protector release"),
// This is dead code, since the protector release access itself can never
// cause UB (while the protector is active, if some other access invalidates
// further use of the protected tag, that is immediate UB).
// Describing the cause of UB is the only time this function is called.
Self::FnExit(_) => unreachable!("protector accesses can never be the source of UB"),
}
}
}
Expand All @@ -40,7 +44,7 @@ impl AccessCause {
Self::Explicit(kind) => format!("{rel} {kind}"),
Self::Reborrow => format!("reborrow (acting as a {rel} read access)"),
Self::Dealloc => format!("deallocation (acting as a {rel} write access)"),
Self::FnExit => format!("protector release (acting as a {rel} read access)"),
Self::FnExit(kind) => format!("protector release (acting as a {rel} {kind})"),
}
}
}
Expand Down
19 changes: 4 additions & 15 deletions src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,11 @@ impl<'tcx> Tree {
let global = machine.borrow_tracker.as_ref().unwrap();
let span = machine.current_span();
self.perform_access(
access_kind,
tag,
Some(range),
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
global,
alloc_id,
span,
diagnostics::AccessCause::Explicit(access_kind),
)
}

Expand Down Expand Up @@ -115,15 +113,8 @@ impl<'tcx> Tree {
alloc_id: AllocId, // diagnostics
) -> InterpResult<'tcx> {
let span = machine.current_span();
self.perform_access(
AccessKind::Read,
tag,
None, // no specified range because it occurs on the entire allocation
global,
alloc_id,
span,
diagnostics::AccessCause::FnExit,
)
// `None` makes it the magic on-protector-end operation
self.perform_access(tag, None, global, alloc_id, span)
}
}

Expand Down Expand Up @@ -297,13 +288,11 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

// All reborrows incur a (possibly zero-sized) read access to the parent
tree_borrows.perform_access(
AccessKind::Read,
orig_tag,
Some(range),
Some((range, AccessKind::Read, diagnostics::AccessCause::Reborrow)),
this.machine.borrow_tracker.as_ref().unwrap(),
alloc_id,
this.machine.current_span(),
diagnostics::AccessCause::Reborrow,
)?;
// Record the parent-child pair in the tree.
tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?;
Expand Down
4 changes: 4 additions & 0 deletions src/borrow_tracker/tree_borrows/perms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ impl Permission {
pub fn is_disabled(&self) -> bool {
self.inner == Disabled
}
/// Check if `self` is the post-child-write state of a pointer (is `Active`).
pub fn is_active(&self) -> bool {
self.inner == Active
}

/// Default initial permission of the root of a new tree at inbounds positions.
/// Must *only* be used for the root, this is not in general an "initial" permission!
Expand Down
41 changes: 23 additions & 18 deletions src/borrow_tracker/tree_borrows/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,13 +530,11 @@ impl<'tcx> Tree {
span: Span, // diagnostics
) -> InterpResult<'tcx> {
self.perform_access(
AccessKind::Write,
tag,
Some(access_range),
Some((access_range, AccessKind::Write, diagnostics::AccessCause::Dealloc)),
global,
alloc_id,
span,
diagnostics::AccessCause::Dealloc,
)?;
for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size) {
TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
Expand Down Expand Up @@ -570,12 +568,16 @@ impl<'tcx> Tree {
}

/// Map the per-node and per-location `LocationState::perform_access`
/// to each location of `access_range`, on every tag of the allocation.
/// to each location of the first component of `access_range_and_kind`,
/// on every tag of the allocation.
///
/// If `access_range` is `None`, this is interpreted as the special
/// If `access_range_and_kind` is `None`, this is interpreted as the special
/// access that is applied on protector release:
/// - the access will be applied only to initialized locations of the allocation,
/// - and it will not be visible to children.
/// - it will not be visible to children,
/// - it will be recorded as a `FnExit` diagnostic access
/// - and it will be a read except if the location is `Active`, i.e. has been written to,
/// in which case it will be a write.
///
/// `LocationState::perform_access` will take care of raising transition
/// errors and updating the `initialized` status of each location,
Expand All @@ -585,13 +587,11 @@ impl<'tcx> Tree {
/// - recording the history.
pub fn perform_access(
&mut self,
access_kind: AccessKind,
tag: BorTag,
access_range: Option<AllocRange>,
access_range_and_kind: Option<(AllocRange, AccessKind, diagnostics::AccessCause)>,
global: &GlobalState,
alloc_id: AllocId, // diagnostics
span: Span, // diagnostics
access_cause: diagnostics::AccessCause, // diagnostics
alloc_id: AllocId, // diagnostics
span: Span, // diagnostics
) -> InterpResult<'tcx> {
use std::ops::Range;
// Performs the per-node work:
Expand All @@ -605,6 +605,8 @@ impl<'tcx> Tree {
// `perms_range` is only for diagnostics (it is the range of
// the `RangeMap` on which we are currently working).
let node_app = |perms_range: Range<u64>,
access_kind: AccessKind,
access_cause: diagnostics::AccessCause,
args: NodeAppArgs<'_>|
-> Result<ContinueTraversal, TransitionError> {
let NodeAppArgs { node, mut perm, rel_pos } = args;
Expand All @@ -618,14 +620,13 @@ impl<'tcx> Tree {

let protected = global.borrow().protected_tags.contains_key(&node.tag);
let transition = old_state.perform_access(access_kind, rel_pos, protected)?;

// Record the event as part of the history
if !transition.is_noop() {
node.debug_info.history.push(diagnostics::Event {
transition,
is_foreign: rel_pos.is_foreign(),
access_cause,
access_range,
access_range: access_range_and_kind.map(|x| x.0),
transition_range: perms_range,
span,
});
Expand All @@ -636,6 +637,7 @@ impl<'tcx> Tree {
// Error handler in case `node_app` goes wrong.
// Wraps the faulty transition in more context for diagnostics.
let err_handler = |perms_range: Range<u64>,
access_cause: diagnostics::AccessCause,
args: ErrHandlerArgs<'_, TransitionError>|
-> InterpError<'tcx> {
let ErrHandlerArgs { error_kind, conflicting_info, accessed_info } = args;
Expand All @@ -650,16 +652,16 @@ impl<'tcx> Tree {
.build()
};

if let Some(access_range) = access_range {
if let Some((access_range, access_kind, access_cause)) = access_range_and_kind {
// Default branch: this is a "normal" access through a known range.
// We iterate over affected locations and traverse the tree for each of them.
for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size)
{
TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
.traverse_parents_this_children_others(
tag,
|args| node_app(perms_range.clone(), args),
|args| err_handler(perms_range.clone(), args),
|args| node_app(perms_range.clone(), access_kind, access_cause, args),
|args| err_handler(perms_range.clone(), access_cause, args),
)?;
}
} else {
Expand All @@ -678,11 +680,14 @@ impl<'tcx> Tree {
if let Some(p) = perms.get(idx)
&& p.initialized
{
let access_kind =
if p.permission.is_active() { AccessKind::Write } else { AccessKind::Read };
let access_cause = diagnostics::AccessCause::FnExit(access_kind);
TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
.traverse_nonchildren(
tag,
|args| node_app(perms_range.clone(), args),
|args| err_handler(perms_range.clone(), args),
|args| node_app(perms_range.clone(), access_kind, access_cause, args),
|args| err_handler(perms_range.clone(), access_cause, args),
)?;
}
}
Expand Down
10 changes: 1 addition & 9 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use rustc_middle::ty::{
FloatTy, IntTy, Ty, TyCtxt, UintTy,
};
use rustc_session::config::CrateType;
use rustc_span::{sym, Span, Symbol};
use rustc_span::{Span, Symbol};
use rustc_target::abi::{Align, FieldIdx, FieldsShape, Size, Variants};
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -1182,14 +1182,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.alloc_mark_immutable(provenance.get_alloc_id().unwrap()).unwrap();
}

fn item_link_name(&self, def_id: DefId) -> Symbol {
let tcx = self.eval_context_ref().tcx;
match tcx.get_attrs(def_id, sym::link_name).filter_map(|a| a.value_str()).next() {
Some(name) => name,
None => tcx.item_name(def_id),
}
}

/// Converts `src` from floating point to integer type `dest_ty`
/// after rounding with mode `round`.
/// Returns `None` if `f` is NaN or out of range.
Expand Down
4 changes: 2 additions & 2 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
// foreign function
// Any needed call to `goto_block` will be performed by `emulate_foreign_item`.
let args = ecx.copy_fn_args(args); // FIXME: Should `InPlace` arguments be reset to uninit?
let link_name = ecx.item_link_name(instance.def_id());
let link_name = Symbol::intern(ecx.tcx.symbol_name(instance).name);
return ecx.emulate_foreign_item(link_name, abi, &args, dest, ret, unwind);
}

Expand Down Expand Up @@ -1050,7 +1050,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
ecx: &MiriInterpCx<'tcx>,
def_id: DefId,
) -> InterpResult<'tcx, StrictPointer> {
let link_name = ecx.item_link_name(def_id);
let link_name = Symbol::intern(ecx.tcx.symbol_name(Instance::mono(*ecx.tcx, def_id)).name);
if let Some(&ptr) = ecx.machine.extern_statics.get(&link_name) {
// Various parts of the engine rely on `get_alloc_info` for size and alignment
// information. That uses the type information of this static.
Expand Down
15 changes: 0 additions & 15 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
unwind: mir::UnwindAction,
) -> InterpResult<'tcx, Option<(&'tcx mir::Body<'tcx>, ty::Instance<'tcx>)>> {
let this = self.eval_context_mut();
let tcx = this.tcx.tcx;

// Some shims forward to other MIR bodies.
match link_name.as_str() {
// This matches calls to the foreign item `panic_impl`.
// The implementation is provided by the function with the `#[panic_handler]` attribute.
"panic_impl" => {
// We don't use `check_shim` here because we are just forwarding to the lang
// item. Argument count checking will be performed when the returned `Body` is
// called.
this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?;
let panic_impl_id = tcx.lang_items().panic_impl().unwrap();
let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id);
return Ok(Some((
this.load_mir(panic_impl_instance.def, None)?,
panic_impl_instance,
)));
}
"__rust_alloc_error_handler" => {
// Forward to the right symbol that implements this function.
let Some(handler_kind) = this.tcx.alloc_error_handler_kind(()) else {
Expand Down
41 changes: 33 additions & 8 deletions src/shims/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,32 @@ impl FdTable {

impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();

let Some(dup_fd) = this.machine.fds.dup(old_fd) else {
return this.fd_not_found();
};
Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, 0))
}

fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();

let Some(dup_fd) = this.machine.fds.dup(old_fd) else {
return this.fd_not_found();
};
if new_fd != old_fd {
// Close new_fd if it is previously opened.
// If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive.
if let Some(file_descriptor) = this.machine.fds.fds.insert(new_fd, dup_fd) {
// Ignore close error (not interpreter's) according to dup2() doc.
file_descriptor.close(this.machine.communicate())?.ok();
}
}
Ok(new_fd)
}

fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();

Expand Down Expand Up @@ -334,14 +360,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

let fd = this.read_scalar(fd_op)?.to_i32()?;

Ok(Scalar::from_i32(if let Some(file_descriptor) = this.machine.fds.remove(fd) {
let result = file_descriptor.close(this.machine.communicate())?;
// return `0` if close is successful
let result = result.map(|()| 0i32);
this.try_unwrap_io_result(result)?
} else {
this.fd_not_found()?
}))
let Some(file_descriptor) = this.machine.fds.remove(fd) else {
return Ok(Scalar::from_i32(this.fd_not_found()?));
};
let result = file_descriptor.close(this.machine.communicate())?;
// return `0` if close is successful
let result = result.map(|()| 0i32);
Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
}

/// Function used when a file descriptor does not exist. It returns `Ok(-1)`and sets
Expand Down
13 changes: 13 additions & 0 deletions src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let result = this.fcntl(args)?;
this.write_scalar(Scalar::from_i32(result), dest)?;
}
"dup" => {
let [old_fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let old_fd = this.read_scalar(old_fd)?.to_i32()?;
let new_fd = this.dup(old_fd)?;
this.write_scalar(Scalar::from_i32(new_fd), dest)?;
}
"dup2" => {
let [old_fd, new_fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let old_fd = this.read_scalar(old_fd)?.to_i32()?;
let new_fd = this.read_scalar(new_fd)?.to_i32()?;
let result = this.dup2(old_fd, new_fd)?;
this.write_scalar(Scalar::from_i32(result), dest)?;
}

// File and file system access
"open" | "open64" => {
Expand Down
11 changes: 11 additions & 0 deletions src/shims/x86/avx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,17 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

this.write_scalar(Scalar::from_i32(res.into()), dest)?;
}
// Used to implement the `_mm256_zeroupper` and `_mm256_zeroall` functions.
// These function clear out the upper 128 bits of all avx registers or
// zero out all avx registers respectively.
"vzeroupper" | "vzeroall" => {
// These functions are purely a performance hint for the CPU.
// Any registers currently in use will be saved beforehand by the
// compiler, making these functions no-ops.

// The only thing that needs to be ensured is the correct calling convention.
let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
}
_ => return Ok(EmulateItemResult::NotSupported),
}
Ok(EmulateItemResult::NeedsReturn)
Expand Down
Loading

0 comments on commit 2ede4a4

Please sign in to comment.