Auto merge of #127317 - RalfJung:miri-sync, r=RalfJung

Miri subtree update

r? `@ghost`

src/tools/miri/rust-version

@@ -1 +1 @@
-9ed2ab3790ff41bf741dd690befd6a1c1e2b23ca
+66b4f0021bfb11a8c20d084c99a40f4a78ce1d38

src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs

@@ -19,7 +19,7 @@ pub enum AccessCause {
     Explicit(AccessKind),
     Reborrow,
     Dealloc,
-    FnExit,
+    FnExit(AccessKind),
 }
 
 impl fmt::Display for AccessCause {
@@ -28,7 +28,11 @@ impl fmt::Display for AccessCause {
             Self::Explicit(kind) => write!(f, "{kind}"),
             Self::Reborrow => write!(f, "reborrow"),
             Self::Dealloc => write!(f, "deallocation"),
-            Self::FnExit => write!(f, "protector release"),
+            // This is dead code, since the protector release access itself can never
+            // cause UB (while the protector is active, if some other access invalidates
+            // further use of the protected tag, that is immediate UB).
+            // Describing the cause of UB is the only time this function is called.
+            Self::FnExit(_) => unreachable!("protector accesses can never be the source of UB"),
         }
     }
 }
@@ -40,7 +44,7 @@ impl AccessCause {
             Self::Explicit(kind) => format!("{rel} {kind}"),
             Self::Reborrow => format!("reborrow (acting as a {rel} read access)"),
             Self::Dealloc => format!("deallocation (acting as a {rel} write access)"),
-            Self::FnExit => format!("protector release (acting as a {rel} read access)"),
+            Self::FnExit(kind) => format!("protector release (acting as a {rel} {kind})"),
         }
     }
 }

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

@@ -68,13 +68,11 @@ impl<'tcx> Tree {
         let global = machine.borrow_tracker.as_ref().unwrap();
         let span = machine.current_span();
         self.perform_access(
-            access_kind,
             tag,
-            Some(range),
+            Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
             global,
             alloc_id,
             span,
-            diagnostics::AccessCause::Explicit(access_kind),
         )
     }
 
@@ -115,15 +113,8 @@ impl<'tcx> Tree {
         alloc_id: AllocId, // diagnostics
     ) -> InterpResult<'tcx> {
         let span = machine.current_span();
-        self.perform_access(
-            AccessKind::Read,
-            tag,
-            None, // no specified range because it occurs on the entire allocation
-            global,
-            alloc_id,
-            span,
-            diagnostics::AccessCause::FnExit,
-        )
+        // `None` makes it the magic on-protector-end operation
+        self.perform_access(tag, None, global, alloc_id, span)
     }
 }
 
@@ -297,13 +288,11 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         // All reborrows incur a (possibly zero-sized) read access to the parent
         tree_borrows.perform_access(
-            AccessKind::Read,
             orig_tag,
-            Some(range),
+            Some((range, AccessKind::Read, diagnostics::AccessCause::Reborrow)),
             this.machine.borrow_tracker.as_ref().unwrap(),
             alloc_id,
             this.machine.current_span(),
-            diagnostics::AccessCause::Reborrow,
         )?;
         // Record the parent-child pair in the tree.
         tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?;

src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs

@@ -186,6 +186,10 @@ impl Permission {
     pub fn is_disabled(&self) -> bool {
         self.inner == Disabled
     }
+    /// Check if `self` is the post-child-write state of a pointer (is `Active`).
+    pub fn is_active(&self) -> bool {
+        self.inner == Active
+    }
 
     /// Default initial permission of the root of a new tree at inbounds positions.
     /// Must *only* be used for the root, this is not in general an "initial" permission!

src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs

@@ -530,13 +530,11 @@ impl<'tcx> Tree {
         span: Span,        // diagnostics
     ) -> InterpResult<'tcx> {
         self.perform_access(
-            AccessKind::Write,
             tag,
-            Some(access_range),
+            Some((access_range, AccessKind::Write, diagnostics::AccessCause::Dealloc)),
             global,
             alloc_id,
             span,
-            diagnostics::AccessCause::Dealloc,
         )?;
         for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size) {
             TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
@@ -570,12 +568,16 @@ impl<'tcx> Tree {
     }
 
     /// Map the per-node and per-location `LocationState::perform_access`
-    /// to each location of `access_range`, on every tag of the allocation.
+    /// to each location of the first component of `access_range_and_kind`,
+    /// on every tag of the allocation.
     ///
-    /// If `access_range` is `None`, this is interpreted as the special
+    /// If `access_range_and_kind` is `None`, this is interpreted as the special
     /// access that is applied on protector release:
     /// - the access will be applied only to initialized locations of the allocation,
-    /// - and it will not be visible to children.
+    /// - it will not be visible to children,
+    /// - it will be recorded as a `FnExit` diagnostic access
+    /// - and it will be a read except if the location is `Active`, i.e. has been written to,
+    ///   in which case it will be a write.
     ///
     /// `LocationState::perform_access` will take care of raising transition
     /// errors and updating the `initialized` status of each location,
@@ -585,13 +587,11 @@ impl<'tcx> Tree {
     /// - recording the history.
     pub fn perform_access(
         &mut self,
-        access_kind: AccessKind,
         tag: BorTag,
-        access_range: Option<AllocRange>,
+        access_range_and_kind: Option<(AllocRange, AccessKind, diagnostics::AccessCause)>,
         global: &GlobalState,
-        alloc_id: AllocId,                      // diagnostics
-        span: Span,                             // diagnostics
-        access_cause: diagnostics::AccessCause, // diagnostics
+        alloc_id: AllocId, // diagnostics
+        span: Span,        // diagnostics
     ) -> InterpResult<'tcx> {
         use std::ops::Range;
         // Performs the per-node work:
@@ -605,6 +605,8 @@ impl<'tcx> Tree {
         // `perms_range` is only for diagnostics (it is the range of
         // the `RangeMap` on which we are currently working).
         let node_app = |perms_range: Range<u64>,
+                        access_kind: AccessKind,
+                        access_cause: diagnostics::AccessCause,
                         args: NodeAppArgs<'_>|
          -> Result<ContinueTraversal, TransitionError> {
             let NodeAppArgs { node, mut perm, rel_pos } = args;
@@ -618,14 +620,13 @@ impl<'tcx> Tree {
 
             let protected = global.borrow().protected_tags.contains_key(&node.tag);
             let transition = old_state.perform_access(access_kind, rel_pos, protected)?;
-
             // Record the event as part of the history
             if !transition.is_noop() {
                 node.debug_info.history.push(diagnostics::Event {
                     transition,
                     is_foreign: rel_pos.is_foreign(),
                     access_cause,
-                    access_range,
+                    access_range: access_range_and_kind.map(|x| x.0),
                     transition_range: perms_range,
                     span,
                 });
@@ -636,6 +637,7 @@ impl<'tcx> Tree {
         // Error handler in case `node_app` goes wrong.
         // Wraps the faulty transition in more context for diagnostics.
         let err_handler = |perms_range: Range<u64>,
+                           access_cause: diagnostics::AccessCause,
                            args: ErrHandlerArgs<'_, TransitionError>|
          -> InterpError<'tcx> {
             let ErrHandlerArgs { error_kind, conflicting_info, accessed_info } = args;
@@ -650,7 +652,7 @@ impl<'tcx> Tree {
             .build()
         };
 
-        if let Some(access_range) = access_range {
+        if let Some((access_range, access_kind, access_cause)) = access_range_and_kind {
             // Default branch: this is a "normal" access through a known range.
             // We iterate over affected locations and traverse the tree for each of them.
             for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size)
@@ -658,8 +660,8 @@ impl<'tcx> Tree {
                 TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
                     .traverse_parents_this_children_others(
                         tag,
-                        |args| node_app(perms_range.clone(), args),
-                        |args| err_handler(perms_range.clone(), args),
+                        |args| node_app(perms_range.clone(), access_kind, access_cause, args),
+                        |args| err_handler(perms_range.clone(), access_cause, args),
                     )?;
             }
         } else {
@@ -678,11 +680,14 @@ impl<'tcx> Tree {
                 if let Some(p) = perms.get(idx)
                     && p.initialized
                 {
+                    let access_kind =
+                        if p.permission.is_active() { AccessKind::Write } else { AccessKind::Read };
+                    let access_cause = diagnostics::AccessCause::FnExit(access_kind);
                     TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
                         .traverse_nonchildren(
                         tag,
-                        |args| node_app(perms_range.clone(), args),
-                        |args| err_handler(perms_range.clone(), args),
+                        |args| node_app(perms_range.clone(), access_kind, access_cause, args),
+                        |args| err_handler(perms_range.clone(), access_cause, args),
                     )?;
                 }
             }

src/tools/miri/src/helpers.rs

@@ -24,7 +24,7 @@ use rustc_middle::ty::{
     FloatTy, IntTy, Ty, TyCtxt, UintTy,
 };
 use rustc_session::config::CrateType;
-use rustc_span::{sym, Span, Symbol};
+use rustc_span::{Span, Symbol};
 use rustc_target::abi::{Align, FieldIdx, FieldsShape, Size, Variants};
 use rustc_target::spec::abi::Abi;
 
@@ -1182,14 +1182,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         this.alloc_mark_immutable(provenance.get_alloc_id().unwrap()).unwrap();
     }
 
-    fn item_link_name(&self, def_id: DefId) -> Symbol {
-        let tcx = self.eval_context_ref().tcx;
-        match tcx.get_attrs(def_id, sym::link_name).filter_map(|a| a.value_str()).next() {
-            Some(name) => name,
-            None => tcx.item_name(def_id),
-        }
-    }
-
     /// Converts `src` from floating point to integer type `dest_ty`
     /// after rounding with mode `round`.
     /// Returns `None` if `f` is NaN or out of range.

src/tools/miri/src/machine.rs

@@ -954,7 +954,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
             // foreign function
             // Any needed call to `goto_block` will be performed by `emulate_foreign_item`.
             let args = ecx.copy_fn_args(args); // FIXME: Should `InPlace` arguments be reset to uninit?
-            let link_name = ecx.item_link_name(instance.def_id());
+            let link_name = Symbol::intern(ecx.tcx.symbol_name(instance).name);
             return ecx.emulate_foreign_item(link_name, abi, &args, dest, ret, unwind);
         }
 
@@ -1050,7 +1050,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
         ecx: &MiriInterpCx<'tcx>,
         def_id: DefId,
     ) -> InterpResult<'tcx, StrictPointer> {
-        let link_name = ecx.item_link_name(def_id);
+        let link_name = Symbol::intern(ecx.tcx.symbol_name(Instance::mono(*ecx.tcx, def_id)).name);
         if let Some(&ptr) = ecx.machine.extern_statics.get(&link_name) {
             // Various parts of the engine rely on `get_alloc_info` for size and alignment
             // information. That uses the type information of this static.

src/tools/miri/src/shims/foreign_items.rs

@@ -46,24 +46,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         unwind: mir::UnwindAction,
     ) -> InterpResult<'tcx, Option<(&'tcx mir::Body<'tcx>, ty::Instance<'tcx>)>> {
         let this = self.eval_context_mut();
-        let tcx = this.tcx.tcx;
 
         // Some shims forward to other MIR bodies.
         match link_name.as_str() {
-            // This matches calls to the foreign item `panic_impl`.
-            // The implementation is provided by the function with the `#[panic_handler]` attribute.
-            "panic_impl" => {
-                // We don't use `check_shim` here because we are just forwarding to the lang
-                // item. Argument count checking will be performed when the returned `Body` is
-                // called.
-                this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?;
-                let panic_impl_id = tcx.lang_items().panic_impl().unwrap();
-                let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id);
-                return Ok(Some((
-                    this.load_mir(panic_impl_instance.def, None)?,
-                    panic_impl_instance,
-                )));
-            }
             "__rust_alloc_error_handler" => {
                 // Forward to the right symbol that implements this function.
                 let Some(handler_kind) = this.tcx.alloc_error_handler_kind(()) else {

src/tools/miri/src/shims/unix/fd.rs

@@ -273,6 +273,32 @@ impl FdTable {
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
+    fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, i32> {
+        let this = self.eval_context_mut();
+
+        let Some(dup_fd) = this.machine.fds.dup(old_fd) else {
+            return this.fd_not_found();
+        };
+        Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, 0))
+    }
+
+    fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, i32> {
+        let this = self.eval_context_mut();
+
+        let Some(dup_fd) = this.machine.fds.dup(old_fd) else {
+            return this.fd_not_found();
+        };
+        if new_fd != old_fd {
+            // Close new_fd if it is previously opened.
+            // If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive.
+            if let Some(file_descriptor) = this.machine.fds.fds.insert(new_fd, dup_fd) {
+                // Ignore close error (not interpreter's) according to dup2() doc.
+                file_descriptor.close(this.machine.communicate())?.ok();
+            }
+        }
+        Ok(new_fd)
+    }
+
     fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
@@ -334,14 +360,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         let fd = this.read_scalar(fd_op)?.to_i32()?;
 
-        Ok(Scalar::from_i32(if let Some(file_descriptor) = this.machine.fds.remove(fd) {
-            let result = file_descriptor.close(this.machine.communicate())?;
-            // return `0` if close is successful
-            let result = result.map(|()| 0i32);
-            this.try_unwrap_io_result(result)?
-        } else {
-            this.fd_not_found()?
-        }))
+        let Some(file_descriptor) = this.machine.fds.remove(fd) else {
+            return Ok(Scalar::from_i32(this.fd_not_found()?));
+        };
+        let result = file_descriptor.close(this.machine.communicate())?;
+        // return `0` if close is successful
+        let result = result.map(|()| 0i32);
+        Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?))
     }
 
     /// Function used when a file descriptor does not exist. It returns `Ok(-1)`and sets

src/tools/miri/src/shims/unix/foreign_items.rs

@@ -115,6 +115,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let result = this.fcntl(args)?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
+            "dup" => {
+                let [old_fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let old_fd = this.read_scalar(old_fd)?.to_i32()?;
+                let new_fd = this.dup(old_fd)?;
+                this.write_scalar(Scalar::from_i32(new_fd), dest)?;
+            }
+            "dup2" => {
+                let [old_fd, new_fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let old_fd = this.read_scalar(old_fd)?.to_i32()?;
+                let new_fd = this.read_scalar(new_fd)?.to_i32()?;
+                let result = this.dup2(old_fd, new_fd)?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
 
             // File and file system access
             "open" | "open64" => {

src/tools/miri/src/shims/x86/avx.rs

@@ -338,6 +338,17 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
                 this.write_scalar(Scalar::from_i32(res.into()), dest)?;
             }
+            // Used to implement the `_mm256_zeroupper` and `_mm256_zeroall` functions.
+            // These function clear out the upper 128 bits of all avx registers or
+            // zero out all avx registers respectively.
+            "vzeroupper" | "vzeroall" => {
+                // These functions are purely a performance hint for the CPU.
+                // Any registers currently in use will be saved beforehand by the
+                // compiler, making these functions no-ops.
+
+                // The only thing that needs to be ensured is the correct calling convention.
+                let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+            }
             _ => return Ok(EmulateItemResult::NotSupported),
         }
         Ok(EmulateItemResult::NeedsReturn)

src/tools/miri/tests/fail/tree_borrows/protector-write-lazy.rs

@@ -0,0 +1,35 @@
+//@compile-flags: -Zmiri-tree-borrows
+// This test tests that TB's protector end semantics correctly ensure
+// that protected activated writes can be reordered.
+fn the_other_function(ref_to_fst_elem: &mut i32, ptr_to_vec: *mut i32) -> *mut i32 {
+    // Activate the reference. Afterwards, we should be able to reorder arbitrary writes.
+    *ref_to_fst_elem = 0;
+    // Here is such an arbitrary write.
+    // It could be moved down after the retag, in which case the `funky_ref` would be invalidated.
+    // We need to ensure that the `funky_ptr` is unusable even if the write to `ref_to_fst_elem`
+    // happens before the retag.
+    *ref_to_fst_elem = 42;
+    // this creates a reference that is Reserved Lazy on the first element (offset 0).
+    // It does so by doing a proper retag on the second element (offset 1), which is fine
+    // since nothing else happens at that offset, but the lazy init mechanism means it's
+    // also reserved at offset 0, but not initialized.
+    let funky_ptr_lazy_on_fst_elem =
+        unsafe { (&mut *(ptr_to_vec.wrapping_add(1))) as *mut i32 }.wrapping_sub(1);
+    // If we write to `ref_to_fst_elem` here, then any further access to `funky_ptr_lazy_on_fst_elem` would
+    // definitely be UB. Since the compiler ought to be able to reorder the write of `42` above down to
+    // here, that means we want this program to also be UB.
+    return funky_ptr_lazy_on_fst_elem;
+}
+
+fn main() {
+    let mut v = vec![0, 1];
+    // get a pointer to the root of the allocation
+    // note that it's not important it's the actual root, what matters is that it's a parent
+    // of both references that will be created
+    let ptr_to_vec = v.as_mut_ptr();
+    let ref_to_fst_elem = unsafe { &mut *ptr_to_vec };
+    let funky_ptr_lazy_on_fst_elem = the_other_function(ref_to_fst_elem, ptr_to_vec);
+    // now we try to use the funky lazy pointer.
+    // It should be UB, since the write-on-protector-end should disable it.
+    unsafe { println!("Value of funky: {}", *funky_ptr_lazy_on_fst_elem) } //~ ERROR: /reborrow through .* is forbidden/
+}

src/tools/miri/tests/fail/tree_borrows/protector-write-lazy.stderr

@@ -0,0 +1,27 @@
+error: Undefined Behavior: reborrow through <TAG> at ALLOC[0x0] is forbidden
+  --> $DIR/protector-write-lazy.rs:LL:CC
+   |
+LL |     unsafe { println!("Value of funky: {}", *funky_ptr_lazy_on_fst_elem) }
+   |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ reborrow through <TAG> 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 <TAG> has state Disabled which forbids this reborrow (acting as a child read access)
+help: the accessed tag <TAG> was created here, in the initial state Reserved
+  --> $DIR/protector-write-lazy.rs:LL:CC
+   |
+LL |         unsafe { (&mut *(ptr_to_vec.wrapping_add(1))) as *mut i32 }.wrapping_sub(1);
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: the accessed tag <TAG> later transitioned to Disabled due to a protector release (acting as a foreign write access) on every location previously accessed by this tag
+  --> $DIR/protector-write-lazy.rs:LL:CC
+   |
+LL | }
+   |  ^
+   = help: this transition corresponds to a loss of read and write permissions
+   = note: BACKTRACE (of the first span):
+   = note: inside `main` at $DIR/protector-write-lazy.rs:LL:CC
+   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/pass-dep/libc/libc-fs.rs

@@ -15,6 +15,7 @@ use std::path::PathBuf;
 mod utils;
 
 fn main() {
+    test_dup();
     test_dup_stdout_stderr();
     test_canonicalize_too_long();
     test_rename();
@@ -74,6 +75,31 @@ fn test_dup_stdout_stderr() {
     }
 }
 
+fn test_dup() {
+    let bytes = b"dup and dup2";
+    let path = utils::prepare_with_content("miri_test_libc_dup.txt", bytes);
+
+    let mut name = path.into_os_string();
+    name.push("\0");
+    let name_ptr = name.as_bytes().as_ptr().cast::<libc::c_char>();
+    unsafe {
+        let fd = libc::open(name_ptr, libc::O_RDONLY);
+        let mut first_buf = [0u8; 4];
+        libc::read(fd, first_buf.as_mut_ptr() as *mut libc::c_void, 4);
+        assert_eq!(&first_buf, b"dup ");
+
+        let new_fd = libc::dup(fd);
+        let mut second_buf = [0u8; 4];
+        libc::read(new_fd, second_buf.as_mut_ptr() as *mut libc::c_void, 4);
+        assert_eq!(&second_buf, b"and ");
+
+        let new_fd2 = libc::dup2(fd, 8);
+        let mut third_buf = [0u8; 4];
+        libc::read(new_fd2, third_buf.as_mut_ptr() as *mut libc::c_void, 4);
+        assert_eq!(&third_buf, b"dup2");
+    }
+}
+
 fn test_canonicalize_too_long() {
     // Make sure we get an error for long paths.
     let too_long = "x/".repeat(libc::PATH_MAX.try_into().unwrap());

src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx.rs

@@ -1342,6 +1342,11 @@ unsafe fn test_avx() {
         assert_eq!(r, 1);
     }
     test_mm_testnzc_ps();
+
+    // These intrinsics are functionally no-ops. The only thing
+    // that needs to be tested is that they can be executed.
+    _mm256_zeroupper();
+    _mm256_zeroall();
 }
 
 #[target_feature(enable = "sse2")]