diff --git a/src/tools/miri/bench-cargo-miri/slice-chunked/Cargo.lock b/src/tools/miri/bench-cargo-miri/slice-chunked/Cargo.lock new file mode 100644 index 00000000000..0f9e5db52d6 --- /dev/null +++ b/src/tools/miri/bench-cargo-miri/slice-chunked/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "slice-chunked" +version = "0.1.0" diff --git a/src/tools/miri/bench-cargo-miri/slice-chunked/Cargo.toml b/src/tools/miri/bench-cargo-miri/slice-chunked/Cargo.toml new file mode 100644 index 00000000000..8fed6ad2f31 --- /dev/null +++ b/src/tools/miri/bench-cargo-miri/slice-chunked/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "slice-chunked" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/src/tools/miri/bench-cargo-miri/slice-chunked/src/main.rs b/src/tools/miri/bench-cargo-miri/slice-chunked/src/main.rs new file mode 100644 index 00000000000..b498c8bec75 --- /dev/null +++ b/src/tools/miri/bench-cargo-miri/slice-chunked/src/main.rs @@ -0,0 +1,26 @@ +//! This is a small example using slice::chunks, which creates a very large Tree Borrows tree. +//! Thanks to ##3837, the GC now compacts the tree, so this test can be run in a reasonable time again. +//! The actual code is adapted from tiny_skia, see https://github.com/RazrFalcon/tiny-skia/blob/master/src/pixmap.rs#L121 +//! To make this benchmark demonstrate the effectiveness, run with MIRIFLAGS="-Zmiri-tree-borrows -Zmiri-provenance-gc=100" + +const N: usize = 1000; + +fn input_vec() -> Vec { + vec![0; N] +} + +fn main() { + let data_len = 2 * N; + let mut rgba_data = Vec::with_capacity(data_len); + let img_data = input_vec(); + for slice in img_data.chunks(2) { + let gray = slice[0]; + let alpha = slice[1]; + rgba_data.push(gray); + rgba_data.push(gray); + rgba_data.push(gray); + rgba_data.push(alpha); + } + + assert_eq!(rgba_data.len(), data_len); +} diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 1eca86baeaa..c33d9ad8614 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -fdf61d499c8a8421ecf98e7924bb87caf43a9938 +0d634185dfddefe09047881175f35c65d68dcff1 diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 7a3d76a9beb..9e205cd0064 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -71,6 +71,12 @@ pub struct FrameState { impl VisitProvenance for FrameState { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { + // Visit all protected tags. At least in Tree Borrows, + // protected tags can not be GC'd because they still have + // an access coming when the protector ends. Additionally, + // the tree compacting mechanism of TB's GC relies on the fact + // that all protected tags are marked as live for correctness, + // so we _have_ to visit them here. for (id, tag) in &self.protected_tags { visit(Some(*id), Some(*tag)); } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 5461edb51d3..c29bd719b15 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -130,7 +130,7 @@ mod transition { Active => if protected { // We wrote, someone else reads -- that's bad. - // (If this is initialized, this move-to-protected will mean insta-UB.) + // (Since Active is always initialized, this move-to-protected will mean insta-UB.) Disabled } else { // We don't want to disable here to allow read-read reordering: it is crucial @@ -267,6 +267,44 @@ impl Permission { transition::perform_access(kind, rel_pos, old_state, protected) .map(|new_state| PermTransition { from: old_state, to: new_state }) } + + /// During a provenance GC, we want to compact the tree. + /// For this, we want to merge nodes upwards if they have a singleton parent. + /// But we need to be careful: If the parent is Frozen, and the child is Reserved, + /// we can not do such a merge. In general, such a merge is possible if the parent + /// allows similar accesses, and in particular if the parent never causes UB on its + /// own. This is enforced by a test, namely `tree_compacting_is_sound`. See that + /// test for more information. + /// This method is only sound if the parent is not protected. We never attempt to + /// remove protected parents. + pub fn can_be_replaced_by_child(self, child: Self) -> bool { + match (self.inner, child.inner) { + // ReservedIM can be replaced by anything, as it allows all + // transitions. + (ReservedIM, _) => true, + // Reserved (as parent, where conflictedness does not matter) + // can be replaced by all but ReservedIM, + // since ReservedIM alone would survive foreign writes + (ReservedFrz { .. }, ReservedIM) => false, + (ReservedFrz { .. }, _) => true, + // Active can not be replaced by something surviving + // foreign reads and then remaining writable. + (Active, ReservedIM) => false, + (Active, ReservedFrz { .. }) => false, + // Replacing a state by itself is always okay, even if the child state is protected. + (Active, Active) => true, + // Active can be replaced by Frozen, since it is not protected. + (Active, Frozen) => true, + (Active, Disabled) => true, + // Frozen can only be replaced by Disabled (and itself). + (Frozen, Frozen) => true, + (Frozen, Disabled) => true, + (Frozen, _) => false, + // Disabled can not be replaced by anything else. + (Disabled, Disabled) => true, + (Disabled, _) => false, + } + } } impl PermTransition { diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 56643c6cbe8..53e722259fd 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -128,6 +128,22 @@ impl LocationState { Ok(transition) } + /// Like `perform_access`, but ignores the concrete error cause and also uses state-passing + /// rather than a mutable reference. As such, it returns `Some(x)` if the transition succeeded, + /// or `None` if there was an error. + #[cfg(test)] + fn perform_access_no_fluff( + mut self, + access_kind: AccessKind, + rel_pos: AccessRelatedness, + protected: bool, + ) -> Option { + match self.perform_access(access_kind, rel_pos, protected) { + Ok(_) => Some(self), + Err(_) => None, + } + } + // Helper to optimize the tree traversal. // The optimization here consists of observing thanks to the tests // `foreign_read_is_noop_after_foreign_write` and `all_transitions_idempotent`, @@ -149,9 +165,11 @@ impl LocationState { // is possible, it also updates the internal state to keep track of whether // the propagation can be skipped next time. // It is a performance loss not to call this function when a foreign access occurs. - // It is unsound not to call this function when a child access occurs. + // FIXME: This optimization is wrong, and is currently disabled (by ignoring the + // result returned here). Since we presumably want an optimization like this, + // we should add it back. See #3864 for more information. fn skip_if_known_noop( - &mut self, + &self, access_kind: AccessKind, rel_pos: AccessRelatedness, ) -> ContinueTraversal { @@ -174,22 +192,37 @@ impl LocationState { // No need to update `self.latest_foreign_access`, // the type of the current streak among nonempty read-only // or nonempty with at least one write has not changed. - ContinueTraversal::SkipChildren + ContinueTraversal::SkipSelfAndChildren } else { // Otherwise propagate this time, and also record the // access that just occurred so that we can skip the propagation // next time. - self.latest_foreign_access = Some(access_kind); ContinueTraversal::Recurse } } else { // A child access occurred, this breaks the streak of foreign // accesses in a row and the sequence since the previous child access // is now empty. - self.latest_foreign_access = None; ContinueTraversal::Recurse } } + + /// Records a new access, so that future access can potentially be skipped + /// by `skip_if_known_noop`. + /// The invariants for this function are closely coupled to the function above: + /// It MUST be called on child accesses, and on foreign accesses MUST be called + /// when `skip_if_know_noop` returns `Recurse`, and MUST NOT be called otherwise. + /// FIXME: This optimization is wrong, and is currently disabled (by ignoring the + /// result returned here). Since we presumably want an optimization like this, + /// we should add it back. See #3864 for more information. + #[allow(unused)] + fn record_new_access(&mut self, access_kind: AccessKind, rel_pos: AccessRelatedness) { + if rel_pos.is_foreign() { + self.latest_foreign_access = Some(access_kind); + } else { + self.latest_foreign_access = None; + } + } } impl fmt::Display for LocationState { @@ -282,13 +315,29 @@ struct TreeVisitor<'tree> { /// Whether to continue exploring the children recursively or not. enum ContinueTraversal { Recurse, - SkipChildren, + SkipSelfAndChildren, +} + +#[derive(Clone, Copy)] +pub enum ChildrenVisitMode { + VisitChildrenOfAccessed, + SkipChildrenOfAccessed, +} + +enum RecursionState { + BeforeChildren, + AfterChildren, } /// Stack of nodes left to explore in a tree traversal. -struct TreeVisitorStack { +/// See the docs of `traverse_this_parents_children_other` for details on the +/// traversal order. +struct TreeVisitorStack { /// Identifier of the original access. initial: UniIndex, + /// Function describing whether to continue at a tag. + /// This is only invoked for foreign accesses. + f_continue: NodeContinue, /// Function to apply to each tag. f_propagate: NodeApp, /// Handler to add the required context to diagnostics. @@ -296,152 +345,232 @@ struct TreeVisitorStack { /// Mutable state of the visit: the tags left to handle. /// Every tag pushed should eventually be handled, /// and the precise order is relevant for diagnostics. - stack: Vec<(UniIndex, AccessRelatedness)>, + /// Since the traversal is piecewise bottom-up, we need to + /// remember whether we're here initially, or after visiting all children. + /// The last element indicates this. + /// This is just an artifact of how you hand-roll recursion, + /// it does not have a deeper meaning otherwise. + stack: Vec<(UniIndex, AccessRelatedness, RecursionState)>, } -impl TreeVisitorStack +impl + TreeVisitorStack where - NodeApp: Fn(NodeAppArgs<'_>) -> Result, + NodeContinue: Fn(&NodeAppArgs<'_>) -> ContinueTraversal, + NodeApp: Fn(NodeAppArgs<'_>) -> Result<(), InnErr>, ErrHandler: Fn(ErrHandlerArgs<'_, InnErr>) -> OutErr, { - /// Apply the function to the current `tag`, and push its children - /// to the stack of future tags to visit. - fn exec_and_visit( + fn should_continue_at( + &self, + this: &mut TreeVisitor<'_>, + idx: UniIndex, + rel_pos: AccessRelatedness, + ) -> ContinueTraversal { + let node = this.nodes.get_mut(idx).unwrap(); + let args = NodeAppArgs { node, perm: this.perms.entry(idx), rel_pos }; + (self.f_continue)(&args) + } + + fn propagate_at( &mut self, this: &mut TreeVisitor<'_>, idx: UniIndex, - exclude: Option, rel_pos: AccessRelatedness, ) -> Result<(), OutErr> { - // 1. apply the propagation function let node = this.nodes.get_mut(idx).unwrap(); - let recurse = - (self.f_propagate)(NodeAppArgs { node, perm: this.perms.entry(idx), rel_pos }) - .map_err(|error_kind| { - (self.err_builder)(ErrHandlerArgs { - error_kind, - conflicting_info: &this.nodes.get(idx).unwrap().debug_info, - accessed_info: &this.nodes.get(self.initial).unwrap().debug_info, - }) - })?; - let node = this.nodes.get(idx).unwrap(); - // 2. add the children to the stack for future traversal - if matches!(recurse, ContinueTraversal::Recurse) { - let general_child_rel = rel_pos.for_child(); - for &child in node.children.iter() { - // Some child might be excluded from here and handled separately, - // e.g. the initially accessed tag. - if Some(child) != exclude { - // We should still ensure that if we don't skip the initially accessed - // it will receive the proper `AccessRelatedness`. - let this_child_rel = if child == self.initial { - AccessRelatedness::This - } else { - general_child_rel - }; - self.stack.push((child, this_child_rel)); + (self.f_propagate)(NodeAppArgs { node, perm: this.perms.entry(idx), rel_pos }).map_err( + |error_kind| { + (self.err_builder)(ErrHandlerArgs { + error_kind, + conflicting_info: &this.nodes.get(idx).unwrap().debug_info, + accessed_info: &this.nodes.get(self.initial).unwrap().debug_info, + }) + }, + ) + } + + fn go_upwards_from_accessed( + &mut self, + this: &mut TreeVisitor<'_>, + accessed_node: UniIndex, + visit_children: ChildrenVisitMode, + ) -> Result<(), OutErr> { + // We want to visit the accessed node's children first. + // However, we will below walk up our parents and push their children (our cousins) + // onto the stack. To ensure correct iteration order, this method thus finishes + // by reversing the stack. This only works if the stack is empty initially. + assert!(self.stack.is_empty()); + // First, handle accessed node. A bunch of things need to + // be handled differently here compared to the further parents + // of `accesssed_node`. + { + self.propagate_at(this, accessed_node, AccessRelatedness::This)?; + if matches!(visit_children, ChildrenVisitMode::VisitChildrenOfAccessed) { + let accessed_node = this.nodes.get(accessed_node).unwrap(); + // We `rev()` here because we reverse the entire stack later. + for &child in accessed_node.children.iter().rev() { + self.stack.push(( + child, + AccessRelatedness::AncestorAccess, + RecursionState::BeforeChildren, + )); + } + } + } + // Then, handle the accessed node's parents. Here, we need to + // make sure we only mark the "cousin" subtrees for later visitation, + // not the subtree that contains the accessed node. + let mut last_node = accessed_node; + while let Some(current) = this.nodes.get(last_node).unwrap().parent { + self.propagate_at(this, current, AccessRelatedness::StrictChildAccess)?; + let node = this.nodes.get(current).unwrap(); + // We `rev()` here because we reverse the entire stack later. + for &child in node.children.iter().rev() { + if last_node == child { + continue; + } + self.stack.push(( + child, + AccessRelatedness::DistantAccess, + RecursionState::BeforeChildren, + )); + } + last_node = current; + } + // Reverse the stack, as discussed above. + self.stack.reverse(); + Ok(()) + } + + fn finish_foreign_accesses(&mut self, this: &mut TreeVisitor<'_>) -> Result<(), OutErr> { + while let Some((idx, rel_pos, step)) = self.stack.last_mut() { + let idx = *idx; + let rel_pos = *rel_pos; + match *step { + // How to do bottom-up traversal, 101: Before you handle a node, you handle all children. + // For this, you must first find the children, which is what this code here does. + RecursionState::BeforeChildren => { + // Next time we come back will be when all the children are handled. + *step = RecursionState::AfterChildren; + // Now push the children, except if we are told to skip this subtree. + let handle_children = self.should_continue_at(this, idx, rel_pos); + match handle_children { + ContinueTraversal::Recurse => { + let node = this.nodes.get(idx).unwrap(); + for &child in node.children.iter() { + self.stack.push((child, rel_pos, RecursionState::BeforeChildren)); + } + } + ContinueTraversal::SkipSelfAndChildren => { + // skip self + self.stack.pop(); + continue; + } + } + } + // All the children are handled, let's actually visit this node + RecursionState::AfterChildren => { + self.stack.pop(); + self.propagate_at(this, idx, rel_pos)?; } } } Ok(()) } - fn new(initial: UniIndex, f_propagate: NodeApp, err_builder: ErrHandler) -> Self { - Self { initial, f_propagate, err_builder, stack: Vec::new() } - } - - /// Finish the exploration by applying `exec_and_visit` until - /// the stack is empty. - fn finish(&mut self, visitor: &mut TreeVisitor<'_>) -> Result<(), OutErr> { - while let Some((idx, rel_pos)) = self.stack.pop() { - self.exec_and_visit(visitor, idx, /* no children to exclude */ None, rel_pos)?; - } - Ok(()) - } - - /// Push all ancestors to the exploration stack in order of nearest ancestor - /// towards the top. - fn push_and_visit_strict_ancestors( - &mut self, - visitor: &mut TreeVisitor<'_>, - ) -> Result<(), OutErr> { - let mut path_ascend = Vec::new(); - // First climb to the root while recording the path - let mut curr = self.initial; - while let Some(ancestor) = visitor.nodes.get(curr).unwrap().parent { - path_ascend.push((ancestor, curr)); - curr = ancestor; - } - // Then descend: - // - execute f_propagate on each node - // - record children in visit - while let Some((ancestor, next_in_path)) = path_ascend.pop() { - // Explore ancestors in descending order. - // `next_in_path` is excluded from the recursion because it - // will be the `ancestor` of the next iteration. - // It also needs a different `AccessRelatedness` than the other - // children of `ancestor`. - self.exec_and_visit( - visitor, - ancestor, - Some(next_in_path), - AccessRelatedness::StrictChildAccess, - )?; - } - Ok(()) + fn new( + initial: UniIndex, + f_continue: NodeContinue, + f_propagate: NodeApp, + err_builder: ErrHandler, + ) -> Self { + Self { initial, f_continue, f_propagate, err_builder, stack: Vec::new() } } } impl<'tree> TreeVisitor<'tree> { - // Applies `f_propagate` to every vertex of the tree top-down in the following order: first - // all ancestors of `start`, then `start` itself, then children of `start`, then the rest. - // This ensures that errors are triggered in the following order - // - first invalid accesses with insufficient permissions, closest to the root first, - // - then protector violations, closest to `start` first. - // - // `f_propagate` should follow the following format: for a given `Node` it updates its - // `Permission` depending on the position relative to `start` (given by an - // `AccessRelatedness`). - // It outputs whether the tree traversal for this subree should continue or not. - fn traverse_parents_this_children_others( + /// Applies `f_propagate` to every vertex of the tree in a piecewise bottom-up way: First, visit + /// all ancestors of `start` (starting with `start` itself), then children of `start`, then the rest, + /// going bottom-up in each of these two "pieces" / sections. + /// This ensures that errors are triggered in the following order + /// - first invalid accesses with insufficient permissions, closest to the accessed node first, + /// - then protector violations, bottom-up, starting with the children of the accessed node, and then + /// going upwards and outwards. + /// + /// The following graphic visualizes it, with numbers indicating visitation order and `start` being + /// the node that is visited first ("1"): + /// + /// ```text + /// 3 + /// /| + /// / | + /// 9 2 + /// | |\ + /// | | \ + /// 8 1 7 + /// / \ + /// 4 6 + /// | + /// 5 + /// ``` + /// + /// `f_propagate` should follow the following format: for a given `Node` it updates its + /// `Permission` depending on the position relative to `start` (given by an + /// `AccessRelatedness`). + /// `f_continue` is called earlier on foreign nodes, and describes whether to even start + /// visiting the subtree at that node. If it e.g. returns `SkipSelfAndChildren` on node 6 + /// above, then nodes 5 _and_ 6 would not be visited by `f_propagate`. It is not used for + /// notes having a child access (nodes 1, 2, 3). + /// + /// Finally, remember that the iteration order is not relevant for UB, it only affects + /// diagnostics. It also affects tree traversal optimizations built on top of this, so + /// those need to be reviewed carefully as well whenever this changes. + fn traverse_this_parents_children_other( mut self, start: BorTag, - f_propagate: impl Fn(NodeAppArgs<'_>) -> Result, + f_continue: impl Fn(&NodeAppArgs<'_>) -> ContinueTraversal, + f_propagate: impl Fn(NodeAppArgs<'_>) -> Result<(), InnErr>, err_builder: impl Fn(ErrHandlerArgs<'_, InnErr>) -> OutErr, ) -> Result<(), OutErr> { let start_idx = self.tag_mapping.get(&start).unwrap(); - let mut stack = TreeVisitorStack::new(start_idx, f_propagate, err_builder); - stack.push_and_visit_strict_ancestors(&mut self)?; - // All (potentially zero) ancestors have been explored, - // it's time to explore the `start` tag. - stack.exec_and_visit( + let mut stack = TreeVisitorStack::new(start_idx, f_continue, f_propagate, err_builder); + // Visits the accessed node itself, and all its parents, i.e. all nodes + // undergoing a child access. Also pushes the children and the other + // cousin nodes (i.e. all nodes undergoing a foreign access) to the stack + // to be processed later. + stack.go_upwards_from_accessed( &mut self, start_idx, - /* no children to exclude */ None, - AccessRelatedness::This, + ChildrenVisitMode::VisitChildrenOfAccessed, )?; - // Then finish with a normal DFS. - stack.finish(&mut self) + // Now visit all the foreign nodes we remembered earlier. + // For this we go bottom-up, but also allow f_continue to skip entire + // subtrees from being visited if it would be a NOP. + stack.finish_foreign_accesses(&mut self) } - // Applies `f_propagate` to every non-child vertex of the tree (ancestors first). - // - // `f_propagate` should follow the following format: for a given `Node` it updates its - // `Permission` depending on the position relative to `start` (given by an - // `AccessRelatedness`). - // It outputs whether the tree traversal for this subree should continue or not. + /// Like `traverse_this_parents_children_other`, but skips the children of `start`. fn traverse_nonchildren( mut self, start: BorTag, - f_propagate: impl Fn(NodeAppArgs<'_>) -> Result, + f_continue: impl Fn(&NodeAppArgs<'_>) -> ContinueTraversal, + f_propagate: impl Fn(NodeAppArgs<'_>) -> Result<(), InnErr>, err_builder: impl Fn(ErrHandlerArgs<'_, InnErr>) -> OutErr, ) -> Result<(), OutErr> { let start_idx = self.tag_mapping.get(&start).unwrap(); - let mut stack = TreeVisitorStack::new(start_idx, f_propagate, err_builder); - stack.push_and_visit_strict_ancestors(&mut self)?; - // We *don't* visit the `start` tag, and we don't push its children. - // Only finish the DFS with the cousins. - stack.finish(&mut self) + let mut stack = TreeVisitorStack::new(start_idx, f_continue, f_propagate, err_builder); + // Visits the accessed node itself, and all its parents, i.e. all nodes + // undergoing a child access. Also pushes the other cousin nodes to the + // stack, but not the children of the accessed node. + stack.go_upwards_from_accessed( + &mut self, + start_idx, + ChildrenVisitMode::SkipChildrenOfAccessed, + )?; + // Now visit all the foreign nodes we remembered earlier. + // For this we go bottom-up, but also allow f_continue to skip entire + // subtrees from being visited if it would be a NOP. + stack.finish_foreign_accesses(&mut self) } } @@ -538,16 +667,18 @@ impl<'tcx> Tree { )?; 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( + .traverse_this_parents_children_other( tag, - |args: NodeAppArgs<'_>| -> Result { + // visit all children, skipping none + |_| ContinueTraversal::Recurse, + |args: NodeAppArgs<'_>| -> Result<(), TransitionError> { let NodeAppArgs { node, .. } = args; if global.borrow().protected_tags.get(&node.tag) == Some(&ProtectorKind::StrongProtector) { Err(TransitionError::ProtectedDealloc) } else { - Ok(ContinueTraversal::Recurse) + Ok(()) } }, |args: ErrHandlerArgs<'_, TransitionError>| -> InterpError<'tcx> { @@ -604,19 +735,28 @@ impl<'tcx> Tree { // // `perms_range` is only for diagnostics (it is the range of // the `RangeMap` on which we are currently working). + let node_skipper = |access_kind: AccessKind, args: &NodeAppArgs<'_>| -> ContinueTraversal { + let NodeAppArgs { node, perm, rel_pos } = args; + + let old_state = perm + .get() + .copied() + .unwrap_or_else(|| LocationState::new_uninit(node.default_initial_perm)); + // FIXME: See #3684 + let _would_skip_if_not_for_fixme = old_state.skip_if_known_noop(access_kind, *rel_pos); + ContinueTraversal::Recurse + }; let node_app = |perms_range: Range, access_kind: AccessKind, access_cause: diagnostics::AccessCause, args: NodeAppArgs<'_>| - -> Result { + -> Result<(), TransitionError> { let NodeAppArgs { node, mut perm, rel_pos } = args; let old_state = perm.or_insert(LocationState::new_uninit(node.default_initial_perm)); - match old_state.skip_if_known_noop(access_kind, rel_pos) { - ContinueTraversal::SkipChildren => return Ok(ContinueTraversal::SkipChildren), - _ => {} - } + // FIXME: See #3684 + // old_state.record_new_access(access_kind, rel_pos); let protected = global.borrow().protected_tags.contains_key(&node.tag); let transition = old_state.perform_access(access_kind, rel_pos, protected)?; @@ -631,7 +771,7 @@ impl<'tcx> Tree { span, }); } - Ok(ContinueTraversal::Recurse) + Ok(()) }; // Error handler in case `node_app` goes wrong. @@ -658,8 +798,9 @@ impl<'tcx> Tree { 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( + .traverse_this_parents_children_other( tag, + |args| node_skipper(access_kind, args), |args| node_app(perms_range.clone(), access_kind, access_cause, args), |args| err_handler(perms_range.clone(), access_cause, args), )?; @@ -686,6 +827,7 @@ impl<'tcx> Tree { TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms } .traverse_nonchildren( tag, + |args| node_skipper(access_kind, args), |args| node_app(perms_range.clone(), access_kind, access_cause, args), |args| err_handler(perms_range.clone(), access_cause, args), )?; @@ -714,6 +856,60 @@ impl Tree { node.children.is_empty() && !live.contains(&node.tag) } + /// Checks whether a node can be replaced by its only child. + /// If so, returns the index of said only child. + /// If not, returns none. + fn can_be_replaced_by_single_child( + &self, + idx: UniIndex, + live: &FxHashSet, + ) -> Option { + let node = self.nodes.get(idx).unwrap(); + + // We never want to replace the root node, as it is also kept in `root_ptr_tags`. + if node.children.len() != 1 || live.contains(&node.tag) || node.parent.is_none() { + return None; + } + // Since protected nodes are never GC'd (see `borrow_tracker::FrameExtra::visit_provenance`), + // we know that `node` is not protected because otherwise `live` would + // have contained `node.tag`. + let child_idx = node.children[0]; + let child = self.nodes.get(child_idx).unwrap(); + // Check that for that one child, `can_be_replaced_by_child` holds for the permission + // on all locations. + for (_, data) in self.rperms.iter_all() { + let parent_perm = + data.get(idx).map(|x| x.permission).unwrap_or_else(|| node.default_initial_perm); + let child_perm = data + .get(child_idx) + .map(|x| x.permission) + .unwrap_or_else(|| child.default_initial_perm); + if !parent_perm.can_be_replaced_by_child(child_perm) { + return None; + } + } + + Some(child_idx) + } + + /// Properly removes a node. + /// The node to be removed should not otherwise be usable. It also + /// should have no children, but this is not checked, so that nodes + /// whose children were rotated somewhere else can be deleted without + /// having to first modify them to clear that array. + fn remove_useless_node(&mut self, this: UniIndex) { + // Due to the API of UniMap we must make sure to call + // `UniValMap::remove` for the key of this node on *all* maps that used it + // (which are `self.nodes` and every range of `self.rperms`) + // before we can safely apply `UniKeyMap::remove` to truly remove + // this tag from the `tag_mapping`. + let node = self.nodes.remove(this).unwrap(); + for (_perms_range, perms) in self.rperms.iter_mut_all() { + perms.remove(this); + } + self.tag_mapping.remove(&node.tag); + } + /// Traverses the entire tree looking for useless tags. /// Removes from the tree all useless child nodes of root. /// It will not delete the root itself. @@ -754,29 +950,30 @@ impl Tree { // list of remaining children back in. let mut children_of_node = mem::take(&mut self.nodes.get_mut(*tag).unwrap().children); - // Remove all useless children, and save them for later. - // The closure needs `&self` and the loop below needs `&mut self`, so we need to `collect` - // in to a temporary list. - let to_remove: Vec<_> = - children_of_node.drain_filter(|x| self.is_useless(*x, live)).collect(); + // Remove all useless children. + children_of_node.retain_mut(|idx| { + if self.is_useless(*idx, live) { + // Delete `idx` node everywhere else. + self.remove_useless_node(*idx); + // And delete it from children_of_node. + false + } else { + if let Some(nextchild) = self.can_be_replaced_by_single_child(*idx, live) { + // `nextchild` is our grandchild, and will become our direct child. + // Delete the in-between node, `idx`. + self.remove_useless_node(*idx); + // Set the new child's parent. + self.nodes.get_mut(nextchild).unwrap().parent = Some(*tag); + // Save the new child in children_of_node. + *idx = nextchild; + } + // retain it + true + } + }); // Put back the now-filtered vector. self.nodes.get_mut(*tag).unwrap().children = children_of_node; - // Now, all that is left is unregistering the children saved in `to_remove`. - for idx in to_remove { - // Note: In the rest of this comment, "this node" refers to `idx`. - // This node has no more children (if there were any, they have already been removed). - // It is also unreachable as determined by the GC, so we can remove it everywhere. - // Due to the API of UniMap we must make sure to call - // `UniValMap::remove` for the key of this node on *all* maps that used it - // (which are `self.nodes` and every range of `self.rperms`) - // before we can safely apply `UniKeyMap::remove` to truly remove - // this tag from the `tag_mapping`. - let node = self.nodes.remove(idx).unwrap(); - for (_perms_range, perms) in self.rperms.iter_mut_all() { - perms.remove(idx); - } - self.tag_mapping.remove(&node.tag); - } + // We are done, the parent can continue. stack.pop(); continue; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs index 654419c099d..f64f7bf8e8c 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs @@ -64,6 +64,71 @@ fn all_read_accesses_commute() { } } +fn as_foreign_or_child(related: AccessRelatedness) -> &'static str { + if related.is_foreign() { "foreign" } else { "child" } +} + +fn as_protected(b: bool) -> &'static str { + if b { " (protected)" } else { "" } +} + +fn as_lazy_or_init(b: bool) -> &'static str { + if b { "initialized" } else { "lazy" } +} + +/// Test that tree compacting (as performed by the GC) is sound. +/// Specifically, the GC will replace a parent by a child if the parent is not +/// protected, and if `can_be_replaced_by_child(parent, child)` is true. +/// To check that this is sound, the function must be a simulation, i.e. +/// if both are accessed, the results must still be in simulation, and also +/// if an access is UB, it must also be UB if done only at the child. +#[test] +fn tree_compacting_is_sound() { + // The parent is unprotected + let parent_protected = false; + for ([parent, child], child_protected) in <([LocationState; 2], bool)>::exhaustive() { + if child_protected { + precondition!(child.compatible_with_protector()) + } + precondition!(parent.permission().can_be_replaced_by_child(child.permission())); + for (kind, rel) in <(AccessKind, AccessRelatedness)>::exhaustive() { + let new_parent = parent.perform_access_no_fluff(kind, rel, parent_protected); + let new_child = child.perform_access_no_fluff(kind, rel, child_protected); + match (new_parent, new_child) { + (Some(np), Some(nc)) => { + assert!( + np.permission().can_be_replaced_by_child(nc.permission()), + "`can_be_replaced_by_child` is not a simulation: on a {} {} to a {} parent and a {} {}{} child, the parent becomes {}, the child becomes {}, and these are not in simulation!", + as_foreign_or_child(rel), + kind, + parent.permission(), + as_lazy_or_init(child.is_initialized()), + child.permission(), + as_protected(child_protected), + np.permission(), + nc.permission() + ) + } + (_, None) => { + // the child produced UB, this is fine no matter what the parent does + } + (None, Some(nc)) => { + panic!( + "`can_be_replaced_by_child` does not have the UB property: on a {} {} to a(n) {} parent and a(n) {} {}{} child, only the parent causes UB, while the child becomes {}, and it is not allowed for only the parent to cause UB!", + as_foreign_or_child(rel), + kind, + parent.permission(), + as_lazy_or_init(child.is_initialized()), + child.permission(), + as_protected(child_protected), + nc.permission() + ) + } + } + } + } +} + #[test] #[rustfmt::skip] // Ensure that of 2 accesses happen, one foreign and one a child, and we are protected, that we diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs index 92bae6203b3..cbc25724cb6 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs @@ -221,6 +221,10 @@ impl<'a, V> UniEntry<'a, V> { } self.inner.as_mut().unwrap() } + + pub fn get(&self) -> Option<&V> { + self.inner.as_ref() + } } mod tests { diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index a4d3e3f7af3..87ce4c0c931 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -172,6 +172,8 @@ pub enum BlockReason { Futex { addr: u64 }, /// Blocked on an InitOnce. InitOnce(InitOnceId), + /// Blocked on epoll. + Epoll, } /// The state of a thread. diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index f796e845a23..ece393caf9a 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -35,6 +35,7 @@ clippy::box_default, clippy::needless_question_mark, clippy::needless_lifetimes, + clippy::too_long_first_doc_paragraph, rustc::diagnostic_outside_of_impl, // We are not implementing queries here so it's fine rustc::potential_query_instability, diff --git a/src/tools/miri/src/provenance_gc.rs b/src/tools/miri/src/provenance_gc.rs index 8edd80744dd..d4bed69c670 100644 --- a/src/tools/miri/src/provenance_gc.rs +++ b/src/tools/miri/src/provenance_gc.rs @@ -184,12 +184,35 @@ impl LiveAllocs<'_, '_> { } } +fn remove_unreachable_tags<'tcx>(this: &mut MiriInterpCx<'tcx>, tags: FxHashSet) { + // Avoid iterating all allocations if there's no borrow tracker anyway. + if this.machine.borrow_tracker.is_some() { + this.memory.alloc_map().iter(|it| { + for (_id, (_kind, alloc)) in it { + alloc.extra.borrow_tracker.as_ref().unwrap().remove_unreachable_tags(&tags); + } + }); + } +} + +fn remove_unreachable_allocs<'tcx>(this: &mut MiriInterpCx<'tcx>, allocs: FxHashSet) { + let allocs = LiveAllocs { ecx: this, collected: allocs }; + this.machine.allocation_spans.borrow_mut().retain(|id, _| allocs.is_live(*id)); + this.machine.symbolic_alignment.borrow_mut().retain(|id, _| allocs.is_live(*id)); + this.machine.alloc_addresses.borrow_mut().remove_unreachable_allocs(&allocs); + if let Some(borrow_tracker) = &this.machine.borrow_tracker { + borrow_tracker.borrow_mut().remove_unreachable_allocs(&allocs); + } + // Clean up core (non-Miri-specific) state. + this.remove_unreachable_allocs(&allocs.collected); +} + impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { fn run_provenance_gc(&mut self) { - // We collect all tags from various parts of the interpreter, but also let this = self.eval_context_mut(); + // We collect all tags and AllocId from every part of the interpreter. let mut tags = FxHashSet::default(); let mut alloc_ids = FxHashSet::default(); this.visit_provenance(&mut |id, tag| { @@ -200,30 +223,9 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { tags.insert(tag); } }); - self.remove_unreachable_tags(tags); - self.remove_unreachable_allocs(alloc_ids); - } - fn remove_unreachable_tags(&mut self, tags: FxHashSet) { - let this = self.eval_context_mut(); - this.memory.alloc_map().iter(|it| { - for (_id, (_kind, alloc)) in it { - if let Some(bt) = &alloc.extra.borrow_tracker { - bt.remove_unreachable_tags(&tags); - } - } - }); - } - - fn remove_unreachable_allocs(&mut self, allocs: FxHashSet) { - let this = self.eval_context_mut(); - let allocs = LiveAllocs { ecx: this, collected: allocs }; - this.machine.allocation_spans.borrow_mut().retain(|id, _| allocs.is_live(*id)); - this.machine.symbolic_alignment.borrow_mut().retain(|id, _| allocs.is_live(*id)); - this.machine.alloc_addresses.borrow_mut().remove_unreachable_allocs(&allocs); - if let Some(borrow_tracker) = &this.machine.borrow_tracker { - borrow_tracker.borrow_mut().remove_unreachable_allocs(&allocs); - } - this.remove_unreachable_allocs(&allocs.collected); + // Based on this, clean up the interpreter state. + remove_unreachable_tags(this, tags); + remove_unreachable_allocs(this, alloc_ids); } } diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index e3b9835e360..3ca5f6bb2df 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -278,6 +278,14 @@ impl WeakFileDescriptionRef { } } +impl VisitProvenance for WeakFileDescriptionRef { + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { + // A weak reference can never be the only reference to some pointer or place. + // Since the actual file description is tracked by strong ref somewhere, + // it is ok to make this a NOP operation. + } +} + /// A unique id for file descriptions. While we could use the address, considering that /// is definitely unique, the address would expose interpreter internal state when used /// for sorting things. So instead we generate a unique id per file description that stays diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index fb1e0afdf9e..ee86cf5f26d 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -2,8 +2,9 @@ use std::cell::RefCell; use std::collections::BTreeMap; use std::io; use std::rc::{Rc, Weak}; +use std::time::Duration; -use crate::shims::unix::fd::{FdId, FileDescriptionRef}; +use crate::shims::unix::fd::{FdId, FileDescriptionRef, WeakFileDescriptionRef}; use crate::shims::unix::*; use crate::*; @@ -19,6 +20,8 @@ struct Epoll { // This is an Rc because EpollInterest need to hold a reference to update // it. ready_list: Rc>>, + /// A list of thread ids blocked on this epoll instance. + thread_id: RefCell>, } /// EpollEventInstance contains information that will be returned by epoll_wait. @@ -58,6 +61,8 @@ pub struct EpollEventInterest { data: u64, /// Ready list of the epoll instance under which this EpollEventInterest is registered. ready_list: Rc>>, + /// The file descriptor value that this EpollEventInterest is registered under. + epfd: i32, } /// EpollReadyEvents reflects the readiness of a file description. @@ -251,6 +256,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("epoll_ctl: encountered unknown unsupported operation {:#x}", op); } + // Throw EINVAL if epfd and fd have the same value. + if epfd_value == fd { + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + return Ok(Scalar::from_i32(-1)); + } + // Check if epfd is a valid epoll file descriptor. let Some(epfd) = this.machine.fds.get(epfd_value) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); @@ -262,10 +274,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut interest_list = epoll_file_description.interest_list.borrow_mut(); let ready_list = &epoll_file_description.ready_list; - let Some(file_descriptor) = this.machine.fds.get(fd) else { + let Some(fd_ref) = this.machine.fds.get(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; - let id = file_descriptor.get_id(); + let id = fd_ref.get_id(); if op == epoll_ctl_add || op == epoll_ctl_mod { // Read event bitmask and data from epoll_event passed by caller. @@ -325,19 +337,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } - let id = file_descriptor.get_id(); // Create an epoll_interest. let interest = Rc::new(RefCell::new(EpollEventInterest { file_descriptor: fd, events, data, ready_list: Rc::clone(ready_list), + epfd: epfd_value, })); if op == epoll_ctl_add { // Insert an epoll_interest to global epoll_interest list. this.machine.epoll_interests.insert_epoll_interest(id, Rc::downgrade(&interest)); - interest_list.insert(epoll_key, interest); + interest_list.insert(epoll_key, Rc::clone(&interest)); } else { // Directly modify the epoll_interest so the global epoll_event_interest table // will be updated too. @@ -346,9 +358,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { epoll_interest.data = data; } - // Readiness will be updated immediately when the epoll_event_interest is added or modified. - this.check_and_update_readiness(&file_descriptor)?; - + // Notification will be returned for current epfd if there is event in the file + // descriptor we registered. + check_and_update_one_event_interest(&fd_ref, interest, id, this)?; return Ok(Scalar::from_i32(0)); } else if op == epoll_ctl_del { let epoll_key = (id, fd); @@ -389,7 +401,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// The `timeout` argument specifies the number of milliseconds that /// `epoll_wait()` will block. Time is measured against the - /// CLOCK_MONOTONIC clock. + /// CLOCK_MONOTONIC clock. If the timeout is zero, the function will not block, + /// while if the timeout is -1, the function will block + /// until at least one event has been retrieved (or an error + /// occurred). /// A call to `epoll_wait()` will block until either: /// • a file descriptor delivers an event; @@ -415,59 +430,100 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { events_op: &OpTy<'tcx>, maxevents: &OpTy<'tcx>, timeout: &OpTy<'tcx>, - ) -> InterpResult<'tcx, Scalar> { + dest: &MPlaceTy<'tcx>, + ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let epfd = this.read_scalar(epfd)?.to_i32()?; + let epfd_value = this.read_scalar(epfd)?.to_i32()?; let events = this.read_immediate(events_op)?; let maxevents = this.read_scalar(maxevents)?.to_i32()?; let timeout = this.read_scalar(timeout)?.to_i32()?; - if epfd <= 0 || maxevents <= 0 { + if epfd_value <= 0 || maxevents <= 0 { let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; - return Ok(Scalar::from_i32(-1)); + this.write_int(-1, dest)?; + return Ok(()); } // This needs to come after the maxevents value check, or else maxevents.try_into().unwrap() // will fail. - let events = this.deref_pointer_as( + let event = this.deref_pointer_as( &events, this.libc_array_ty_layout("epoll_event", maxevents.try_into().unwrap()), )?; - // FIXME: Implement blocking support - if timeout != 0 { - throw_unsup_format!("epoll_wait: timeout value can only be 0"); - } - - let Some(epfd) = this.machine.fds.get(epfd) else { - return Ok(Scalar::from_i32(this.fd_not_found()?)); + let Some(epfd) = this.machine.fds.get(epfd_value) else { + let result_value: i32 = this.fd_not_found()?; + this.write_int(result_value, dest)?; + return Ok(()); }; - let epoll_file_description = epfd - .downcast::() - .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; + // Create a weak ref of epfd and pass it to callback so we will make sure that epfd + // is not close after the thread unblocks. + let weak_epfd = epfd.downgrade(); - let ready_list = epoll_file_description.get_ready_list(); - let mut ready_list = ready_list.borrow_mut(); - let mut num_of_events: i32 = 0; - let mut array_iter = this.project_array_fields(&events)?; - - while let Some(des) = array_iter.next(this)? { - if let Some(epoll_event_instance) = ready_list_next(this, &mut ready_list) { - this.write_int_fields_named( - &[ - ("events", epoll_event_instance.events.into()), - ("u64", epoll_event_instance.data.into()), - ], - &des.1, - )?; - num_of_events = num_of_events.strict_add(1); - } else { - break; - } + // We just need to know if the ready list is empty and borrow the thread_ids out. + // The whole logic is wrapped inside a block so we don't need to manually drop epfd later. + let ready_list_empty; + let mut thread_ids; + { + let epoll_file_description = epfd + .downcast::() + .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; + let binding = epoll_file_description.get_ready_list(); + ready_list_empty = binding.borrow_mut().is_empty(); + thread_ids = epoll_file_description.thread_id.borrow_mut(); } - Ok(Scalar::from_i32(num_of_events)) + if timeout == 0 || !ready_list_empty { + // If the ready list is not empty, or the timeout is 0, we can return immediately. + blocking_epoll_callback(epfd_value, weak_epfd, dest, &event, this)?; + } else { + // Blocking + let timeout = match timeout { + 0.. => { + let duration = Duration::from_millis(timeout.try_into().unwrap()); + Some((TimeoutClock::Monotonic, TimeoutAnchor::Relative, duration)) + } + -1 => None, + ..-1 => { + throw_unsup_format!( + "epoll_wait: Only timeout values greater than or equal to -1 are supported." + ); + } + }; + thread_ids.push(this.active_thread()); + let dest = dest.clone(); + this.block_thread( + BlockReason::Epoll, + timeout, + callback!( + @capture<'tcx> { + epfd_value: i32, + weak_epfd: WeakFileDescriptionRef, + dest: MPlaceTy<'tcx>, + event: MPlaceTy<'tcx>, + } + @unblock = |this| { + blocking_epoll_callback(epfd_value, weak_epfd, &dest, &event, this)?; + Ok(()) + } + @timeout = |this| { + // No notification after blocking timeout. + let Some(epfd) = weak_epfd.upgrade() else { + throw_unsup_format!("epoll FD {epfd_value} got closed while blocking.") + }; + // Remove the current active thread_id from the blocked thread_id list. + epfd.downcast::() + .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))? + .thread_id.borrow_mut() + .retain(|&id| id != this.active_thread()); + this.write_int(0, &dest)?; + Ok(()) + } + ), + ); + } + Ok(()) } /// For a specific file description, get its ready events and update the corresponding ready @@ -477,33 +533,47 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// /// This *will* report an event if anyone is subscribed to it, without any further filtering, so /// do not call this function when an FD didn't have anything happen to it! - fn check_and_update_readiness(&self, fd_ref: &FileDescriptionRef) -> InterpResult<'tcx, ()> { - let this = self.eval_context_ref(); + fn check_and_update_readiness( + &mut self, + fd_ref: &FileDescriptionRef, + ) -> InterpResult<'tcx, ()> { + let this = self.eval_context_mut(); let id = fd_ref.get_id(); + let mut waiter = Vec::new(); // Get a list of EpollEventInterest that is associated to a specific file description. if let Some(epoll_interests) = this.machine.epoll_interests.get_epoll_interest(id) { - let epoll_ready_events = fd_ref.get_epoll_ready_events()?; - // Get the bitmask of ready events. - let ready_events = epoll_ready_events.get_event_bitmask(this); - for weak_epoll_interest in epoll_interests { if let Some(epoll_interest) = weak_epoll_interest.upgrade() { - // This checks if any of the events specified in epoll_event_interest.events - // match those in ready_events. - let epoll_event_interest = epoll_interest.borrow(); - let flags = epoll_event_interest.events & ready_events; - // If there is any event that we are interested in being specified as ready, - // insert an epoll_return to the ready list. - if flags != 0 { - let epoll_key = (id, epoll_event_interest.file_descriptor); - let ready_list = &mut epoll_event_interest.ready_list.borrow_mut(); - let event_instance = - EpollEventInstance::new(flags, epoll_event_interest.data); - ready_list.insert(epoll_key, event_instance); + let is_updated = check_and_update_one_event_interest( + fd_ref, + epoll_interest.clone(), + id, + this, + )?; + if is_updated { + // Edge-triggered notification only notify one thread even if there are + // multiple threads block on the same epfd. + let epfd = this.machine.fds.get(epoll_interest.borrow().epfd).unwrap(); + + // This unwrap can never fail because if the current epoll instance were + // closed and its epfd value reused, the upgrade of weak_epoll_interest + // above would fail. This guarantee holds because only the epoll instance + // holds a strong ref to epoll_interest. + // FIXME: We can randomly pick a thread to unblock. + if let Some(thread_id) = + epfd.downcast::().unwrap().thread_id.borrow_mut().pop() + { + waiter.push(thread_id); + }; } } } } + waiter.sort(); + waiter.dedup(); + for thread_id in waiter { + this.unblock_thread(thread_id, BlockReason::Epoll)?; + } Ok(()) } } @@ -525,3 +595,71 @@ fn ready_list_next( } return None; } + +/// This helper function checks whether an epoll notification should be triggered for a specific +/// epoll_interest and, if necessary, triggers the notification, and returns whether the +/// notification was added/updated. Unlike check_and_update_readiness, this function sends a +/// notification to only one epoll instance. +fn check_and_update_one_event_interest<'tcx>( + fd_ref: &FileDescriptionRef, + interest: Rc>, + id: FdId, + ecx: &MiriInterpCx<'tcx>, +) -> InterpResult<'tcx, bool> { + // Get the bitmask of ready events for a file description. + let ready_events_bitmask = fd_ref.get_epoll_ready_events()?.get_event_bitmask(ecx); + let epoll_event_interest = interest.borrow(); + // This checks if any of the events specified in epoll_event_interest.events + // match those in ready_events. + let flags = epoll_event_interest.events & ready_events_bitmask; + // If there is any event that we are interested in being specified as ready, + // insert an epoll_return to the ready list. + if flags != 0 { + let epoll_key = (id, epoll_event_interest.file_descriptor); + let ready_list = &mut epoll_event_interest.ready_list.borrow_mut(); + let event_instance = EpollEventInstance::new(flags, epoll_event_interest.data); + // Triggers the notification by inserting it to the ready list. + ready_list.insert(epoll_key, event_instance); + return Ok(true); + } + return Ok(false); +} + +/// Callback function after epoll_wait unblocks +fn blocking_epoll_callback<'tcx>( + epfd_value: i32, + weak_epfd: WeakFileDescriptionRef, + dest: &MPlaceTy<'tcx>, + events: &MPlaceTy<'tcx>, + ecx: &mut MiriInterpCx<'tcx>, +) -> InterpResult<'tcx> { + let Some(epfd) = weak_epfd.upgrade() else { + throw_unsup_format!("epoll FD {epfd_value} got closed while blocking.") + }; + + let epoll_file_description = epfd + .downcast::() + .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; + + let ready_list = epoll_file_description.get_ready_list(); + let mut ready_list = ready_list.borrow_mut(); + let mut num_of_events: i32 = 0; + let mut array_iter = ecx.project_array_fields(events)?; + + while let Some(des) = array_iter.next(ecx)? { + if let Some(epoll_event_instance) = ready_list_next(ecx, &mut ready_list) { + ecx.write_int_fields_named( + &[ + ("events", epoll_event_instance.events.into()), + ("u64", epoll_event_instance.data.into()), + ], + &des.1, + )?; + num_of_events = num_of_events.strict_add(1); + } else { + break; + } + } + ecx.write_int(num_of_events, dest)?; + Ok(()) +} diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 581f0db42e1..d64f13f63d9 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -62,8 +62,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { "epoll_wait" => { let [epfd, events, maxevents, timeout] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let result = this.epoll_wait(epfd, events, maxevents, timeout)?; - this.write_scalar(result, dest)?; + this.epoll_wait(epfd, events, maxevents, timeout, dest)?; } "eventfd" => { let [val, flag] = diff --git a/src/tools/miri/src/shims/unix/unnamed_socket.rs b/src/tools/miri/src/shims/unix/unnamed_socket.rs index 745f27398d0..127aef29244 100644 --- a/src/tools/miri/src/shims/unix/unnamed_socket.rs +++ b/src/tools/miri/src/shims/unix/unnamed_socket.rs @@ -339,7 +339,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let pipefd = this.deref_pointer(pipefd)?; + let pipefd = this.deref_pointer_as(pipefd, this.machine.layouts.i32)?; let flags = match flags { Some(flags) => this.read_scalar(flags)?.to_i32()?, None => 0, diff --git a/src/tools/miri/test_dependencies/Cargo.lock b/src/tools/miri/test_dependencies/Cargo.lock index bbead878223..39d41281728 100644 --- a/src/tools/miri/test_dependencies/Cargo.lock +++ b/src/tools/miri/test_dependencies/Cargo.lock @@ -44,6 +44,12 @@ version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" +[[package]] +name = "bytes" +version = "1.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8318a53db07bb3f8dca91a600466bdb3f2eaadeedfdbcf02e1accbad9271ba50" + [[package]] name = "cc" version = "1.1.7" @@ -304,6 +310,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "daa4fb1bc778bd6f04cbfc4bb2d06a7396a8f299dc33ea1900cedaa316f467b1" dependencies = [ "backtrace", + "bytes", "libc", "mio", "pin-project-lite", diff --git a/src/tools/miri/test_dependencies/Cargo.toml b/src/tools/miri/test_dependencies/Cargo.toml index ce11a8abb0e..c24422df26c 100644 --- a/src/tools/miri/test_dependencies/Cargo.toml +++ b/src/tools/miri/test_dependencies/Cargo.toml @@ -20,7 +20,7 @@ tempfile = "3" page_size = "0.6" # Avoid pulling in all of tokio's dependencies. # However, without `net` and `signal`, tokio uses fewer relevant system APIs. -tokio = { version = "1.24", features = ["macros", "rt-multi-thread", "time", "net", "fs", "sync", "signal"] } +tokio = { version = "1.24", features = ["macros", "rt-multi-thread", "time", "net", "fs", "sync", "signal", "io-util"] } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.52", features = [ "Win32_Foundation", "Win32_System_Threading" ] } diff --git a/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.rs b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.rs new file mode 100644 index 00000000000..fb2426f7b66 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.rs @@ -0,0 +1,18 @@ +//@only-target-linux + +// This is a test for registering unsupported fd with epoll. +// Register epoll fd with epoll is allowed in real system, but we do not support this. +fn main() { + // Create two epoll instance. + let epfd0 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd0, -1); + let epfd1 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd1, -1); + + // Register epoll with epoll. + let mut ev = + libc::epoll_event { events: (libc::EPOLLIN | libc::EPOLLET) as _, u64: epfd1 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd0, libc::EPOLL_CTL_ADD, epfd1, &mut ev) }; + //~^ERROR: epoll does not support this file description + assert_eq!(res, 0); +} diff --git a/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.stderr b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.stderr new file mode 100644 index 00000000000..6f9b988d4ee --- /dev/null +++ b/src/tools/miri/tests/fail-dep/libc/libc_epoll_unsupported_fd.stderr @@ -0,0 +1,14 @@ +error: unsupported operation: epoll: epoll does not support this file description + --> $DIR/libc_epoll_unsupported_fd.rs:LL:CC + | +LL | let res = unsafe { libc::epoll_ctl(epfd0, libc::EPOLL_CTL_ADD, epfd1, &mut ev) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ epoll: epoll does not support this file description + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/libc_epoll_unsupported_fd.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail-dep/tokio/sleep.rs b/src/tools/miri/tests/fail-dep/tokio/sleep.rs deleted file mode 100644 index 0fa5080d484..00000000000 --- a/src/tools/miri/tests/fail-dep/tokio/sleep.rs +++ /dev/null @@ -1,14 +0,0 @@ -//@compile-flags: -Zmiri-permissive-provenance -Zmiri-backtrace=full -//@only-target-x86_64-unknown-linux: support for tokio only on linux and x86 -//@error-in-other-file: timeout value can only be 0 -//@normalize-stderr-test: " += note:.*\n" -> "" - -use tokio::time::{sleep, Duration, Instant}; - -#[tokio::main] -async fn main() { - let start = Instant::now(); - sleep(Duration::from_secs(1)).await; - let time_elapsed = &start.elapsed().as_millis(); - assert!((1000..1100).contains(time_elapsed), "{}", time_elapsed); -} diff --git a/src/tools/miri/tests/fail-dep/tokio/sleep.stderr b/src/tools/miri/tests/fail-dep/tokio/sleep.stderr deleted file mode 100644 index d5bf00fc175..00000000000 --- a/src/tools/miri/tests/fail-dep/tokio/sleep.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error: unsupported operation: epoll_wait: timeout value can only be 0 - --> CARGO_REGISTRY/.../epoll.rs:LL:CC - | -LL | / syscall!(epoll_wait( -LL | | self.ep.as_raw_fd(), -LL | | events.as_mut_ptr(), -LL | | events.capacity() as i32, -LL | | timeout, -LL | | )) - | |__________^ epoll_wait: timeout value can only be 0 - | - = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support - -error: aborting due to 1 previous error - diff --git a/src/tools/miri/tests/fail/both_borrows/alias_through_mutation.tree.stderr b/src/tools/miri/tests/fail/both_borrows/alias_through_mutation.tree.stderr index f26e8444a9a..0bd196eab1b 100644 --- a/src/tools/miri/tests/fail/both_borrows/alias_through_mutation.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/alias_through_mutation.tree.stderr @@ -5,24 +5,18 @@ LL | let _val = *target_alias; | ^^^^^^^^^^^^^ read access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child read access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child read access +help: the accessed tag was created here, in the initial state Frozen --> $DIR/alias_through_mutation.rs:LL:CC | LL | *x = &mut *(target as *mut _); | ^^^^^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/alias_through_mutation.rs:LL:CC - | -LL | retarget(&mut target_alias, target); - | ^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/alias_through_mutation.rs:LL:CC | LL | *target = 13; | ^^^^^^^^^^^^ - = help: this transition corresponds to a loss of read and write permissions + = help: this transition corresponds to a loss of read permissions = note: BACKTRACE (of the first span): = note: inside `main` at $DIR/alias_through_mutation.rs:LL:CC diff --git a/src/tools/miri/tests/fail/both_borrows/box_exclusive_violation1.tree.stderr b/src/tools/miri/tests/fail/both_borrows/box_exclusive_violation1.tree.stderr index ee5ef0c5ea8..27d987feb57 100644 --- a/src/tools/miri/tests/fail/both_borrows/box_exclusive_violation1.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/box_exclusive_violation1.tree.stderr @@ -5,19 +5,13 @@ LL | *LEAK = 7; | ^^^^^^^^^ write access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Frozen --> $DIR/box_exclusive_violation1.rs:LL:CC | LL | fn unknown_code_1(x: &i32) { | ^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/box_exclusive_violation1.rs:LL:CC - | -LL | unknown_code_1(&*our); - | ^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/box_exclusive_violation1.rs:LL:CC | LL | *our = 5; diff --git a/src/tools/miri/tests/fail/both_borrows/buggy_as_mut_slice.tree.stderr b/src/tools/miri/tests/fail/both_borrows/buggy_as_mut_slice.tree.stderr index 5593db89971..16e87c4d4eb 100644 --- a/src/tools/miri/tests/fail/both_borrows/buggy_as_mut_slice.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/buggy_as_mut_slice.tree.stderr @@ -5,19 +5,13 @@ LL | v2[1] = 7; | ^^^^^^^^^ write access through at ALLOC[0x4] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved --> $DIR/buggy_as_mut_slice.rs:LL:CC | LL | let v2 = safe::as_mut_slice(&v); | ^^^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/buggy_as_mut_slice.rs:LL:CC - | -LL | unsafe { from_raw_parts_mut(self_.as_ptr() as *mut T, self_.len()) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x4..0x8] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x4..0x8] --> $DIR/buggy_as_mut_slice.rs:LL:CC | LL | v1[1] = 5; diff --git a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.tree.stderr b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.tree.stderr index 7d3725d611a..aaf7c2256a7 100644 --- a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.tree.stderr @@ -5,19 +5,13 @@ LL | b[1] = 6; | ^^^^^^^^ write access through at ALLOC[0x4] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved --> $DIR/buggy_split_at_mut.rs:LL:CC | LL | let (a, b) = safe::split_at_mut(&mut array, 0); | ^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/buggy_split_at_mut.rs:LL:CC - | -LL | from_raw_parts_mut(ptr.offset(mid as isize), len - mid), - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x4..0x8] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x4..0x8] --> $DIR/buggy_split_at_mut.rs:LL:CC | LL | a[1] = 5; diff --git a/src/tools/miri/tests/fail/both_borrows/illegal_write5.tree.stderr b/src/tools/miri/tests/fail/both_borrows/illegal_write5.tree.stderr index 1f3d6e387d4..dec2bb6880f 100644 --- a/src/tools/miri/tests/fail/both_borrows/illegal_write5.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/illegal_write5.tree.stderr @@ -5,19 +5,13 @@ LL | let _val = *xref; | ^^^^^ read access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child read access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child read access +help: the accessed tag was created here, in the initial state Reserved --> $DIR/illegal_write5.rs:LL:CC | LL | let xref = unsafe { &mut *xraw }; | ^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/illegal_write5.rs:LL:CC - | -LL | let xref = unsafe { &mut *xraw }; - | ^^^^^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/illegal_write5.rs:LL:CC | LL | unsafe { *xraw = 15 }; diff --git a/src/tools/miri/tests/fail/both_borrows/load_invalid_shr.tree.stderr b/src/tools/miri/tests/fail/both_borrows/load_invalid_shr.tree.stderr index 292dc8150f1..0fcfc20e436 100644 --- a/src/tools/miri/tests/fail/both_borrows/load_invalid_shr.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/load_invalid_shr.tree.stderr @@ -5,19 +5,13 @@ LL | let _val = *xref_in_mem; | ^^^^^^^^^^^^ reborrow through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this reborrow (acting as a child read access) -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this reborrow (acting as a child read access) +help: the accessed tag was created here, in the initial state Frozen --> $DIR/load_invalid_shr.rs:LL:CC | LL | let xref_in_mem = Box::new(xref); | ^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/load_invalid_shr.rs:LL:CC - | -LL | let xref = unsafe { &*xraw }; - | ^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/load_invalid_shr.rs:LL:CC | LL | unsafe { *xraw = 42 }; // unfreeze diff --git a/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation1.tree.stderr b/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation1.tree.stderr index a5e62e9ea33..5b4ee4a8913 100644 --- a/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation1.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation1.tree.stderr @@ -5,19 +5,13 @@ LL | *LEAK = 7; | ^^^^^^^^^ write access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Frozen --> $DIR/mut_exclusive_violation1.rs:LL:CC | LL | fn unknown_code_1(x: &i32) { | ^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/mut_exclusive_violation1.rs:LL:CC - | -LL | unknown_code_1(&*our); - | ^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/mut_exclusive_violation1.rs:LL:CC | LL | *our = 5; diff --git a/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation2.tree.stderr b/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation2.tree.stderr index 6b6eecbe413..d3bc54b7ef3 100644 --- a/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation2.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation2.tree.stderr @@ -5,19 +5,13 @@ LL | *raw1 = 3; | ^^^^^^^^^ write access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved --> $DIR/mut_exclusive_violation2.rs:LL:CC | LL | let raw1 = ptr1.as_mut(); | ^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/mut_exclusive_violation2.rs:LL:CC - | -LL | let raw1 = ptr1.as_mut(); - | ^^^^^^^^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/mut_exclusive_violation2.rs:LL:CC | LL | *raw2 = 2; diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.tree.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.tree.stderr index f2eaf767a47..f687b3f867f 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.tree.stderr @@ -5,19 +5,13 @@ LL | foo(some_xref); | ^^^^^^^^^ reborrow through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this reborrow (acting as a child read access) -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this reborrow (acting as a child read access) +help: the accessed tag was created here, in the initial state Frozen --> $DIR/pass_invalid_shr_option.rs:LL:CC | LL | let some_xref = unsafe { Some(&*xraw) }; | ^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/pass_invalid_shr_option.rs:LL:CC - | -LL | let some_xref = unsafe { Some(&*xraw) }; - | ^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/pass_invalid_shr_option.rs:LL:CC | LL | unsafe { *xraw = 42 }; // unfreeze diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.tree.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.tree.stderr index 9cfd4239c85..845838f93d5 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.tree.stderr @@ -5,19 +5,13 @@ LL | foo(pair_xref); | ^^^^^^^^^ reborrow through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this reborrow (acting as a child read access) -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this reborrow (acting as a child read access) +help: the accessed tag was created here, in the initial state Frozen --> $DIR/pass_invalid_shr_tuple.rs:LL:CC | LL | let pair_xref = unsafe { (&*xraw0, &*xraw1) }; | ^^^^^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/pass_invalid_shr_tuple.rs:LL:CC - | -LL | let pair_xref = unsafe { (&*xraw0, &*xraw1) }; - | ^^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x4] --> $DIR/pass_invalid_shr_tuple.rs:LL:CC | LL | unsafe { *xraw0 = 42 }; // unfreeze diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.tree.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.tree.stderr index 1169200b57c..e7703729251 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.tree.stderr @@ -5,19 +5,13 @@ LL | ret | ^^^ reborrow through at ALLOC[0x4] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this reborrow (acting as a child read access) -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this reborrow (acting as a child read access) +help: the accessed tag was created here, in the initial state Frozen --> $DIR/return_invalid_shr_option.rs:LL:CC | LL | let ret = Some(unsafe { &(*xraw).1 }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/return_invalid_shr_option.rs:LL:CC - | -LL | let ret = Some(unsafe { &(*xraw).1 }); - | ^^^^^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8] --> $DIR/return_invalid_shr_option.rs:LL:CC | LL | unsafe { *xraw = (42, 23) }; // unfreeze diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.tree.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.tree.stderr index af410534140..a5c6be3f13a 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.tree.stderr @@ -5,19 +5,13 @@ LL | ret | ^^^ reborrow through at ALLOC[0x4] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this reborrow (acting as a child read access) -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this reborrow (acting as a child read access) +help: the accessed tag was created here, in the initial state Frozen --> $DIR/return_invalid_shr_tuple.rs:LL:CC | LL | let ret = (unsafe { &(*xraw).1 },); | ^^^^^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/return_invalid_shr_tuple.rs:LL:CC - | -LL | let ret = (unsafe { &(*xraw).1 },); - | ^^^^^^^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x8] --> $DIR/return_invalid_shr_tuple.rs:LL:CC | LL | unsafe { *xraw = (42, 23) }; // unfreeze diff --git a/src/tools/miri/tests/fail/both_borrows/shr_frozen_violation1.tree.stderr b/src/tools/miri/tests/fail/both_borrows/shr_frozen_violation1.tree.stderr index 97cd64c03be..45b3bceb728 100644 --- a/src/tools/miri/tests/fail/both_borrows/shr_frozen_violation1.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/shr_frozen_violation1.tree.stderr @@ -5,18 +5,12 @@ LL | *(x as *const i32 as *mut i32) = 7; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ write access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Frozen which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Frozen which forbids this child write access +help: the accessed tag was created here, in the initial state Frozen --> $DIR/shr_frozen_violation1.rs:LL:CC | LL | fn unknown_code(x: &i32) { | ^ -help: the conflicting tag was created here, in the initial state Frozen - --> $DIR/shr_frozen_violation1.rs:LL:CC - | -LL | unknown_code(&*x); - | ^^^ = note: BACKTRACE (of the first span): = note: inside `unknown_code` at $DIR/shr_frozen_violation1.rs:LL:CC note: inside `foo` diff --git a/src/tools/miri/tests/panic/target_feature_wasm.rs b/src/tools/miri/tests/fail/function_calls/target_feature_wasm.rs similarity index 79% rename from src/tools/miri/tests/panic/target_feature_wasm.rs rename to src/tools/miri/tests/fail/function_calls/target_feature_wasm.rs index c67d2983f78..bd400e8824a 100644 --- a/src/tools/miri/tests/panic/target_feature_wasm.rs +++ b/src/tools/miri/tests/fail/function_calls/target_feature_wasm.rs @@ -1,4 +1,4 @@ -//@only-target-wasm32: tests WASM-specific behavior +//@only-target-wasm: tests WASM-specific behavior //@compile-flags: -C target-feature=-simd128 fn main() { @@ -6,7 +6,7 @@ fn main() { // But if the compiler actually uses the target feature, it will lead to an error when the module is loaded. // We emulate this with an "unsupported" error. assert!(!cfg!(target_feature = "simd128")); - simd128_fn(); + simd128_fn(); //~ERROR: unavailable target features } #[target_feature(enable = "simd128")] diff --git a/src/tools/miri/tests/fail/function_calls/target_feature_wasm.stderr b/src/tools/miri/tests/fail/function_calls/target_feature_wasm.stderr new file mode 100644 index 00000000000..dc0aca77f9e --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/target_feature_wasm.stderr @@ -0,0 +1,13 @@ +error: abnormal termination: calling a function that requires unavailable target features: simd128 + --> $DIR/target_feature_wasm.rs:LL:CC + | +LL | simd128_fn(); + | ^^^^^^^^^^^^ calling a function that requires unavailable target features: simd128 + | + = note: BACKTRACE: + = note: inside `main` at $DIR/target_feature_wasm.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs b/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs index 860798f2ab1..eb5a16360ff 100644 --- a/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs +++ b/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs @@ -7,7 +7,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm // Explicitly disable SSE4.1 because it is enabled by default on macOS //@compile-flags: -C target-feature=-sse4.1 diff --git a/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr b/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr index 98371cbe7a2..bd969d089c3 100644 --- a/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr @@ -5,25 +5,19 @@ LL | *y += 1; // Failure | ^^^^^^^ write access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Frozen which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Frozen which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved --> $DIR/alternate-read-write.rs:LL:CC | LL | let y = unsafe { &mut *(x as *mut u8) }; | ^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/alternate-read-write.rs:LL:CC - | -LL | let y = unsafe { &mut *(x as *mut u8) }; - | ^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag later transitioned to Active due to a child write access at offsets [0x0..0x1] +help: the accessed tag later transitioned to Active due to a child write access at offsets [0x0..0x1] --> $DIR/alternate-read-write.rs:LL:CC | LL | *y += 1; // Success | ^^^^^^^ = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference -help: the conflicting tag later transitioned to Frozen due to a foreign read access at offsets [0x0..0x1] +help: the accessed tag later transitioned to Frozen due to a foreign read access at offsets [0x0..0x1] --> $DIR/alternate-read-write.rs:LL:CC | LL | let _val = *x; diff --git a/src/tools/miri/tests/fail/tree_borrows/children-can-alias.uniq.stderr b/src/tools/miri/tests/fail/tree_borrows/children-can-alias.uniq.stderr index cc1fea30f53..cdfa8a74238 100644 --- a/src/tools/miri/tests/fail/tree_borrows/children-can-alias.uniq.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/children-can-alias.uniq.stderr @@ -5,19 +5,13 @@ LL | child2.write(2); | ^^^^^^^^^^^^^^^ write access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child write access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved --> $DIR/children-can-alias.rs:LL:CC | LL | let child2 = x.as_ptr(); | ^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/children-can-alias.rs:LL:CC - | -LL | let child2 = x.as_ptr(); - | ^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x1] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x1] --> $DIR/children-can-alias.rs:LL:CC | LL | child1.write(1); diff --git a/src/tools/miri/tests/fail/tree_borrows/error-range.stderr b/src/tools/miri/tests/fail/tree_borrows/error-range.stderr index 37f0b9b62b0..090ae4507bd 100644 --- a/src/tools/miri/tests/fail/tree_borrows/error-range.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/error-range.stderr @@ -5,19 +5,13 @@ LL | rmut[5] += 1; | ^^^^^^^^^^^^ read access through at ALLOC[0x5] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Disabled which forbids this child read access -help: the accessed tag was created here + = help: the accessed tag has state Disabled which forbids this child read access +help: the accessed tag was created here, in the initial state Reserved --> $DIR/error-range.rs:LL:CC | LL | let rmut = &mut *addr_of_mut!(data[0..6]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: the conflicting tag was created here, in the initial state Reserved - --> $DIR/error-range.rs:LL:CC - | -LL | let rmut = &mut *addr_of_mut!(data[0..6]); - | ^^^^ -help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x5..0x6] +help: the accessed tag later transitioned to Disabled due to a foreign write access at offsets [0x5..0x6] --> $DIR/error-range.rs:LL:CC | LL | data[5] = 1; diff --git a/src/tools/miri/tests/fail/tree_borrows/repeated_foreign_read_lazy_conflicted.rs b/src/tools/miri/tests/fail/tree_borrows/repeated_foreign_read_lazy_conflicted.rs new file mode 100644 index 00000000000..36b47a33b18 --- /dev/null +++ b/src/tools/miri/tests/fail/tree_borrows/repeated_foreign_read_lazy_conflicted.rs @@ -0,0 +1,23 @@ +//@compile-flags: -Zmiri-tree-borrows + +use std::ptr::addr_of_mut; + +fn do_something(_: u8) {} + +unsafe fn access_after_sub_1(x: &mut u8, orig_ptr: *mut u8) { + // causes a second access, which should make the lazy part of `x` be `Reserved {conflicted: true}` + do_something(*orig_ptr); + // read from the conflicted pointer + *(x as *mut u8).byte_sub(1) = 42; //~ ERROR: /write access through .* is forbidden/ +} + +pub fn main() { + unsafe { + let mut alloc = [0u8, 0u8]; + let orig_ptr = addr_of_mut!(alloc) as *mut u8; + let foo = &mut *orig_ptr; + // cause a foreign read access to foo + do_something(alloc[0]); + access_after_sub_1(&mut *(foo as *mut u8).byte_add(1), orig_ptr); + } +} diff --git a/src/tools/miri/tests/fail/tree_borrows/repeated_foreign_read_lazy_conflicted.stderr b/src/tools/miri/tests/fail/tree_borrows/repeated_foreign_read_lazy_conflicted.stderr new file mode 100644 index 00000000000..963e8e5eca9 --- /dev/null +++ b/src/tools/miri/tests/fail/tree_borrows/repeated_foreign_read_lazy_conflicted.stderr @@ -0,0 +1,31 @@ +error: Undefined Behavior: write access through at ALLOC[0x0] is forbidden + --> $DIR/repeated_foreign_read_lazy_conflicted.rs:LL:CC + | +LL | *(x as *mut u8).byte_sub(1) = 42; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ write access through at ALLOC[0x0] is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag has state Reserved (conflicted) which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved + --> $DIR/repeated_foreign_read_lazy_conflicted.rs:LL:CC + | +LL | unsafe fn access_after_sub_1(x: &mut u8, orig_ptr: *mut u8) { + | ^ +help: the accessed tag later transitioned to Reserved (conflicted) due to a foreign read access at offsets [0x0..0x1] + --> $DIR/repeated_foreign_read_lazy_conflicted.rs:LL:CC + | +LL | do_something(*orig_ptr); + | ^^^^^^^^^ + = help: this transition corresponds to a temporary loss of write permissions until function exit + = note: BACKTRACE (of the first span): + = note: inside `access_after_sub_1` at $DIR/repeated_foreign_read_lazy_conflicted.rs:LL:CC +note: inside `main` + --> $DIR/repeated_foreign_read_lazy_conflicted.rs:LL:CC + | +LL | access_after_sub_1(&mut *(foo as *mut u8).byte_add(1), orig_ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/tree_borrows/strongly-protected.rs b/src/tools/miri/tests/fail/tree_borrows/strongly-protected.rs index 484c7c3bbff..037031d0345 100644 --- a/src/tools/miri/tests/fail/tree_borrows/strongly-protected.rs +++ b/src/tools/miri/tests/fail/tree_borrows/strongly-protected.rs @@ -1,14 +1,15 @@ //@compile-flags: -Zmiri-tree-borrows //@error-in-other-file: /deallocation through .* is forbidden/ -fn inner(x: &mut i32, f: fn(&mut i32)) { +fn inner(x: &mut i32, f: fn(*mut i32)) { // `f` may mutate, but it may not deallocate! + // `f` takes a raw pointer so that the only protector + // is that on `x` f(x) } fn main() { - inner(Box::leak(Box::new(0)), |x| { - let raw = x as *mut _; + inner(Box::leak(Box::new(0)), |raw| { drop(unsafe { Box::from_raw(raw) }); }); } diff --git a/src/tools/miri/tests/fail/tree_borrows/strongly-protected.stderr b/src/tools/miri/tests/fail/tree_borrows/strongly-protected.stderr index f0afcc7b3ff..9da43055af2 100644 --- a/src/tools/miri/tests/fail/tree_borrows/strongly-protected.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/strongly-protected.stderr @@ -15,7 +15,7 @@ LL | drop(unsafe { Box::from_raw(raw) }); help: the strongly protected tag was created here, in the initial state Reserved --> $DIR/strongly-protected.rs:LL:CC | -LL | fn inner(x: &mut i32, f: fn(&mut i32)) { +LL | fn inner(x: &mut i32, f: fn(*mut i32)) { | ^ = note: BACKTRACE (of the first span): = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC @@ -28,7 +28,7 @@ note: inside closure | LL | drop(unsafe { Box::from_raw(raw) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: inside `<{closure@$DIR/strongly-protected.rs:LL:CC} as std::ops::FnOnce<(&mut i32,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC + = note: inside `<{closure@$DIR/strongly-protected.rs:LL:CC} as std::ops::FnOnce<(*mut i32,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC note: inside `inner` --> $DIR/strongly-protected.rs:LL:CC | @@ -37,8 +37,7 @@ LL | f(x) note: inside `main` --> $DIR/strongly-protected.rs:LL:CC | -LL | / inner(Box::leak(Box::new(0)), |x| { -LL | | let raw = x as *mut _; +LL | / inner(Box::leak(Box::new(0)), |raw| { LL | | drop(unsafe { Box::from_raw(raw) }); LL | | }); | |______^ diff --git a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs index d21f953672d..399d6df73ff 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs +++ b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs @@ -158,7 +158,9 @@ fn wait_wake() { ); } - assert!((200..1000).contains(&start.elapsed().as_millis())); + // When running this in stress-gc mode, things can take quite long. + // So the timeout is 3000 ms. + assert!((200..3000).contains(&start.elapsed().as_millis())); t.join().unwrap(); } diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs new file mode 100644 index 00000000000..2a5d3dff07f --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs @@ -0,0 +1,139 @@ +//@only-target-linux +// test_epoll_block_then_unblock depends on a deterministic schedule. +//@compile-flags: -Zmiri-preemption-rate=0 + +use std::convert::TryInto; +use std::thread; +use std::thread::spawn; + +// This is a set of testcases for blocking epoll. + +fn main() { + test_epoll_block_without_notification(); + test_epoll_block_then_unblock(); + test_notification_after_timeout(); +} + +// Using `as` cast since `EPOLLET` wraps around +const EPOLL_IN_OUT_ET: u32 = (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET) as _; + +#[track_caller] +fn check_epoll_wait( + epfd: i32, + expected_notifications: &[(u32, u64)], + timeout: i32, +) { + let epoll_event = libc::epoll_event { events: 0, u64: 0 }; + let mut array: [libc::epoll_event; N] = [epoll_event; N]; + let maxsize = N; + let array_ptr = array.as_mut_ptr(); + let res = unsafe { libc::epoll_wait(epfd, array_ptr, maxsize.try_into().unwrap(), timeout) }; + if res < 0 { + panic!("epoll_wait failed: {}", std::io::Error::last_os_error()); + } + assert_eq!( + res, + expected_notifications.len().try_into().unwrap(), + "got wrong number of notifications" + ); + let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; + for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) { + let event = return_event.events; + let data = return_event.u64; + assert_eq!(event, expected_event.0, "got wrong events"); + assert_eq!(data, expected_event.1, "got wrong data"); + } +} + +// This test allows epoll_wait to block, then unblock without notification. +fn test_epoll_block_without_notification() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create an eventfd instances. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd = unsafe { libc::eventfd(0, flags) }; + + // Register eventfd with epoll. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) }; + assert_eq!(res, 0); + + // epoll_wait to clear notification. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fd as u64; + check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], 0); + + // This epoll wait blocks, and timeout without notification. + check_epoll_wait::<1>(epfd, &[], 5); +} + +// This test triggers notification and unblocks the epoll_wait before timeout. +fn test_epoll_block_then_unblock() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register one side of the socketpair with epoll. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + + // epoll_wait to clear notification. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[0] as u64; + check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], 0); + + // epoll_wait before triggering notification so it will block then get unblocked before timeout. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = fds[0] as u64; + let thread1 = spawn(move || { + thread::yield_now(); + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + }); + check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], 10); + thread1.join().unwrap(); +} + +// This test triggers a notification after epoll_wait times out. +fn test_notification_after_timeout() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register one side of the socketpair with epoll. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + + // epoll_wait to clear notification. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[0] as u64; + check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], 0); + + // epoll_wait timeouts without notification. + check_epoll_wait::<1>(epfd, &[], 10); + + // Trigger epoll notification after timeout. + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // Check the result of the notification. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = fds[0] as u64; + check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], 10); +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs similarity index 91% rename from src/tools/miri/tests/pass-dep/libc/libc-epoll.rs rename to src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs index 052ce73de23..647b5e60649 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs @@ -21,6 +21,8 @@ fn main() { test_socketpair_epollerr(); test_epoll_lost_events(); test_ready_list_fetching_logic(); + test_epoll_ctl_epfd_equal_fd(); + test_epoll_ctl_notification(); } // Using `as` cast since `EPOLLET` wraps around @@ -630,3 +632,54 @@ fn test_ready_list_fetching_logic() { let expected_value1 = fd1 as u64; check_epoll_wait::<1>(epfd, &[(expected_event1, expected_value1)]); } + +// In epoll_ctl, if the value of epfd equals to fd, EINVAL should be returned. +fn test_epoll_ctl_epfd_equal_fd() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + let array_ptr = std::ptr::without_provenance_mut::(0x100); + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, epfd, array_ptr) }; + let e = std::io::Error::last_os_error(); + assert_eq!(e.raw_os_error(), Some(libc::EINVAL)); + assert_eq!(res, -1); +} + +// We previously used check_and_update_readiness the moment a file description is registered in an +// epoll instance. But this has an unfortunate side effect of returning notification to another +// epfd that shouldn't receive notification. +fn test_epoll_ctl_notification() { + // Create an epoll instance. + let epfd0 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd0, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register one side of the socketpair with epoll. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd0, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + + // epoll_wait to clear notification for epfd0. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[0] as u64; + check_epoll_wait::<1>(epfd0, &[(expected_event, expected_value)]); + + // Create another epoll instance. + let epfd1 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd1, -1); + + // Register the same file description for epfd1. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd1, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + check_epoll_wait::<1>(epfd1, &[(expected_event, expected_value)]); + + // Previously this epoll_wait will receive a notification, but we shouldn't return notification + // for this epfd, because there is no I/O event between the two epoll_wait. + check_epoll_wait::<1>(epfd0, &[]); +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs b/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs index 5dff612bd89..90dbd888392 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs @@ -6,6 +6,7 @@ fn main() { test_pipe(); test_pipe_threaded(); test_race(); + test_pipe_array(); } fn test_pipe() { @@ -97,3 +98,13 @@ fn test_race() { thread::yield_now(); thread1.join().unwrap(); } + +fn test_pipe_array() { + // Declare `pipe` to take an array rather than a `*mut i32`. + extern "C" { + fn pipe(pipefd: &mut [i32; 2]) -> i32; + } + + let mut fds: [i32; 2] = [0; 2]; + assert_eq!(unsafe { pipe(&mut fds) }, 0); +} diff --git a/src/tools/miri/tests/pass-dep/tokio/file-io.rs b/src/tools/miri/tests/pass-dep/tokio/file-io.rs new file mode 100644 index 00000000000..d14af299cd4 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/tokio/file-io.rs @@ -0,0 +1,41 @@ +//@compile-flags: -Zmiri-disable-isolation +//@only-target-linux: We only support tokio on Linux + +use std::fs::remove_file; +use tokio::fs::{File, OpenOptions}; +use tokio::io::{self, AsyncReadExt, AsyncWriteExt}; + +#[path = "../../utils/mod.rs"] +mod utils; + +#[tokio::main] +async fn main() { + test_create_and_write().await.unwrap(); + test_create_and_read().await.unwrap(); +} + +async fn test_create_and_write() -> io::Result<()> { + let path = utils::prepare("foo.txt"); + let mut file = File::create(&path).await?; + + // Write 10 bytes to the file. + file.write(b"some bytes").await?; + assert_eq!(file.metadata().await.unwrap().len(), 10); + + remove_file(&path).unwrap(); + Ok(()) +} + +async fn test_create_and_read() -> io::Result<()> { + let bytes = b"more bytes"; + let path = utils::prepare_with_content("foo.txt", bytes); + let mut file = OpenOptions::new().read(true).open(&path).await.unwrap(); + let mut buffer = [0u8; 10]; + + // Read the whole file. + file.read(&mut buffer[..]).await?; + assert_eq!(&buffer, b"more bytes"); + + remove_file(&path).unwrap(); + Ok(()) +} diff --git a/src/tools/miri/tests/pass-dep/tokio/mpsc-await.rs b/src/tools/miri/tests/pass-dep/tokio/mpsc-await.rs new file mode 100644 index 00000000000..7dea07c6e7d --- /dev/null +++ b/src/tools/miri/tests/pass-dep/tokio/mpsc-await.rs @@ -0,0 +1,20 @@ +//@only-target-linux: We only support tokio on Linux +use tokio::sync::mpsc; + +#[tokio::main] +async fn main() { + let (tx, mut rx) = mpsc::channel(32); + let tx2 = tx.clone(); + + tokio::spawn(async move { + tx.send("sending from handle").await.unwrap(); + }); + + tokio::spawn(async move { + tx2.send("sending from handle").await.unwrap(); + }); + + while let Some(message) = rx.recv().await { + println!("GOT = {}", message); + } +} diff --git a/src/tools/miri/tests/pass-dep/tokio/mpsc-await.stdout b/src/tools/miri/tests/pass-dep/tokio/mpsc-await.stdout new file mode 100644 index 00000000000..52dc6483186 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/tokio/mpsc-await.stdout @@ -0,0 +1,2 @@ +GOT = sending from handle +GOT = sending from handle diff --git a/src/tools/miri/tests/pass-dep/tokio/sleep.rs b/src/tools/miri/tests/pass-dep/tokio/sleep.rs new file mode 100644 index 00000000000..5e63037c8a6 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/tokio/sleep.rs @@ -0,0 +1,11 @@ +//@only-target-linux: We only support tokio on Linux + +use tokio::time::{sleep, Duration, Instant}; + +#[tokio::main] +async fn main() { + let start = Instant::now(); + sleep(Duration::from_millis(100)).await; + let time_elapsed = &start.elapsed().as_millis(); + assert!((100..1000).contains(time_elapsed), "{}", time_elapsed); +} diff --git a/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs b/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs deleted file mode 100644 index 769a7a7d384..00000000000 --- a/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs +++ /dev/null @@ -1,6 +0,0 @@ -// Need to disable preemption to stay on the supported MVP codepath in mio. -//@compile-flags: -Zmiri-permissive-provenance -Zmiri-preemption-rate=0 -//@only-target-x86_64-unknown-linux: support for tokio exists only on linux and x86 - -#[tokio::main] -async fn main() {} diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-sha.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-sha.rs index e65fdc3fbed..79ac4432dff 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-sha.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-sha.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+sha,+sse2,+ssse3,+sse4.1 #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-adx.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-adx.rs index 431e7f2c5eb..0fd4b7c0910 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-adx.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-adx.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+adx #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-aes-vaes.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-aes-vaes.rs index 7363c753617..d4d1b6180a7 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-aes-vaes.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-aes-vaes.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+aes,+vaes,+avx512f #![feature(avx512_target_feature, stdarch_x86_avx512)] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx.rs index 728f57d48f1..3847a80be90 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+avx #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx2.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx2.rs index 80d125bb856..8b8d8880e3b 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx2.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx2.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+avx2 #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs index 66bfcb20f1c..a40eddde97c 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+avx512f,+avx512vl,+avx512bitalg,+avx512vpopcntdq #![feature(avx512_target_feature)] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-bmi.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-bmi.rs index 33424117c45..02f57f4b451 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-bmi.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-bmi.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+bmi1,+bmi2 #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pause-without-sse2.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pause-without-sse2.rs index c8b92fd5458..60da88df046 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pause-without-sse2.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pause-without-sse2.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=-sse2 #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pclmulqdq.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pclmulqdq.rs index 2f242dd5379..86ac5835a16 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pclmulqdq.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-pclmulqdq.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+pclmulqdq #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse3-ssse3.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse3-ssse3.rs index 7566be4431b..0b3be7f3cbd 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse3-ssse3.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse3-ssse3.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm // SSSE3 implicitly enables SSE3 //@compile-flags: -C target-feature=+ssse3 diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse41.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse41.rs index 06607f3fd59..8cd4e6308e2 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse41.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse41.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+sse4.1 #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse42.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse42.rs index 3ac53ea8b93..c87eb518774 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse42.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-sse42.rs @@ -6,7 +6,7 @@ //@ignore-target-avr //@ignore-target-s390x //@ignore-target-thumbv7em -//@ignore-target-wasm32 +//@ignore-target-wasm //@compile-flags: -C target-feature=+sse4.2 #[cfg(target_arch = "x86")] diff --git a/src/tools/miri/tests/pass/tree_borrows/sb_fails.rs b/src/tools/miri/tests/pass/tree_borrows/sb_fails.rs index 1bae30bde60..7da20e76229 100644 --- a/src/tools/miri/tests/pass/tree_borrows/sb_fails.rs +++ b/src/tools/miri/tests/pass/tree_borrows/sb_fails.rs @@ -67,10 +67,11 @@ mod static_memory_modification { #[allow(mutable_transmutes)] pub fn main() { - let _x = unsafe { + let x = unsafe { std::mem::transmute::<&usize, &mut usize>(&X) // In SB this mutable reborrow fails. // But in TB we are allowed to transmute as long as we don't write. }; + assert_eq!(*&*x, 5); } } diff --git a/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs b/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs index adad18c1af3..c741e4de6d5 100644 --- a/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs +++ b/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs @@ -321,7 +321,7 @@ fn not_unpin_not_protected() { pub struct NotUnpin(#[allow(dead_code)] i32, PhantomPinned); fn inner(x: &mut NotUnpin, f: fn(&mut NotUnpin)) { - // `f` may mutate, but it may not deallocate! + // `f` is allowed to deallocate `x`. f(x) }