Skip to content

Miri subtree update #127317

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 17 commits into from
Jul 4, 2024
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
2 changes: 1 addition & 1 deletion src/tools/miri/rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
9ed2ab3790ff41bf741dd690befd6a1c1e2b23ca
66b4f0021bfb11a8c20d084c99a40f4a78ce1d38
10 changes: 7 additions & 3 deletions src/tools/miri/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/tools/miri/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/tools/miri/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/tools/miri/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/tools/miri/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/tools/miri/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/tools/miri/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/tools/miri/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/tools/miri/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/tools/miri/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
Loading