merge in changes from upstream master

Author: Ellen Arteca

miri

@@ -138,7 +138,7 @@ find_sysroot() {
 # Run command.
 case "$COMMAND" in
 install)
-    # "--locked" to respect the Cargo.lock file if it exists,
+    # "--locked" to respect the Cargo.lock file if it exists.
     $CARGO install $CARGO_EXTRA_FLAGS --path "$MIRIDIR" --force --locked "$@"
     $CARGO install $CARGO_EXTRA_FLAGS --path "$MIRIDIR"/cargo-miri --force --locked "$@"
     ;;

rust-version

@@ -1 +1 @@
-6077b7cda466afa2b75a62b232ab46dbeb148bcb
+db41351753df840773ca628d8daa040e95d00eef

src/helpers.rs

@@ -631,7 +631,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         // Ensure that the access is within bounds.
         assert!(op_place.layout.size >= offset + layout.size);
-        let value_place = op_place.offset(offset, MemPlaceMeta::None, layout, this)?;
+        let value_place = op_place.offset(offset, layout, this)?;
         Ok(value_place)
     }
 

src/shims/backtrace.rs

@@ -104,8 +104,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 for (i, ptr) in ptrs.into_iter().enumerate() {
                     let offset = ptr_layout.size * i.try_into().unwrap();
 
-                    let op_place =
-                        buf_place.offset(offset, MemPlaceMeta::None, ptr_layout, this)?;
+                    let op_place = buf_place.offset(offset, ptr_layout, this)?;
 
                     this.write_pointer(ptr, &op_place.into())?;
                 }

src/shims/unix/foreign_items.rs

@@ -440,12 +440,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             // Miscellaneous
             "isatty" => {
                 let [fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                this.read_scalar(fd)?.to_i32()?;
-                // "returns 1 if fd is an open file descriptor referring to a terminal; otherwise 0 is returned, and errno is set to indicate the error"
-                // FIXME: we just say nothing is a terminal.
-                let enotty = this.eval_libc("ENOTTY")?;
-                this.set_last_error(enotty)?;
-                this.write_null(dest)?;
+                let result = this.isatty(fd)?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
             }
             "pthread_atfork" => {
                 let [prepare, parent, child] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

src/shims/unix/fs.rs

@@ -50,6 +50,9 @@ trait FileDescriptor: std::fmt::Debug {
     ) -> InterpResult<'tcx, io::Result<i32>>;
 
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>>;
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32>;
 }
 
 impl FileDescriptor for FileHandle {
@@ -114,6 +117,12 @@ impl FileDescriptor for FileHandle {
         let duplicated = self.file.try_clone()?;
         Ok(Box::new(FileHandle { file: duplicated, writable: self.writable }))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        use std::os::unix::io::AsRawFd;
+        Some(self.file.as_raw_fd())
+    }
 }
 
 impl FileDescriptor for io::Stdin {
@@ -159,6 +168,11 @@ impl FileDescriptor for io::Stdin {
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(io::stdin()))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        Some(libc::STDIN_FILENO)
+    }
 }
 
 impl FileDescriptor for io::Stdout {
@@ -209,6 +223,11 @@ impl FileDescriptor for io::Stdout {
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(io::stdout()))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        Some(libc::STDOUT_FILENO)
+    }
 }
 
 impl FileDescriptor for io::Stderr {
@@ -252,6 +271,11 @@ impl FileDescriptor for io::Stderr {
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(io::stderr()))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        Some(libc::STDERR_FILENO)
+    }
 }
 
 #[derive(Debug)]
@@ -297,6 +321,11 @@ impl FileDescriptor for DummyOutput {
     fn dup<'tcx>(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(DummyOutput))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        None
+    }
 }
 
 #[derive(Debug)]
@@ -1660,6 +1689,38 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
         }
     }
+
+    #[cfg_attr(not(unix), allow(unused))]
+    fn isatty(&mut self, miri_fd: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
+        let this = self.eval_context_mut();
+        #[cfg(unix)]
+        if matches!(this.machine.isolated_op, IsolatedOp::Allow) {
+            let miri_fd = this.read_scalar(miri_fd)?.to_i32()?;
+            if let Some(host_fd) =
+                this.machine.file_handler.handles.get(&miri_fd).and_then(|fd| fd.as_unix_host_fd())
+            {
+                // "returns 1 if fd is an open file descriptor referring to a terminal;
+                // otherwise 0 is returned, and errno is set to indicate the error"
+                // SAFETY: isatty has no preconditions
+                let is_tty = unsafe { libc::isatty(host_fd) };
+                if is_tty == 0 {
+                    let errno = std::io::Error::last_os_error()
+                        .raw_os_error()
+                        .map(Scalar::from_i32)
+                        .unwrap();
+                    this.set_last_error(errno)?;
+                }
+                return Ok(is_tty);
+            }
+        }
+        // We are attemping to use a Unix interface on a non-Unix platform, or we are on a Unix
+        // platform and the passed file descriptor is not open, or isolation is enabled
+        // FIXME: It should be possible to emulate this at least on Windows by using
+        // GetConsoleMode.
+        let enotty = this.eval_libc("ENOTTY")?;
+        this.set_last_error(enotty)?;
+        Ok(0)
+    }
 }
 
 /// Extracts the number of seconds and nanoseconds elapsed between `time` and the unix epoch when

src/shims/windows/foreign_items.rs

@@ -152,23 +152,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     .collect();
 
                 // Set page size.
-                let page_size = system_info.offset(
-                    field_offsets[2],
-                    MemPlaceMeta::None,
-                    dword_layout,
-                    &this.tcx,
-                )?;
+                let page_size = system_info.offset(field_offsets[2], dword_layout, &this.tcx)?;
                 this.write_scalar(
                     Scalar::from_int(PAGE_SIZE, dword_layout.size),
                     &page_size.into(),
                 )?;
                 // Set number of processors.
-                let num_cpus = system_info.offset(
-                    field_offsets[6],
-                    MemPlaceMeta::None,
-                    dword_layout,
-                    &this.tcx,
-                )?;
+                let num_cpus = system_info.offset(field_offsets[6], dword_layout, &this.tcx)?;
                 this.write_scalar(Scalar::from_int(NUM_CPUS, dword_layout.size), &num_cpus.into())?;
             }
 

src/stacked_borrows/mod.rs

@@ -1021,6 +1021,10 @@ impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mi
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
     fn retag(&mut self, kind: RetagKind, place: &PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
+        let retag_fields = this.machine.stacked_borrows.as_mut().unwrap().get_mut().retag_fields;
+        let mut visitor = RetagVisitor { ecx: this, kind, retag_fields };
+        return visitor.visit_value(place);
+
         // Determine mutability and whether to add a protector.
         // Cannot use `builtin_deref` because that reports *immutable* for `Box`,
         // making it useless.
@@ -1037,90 +1041,67 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 // Raw pointers need to be enabled.
                 ty::RawPtr(tym) if kind == RetagKind::Raw =>
                     Some((RefKind::Raw { mutable: tym.mutbl == Mutability::Mut }, false)),
-                // Boxes are handled separately due to that allocator situation.
+                // Boxes are handled separately due to that allocator situation,
+                // see the visitor below.
                 _ => None,
             }
         }
 
-        // We need a visitor to visit all references. However, that requires
-        // a `MPlaceTy` (or `OpTy`), so we have a fast path for reference types that
-        // avoids allocating.
-
-        if let Some((ref_kind, protector)) = qualify(place.layout.ty, kind) {
-            // Fast path.
-            let val = this.read_immediate(&this.place_to_op(place)?)?;
-            let val = this.retag_reference(&val, ref_kind, protector)?;
-            this.write_immediate(*val, place)?;
-            return Ok(());
-        }
-
-        // If we don't want to recurse, we are already done.
-        // EXCEPT if this is a `Box`, then we have to recurse because allocators.
-        // (Yes this means we technically also recursively retag the allocator itself even if field
-        // retagging is not enabled. *shrug*)
-        if !this.machine.stacked_borrows.as_mut().unwrap().get_mut().retag_fields
-            && !place.layout.ty.ty_adt_def().is_some_and(|adt| adt.is_box())
-        {
-            return Ok(());
-        }
-
-        // Skip some types that have no further structure we might care about.
-        if matches!(
-            place.layout.ty.kind(),
-            ty::RawPtr(..)
-                | ty::Ref(..)
-                | ty::Int(..)
-                | ty::Uint(..)
-                | ty::Float(..)
-                | ty::Bool
-                | ty::Char
-        ) {
-            return Ok(());
-        }
-        // Now go visit this thing.
-        let place = this.force_allocation(place)?;
-
-        let mut visitor = RetagVisitor { ecx: this, kind };
-        return visitor.visit_value(&place);
-
         // The actual visitor.
         struct RetagVisitor<'ecx, 'mir, 'tcx> {
             ecx: &'ecx mut MiriEvalContext<'mir, 'tcx>,
             kind: RetagKind,
+            retag_fields: bool,
+        }
+        impl<'ecx, 'mir, 'tcx> RetagVisitor<'ecx, 'mir, 'tcx> {
+            #[inline(always)] // yes this helps in our benchmarks
+            fn retag_place(
+                &mut self,
+                place: &PlaceTy<'tcx, Tag>,
+                ref_kind: RefKind,
+                protector: bool,
+            ) -> InterpResult<'tcx> {
+                let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?;
+                let val = self.ecx.retag_reference(&val, ref_kind, protector)?;
+                self.ecx.write_immediate(*val, place)?;
+                Ok(())
+            }
         }
         impl<'ecx, 'mir, 'tcx> MutValueVisitor<'mir, 'tcx, Evaluator<'mir, 'tcx>>
             for RetagVisitor<'ecx, 'mir, 'tcx>
         {
-            type V = MPlaceTy<'tcx, Tag>;
+            type V = PlaceTy<'tcx, Tag>;
 
             #[inline(always)]
             fn ecx(&mut self) -> &mut MiriEvalContext<'mir, 'tcx> {
                 self.ecx
             }
 
-            fn visit_box(&mut self, place: &MPlaceTy<'tcx, Tag>) -> InterpResult<'tcx> {
+            fn visit_box(&mut self, place: &PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> {
                 // Boxes do not get a protector: protectors reflect that references outlive the call
                 // they were passed in to; that's just not the case for boxes.
-                let (ref_kind, protector) = (RefKind::Unique { two_phase: false }, false);
-
-                let val = self.ecx.read_immediate(&place.into())?;
-                let val = self.ecx.retag_reference(&val, ref_kind, protector)?;
-                self.ecx.write_immediate(*val, &place.into())?;
-                Ok(())
+                self.retag_place(
+                    place,
+                    RefKind::Unique { two_phase: false },
+                    /*protector*/ false,
+                )
             }
 
-            fn visit_value(&mut self, place: &MPlaceTy<'tcx, Tag>) -> InterpResult<'tcx> {
+            fn visit_value(&mut self, place: &PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> {
                 if let Some((ref_kind, protector)) = qualify(place.layout.ty, self.kind) {
-                    let val = self.ecx.read_immediate(&place.into())?;
-                    let val = self.ecx.retag_reference(&val, ref_kind, protector)?;
-                    self.ecx.write_immediate(*val, &place.into())?;
+                    self.retag_place(place, ref_kind, protector)?;
                 } else if matches!(place.layout.ty.kind(), ty::RawPtr(..)) {
                     // Wide raw pointers *do* have fields and their types are strange.
                     // vtables have a type like `&[*const (); 3]` or so!
                     // Do *not* recurse into them.
-                    // (No need to worry about wide references or boxes, those always "qualify".)
-                } else {
-                    // Maybe we need to go deeper.
+                    // (No need to worry about wide references, those always "qualify". And Boxes
+                    // are handles specially by the visitor anyway.)
+                } else if self.retag_fields
+                    || place.layout.ty.ty_adt_def().is_some_and(|adt| adt.is_box())
+                {
+                    // Recurse deeper. Need to always recurse for `Box` to even hit `visit_box`.
+                    // (Yes this means we technically also recursively retag the allocator itself
+                    // even if field retagging is not enabled. *shrug*)
                     self.walk_value(place)?;
                 }
                 Ok(())

tests/compiletest.rs

@@ -19,9 +19,10 @@ fn run_tests(mode: Mode, path: &str, target: Option<String>) -> Result<()> {
         // Less aggressive warnings to make the rustc toolstate management less painful.
         // (We often get warnings when e.g. a feature gets stabilized or some lint gets added/improved.)
         flags.push("-Astable-features".to_owned());
+        flags.push("-Aunused".to_owned());
     } else {
         flags.push("-Dwarnings".to_owned());
-        flags.push("-Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs
+        flags.push("-Dunused".to_owned());
     }
     if let Ok(sysroot) = env::var("MIRI_SYSROOT") {
         flags.push("--sysroot".to_string());

tests/fail/stacked_borrows/issue-miri-1050-1.rs

@@ -3,6 +3,6 @@
 fn main() {
     unsafe {
         let ptr = Box::into_raw(Box::new(0u16));
-        Box::from_raw(ptr as *mut u32);
+        drop(Box::from_raw(ptr as *mut u32));
     }
 }