Rollup merge of #118022 - saethlin:miri, r=saethlin

Miri subtree update

Author: matthiaskrgr

src/tools/miri/ci.sh

@@ -108,7 +108,7 @@ case $HOST_TARGET in
     MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests
     MIRI_TEST_TARGET=aarch64-apple-darwin run_tests
     MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests
-    MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthreads atomic env/var
+    MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc atomic env align
     MIRI_TEST_TARGET=aarch64-linux-android run_tests_minimal hello integer vec panic/panic
     MIRI_TEST_TARGET=wasm32-wasi run_tests_minimal no_std integer strings wasm
     MIRI_TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std integer strings wasm

src/tools/miri/rust-version

@@ -1 +1 @@
-3aaa0f57b7b877ef58532a8de075d1e5a79142bf
+820f06b21f8373060ff7b515715b8440a6a6c197

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

@@ -59,7 +59,7 @@ impl fmt::Debug for BorTag {
 #[derive(Debug)]
 pub struct FrameState {
     /// The ID of the call this frame corresponds to.
-    pub call_id: CallId,
+    call_id: CallId,
 
     /// If this frame is protecting any tags, they are listed here. We use this list to do
     /// incremental updates of the global list of protected tags stored in the
@@ -72,7 +72,7 @@ pub struct FrameState {
     ///
     /// This will contain one tag per reference passed to the function, so
     /// a size of 2 is enough for the vast majority of functions.
-    pub protected_tags: SmallVec<[(AllocId, BorTag); 2]>,
+    protected_tags: SmallVec<[(AllocId, BorTag); 2]>,
 }
 
 impl VisitTags for FrameState {
@@ -85,29 +85,29 @@ impl VisitTags for FrameState {
 #[derive(Debug)]
 pub struct GlobalStateInner {
     /// Borrow tracker method currently in use.
-    pub borrow_tracker_method: BorrowTrackerMethod,
+    borrow_tracker_method: BorrowTrackerMethod,
     /// Next unused pointer ID (tag).
-    pub next_ptr_tag: BorTag,
+    next_ptr_tag: BorTag,
     /// Table storing the "base" tag for each allocation.
     /// The base tag is the one used for the initial pointer.
     /// We need this in a separate table to handle cyclic statics.
-    pub base_ptr_tags: FxHashMap<AllocId, BorTag>,
+    base_ptr_tags: FxHashMap<AllocId, BorTag>,
     /// Next unused call ID (for protectors).
-    pub next_call_id: CallId,
+    next_call_id: CallId,
     /// All currently protected tags.
     /// An item is protected if its tag is in this set, *and* it has the "protected" bit set.
     /// We add tags to this when they are created with a protector in `reborrow`, and
     /// we remove tags from this when the call which is protecting them returns, in
     /// `GlobalStateInner::end_call`. See `Stack::item_popped` for more details.
-    pub protected_tags: FxHashMap<BorTag, ProtectorKind>,
+    protected_tags: FxHashMap<BorTag, ProtectorKind>,
     /// The pointer ids to trace
-    pub tracked_pointer_tags: FxHashSet<BorTag>,
+    tracked_pointer_tags: FxHashSet<BorTag>,
     /// The call ids to trace
-    pub tracked_call_ids: FxHashSet<CallId>,
+    tracked_call_ids: FxHashSet<CallId>,
     /// Whether to recurse into datatypes when searching for pointers to retag.
-    pub retag_fields: RetagFields,
+    retag_fields: RetagFields,
     /// Whether `core::ptr::Unique` gets special (`Box`-like) handling.
-    pub unique_is_unique: bool,
+    unique_is_unique: bool,
 }
 
 impl VisitTags for GlobalStateInner {
@@ -194,7 +194,7 @@ impl GlobalStateInner {
     }
 
     /// Generates a new pointer tag. Remember to also check track_pointer_tags and log its creation!
-    pub fn new_ptr(&mut self) -> BorTag {
+    fn new_ptr(&mut self) -> BorTag {
         let id = self.next_ptr_tag;
         self.next_ptr_tag = id.succ().unwrap();
         id
@@ -210,7 +210,7 @@ impl GlobalStateInner {
         FrameState { call_id, protected_tags: SmallVec::new() }
     }
 
-    pub fn end_call(&mut self, frame: &machine::FrameExtra<'_>) {
+    fn end_call(&mut self, frame: &machine::FrameExtra<'_>) {
         for (_, tag) in &frame
             .borrow_tracker
             .as_ref()
@@ -355,6 +355,38 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             BorrowTrackerMethod::TreeBorrows => this.print_tree(alloc_id, show_unnamed),
         }
     }
+
+    fn on_stack_pop(
+        &self,
+        frame: &Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>,
+    ) -> InterpResult<'tcx> {
+        let this = self.eval_context_ref();
+        let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
+        // The body of this loop needs `borrow_tracker` immutably
+        // so we can't move this code inside the following `end_call`.
+        for (alloc_id, tag) in &frame
+            .extra
+            .borrow_tracker
+            .as_ref()
+            .expect("we should have borrow tracking data")
+            .protected_tags
+        {
+            // Just because the tag is protected doesn't guarantee that
+            // the allocation still exists (weak protectors allow deallocations)
+            // so we must check that the allocation exists.
+            // If it does exist, then we have the guarantee that the
+            // pointer is readable, and the implicit read access inserted
+            // will never cause UB on the pointer itself.
+            let (_, _, kind) = this.get_alloc_info(*alloc_id);
+            if matches!(kind, AllocKind::LiveData) {
+                let alloc_extra = this.get_alloc_extra(*alloc_id).unwrap();
+                let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap();
+                alloc_borrow_tracker.release_protector(&this.machine, borrow_tracker, *tag)?;
+            }
+        }
+        borrow_tracker.borrow_mut().end_call(&frame.extra);
+        Ok(())
+    }
 }
 
 /// Extra per-allocation data for borrow tracking

src/tools/miri/src/machine.rs

@@ -1409,34 +1409,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     ) -> InterpResult<'tcx> {
         // We want this *before* the return value copy, because the return place itself is protected
         // until we do `end_call` here.
-        if let Some(global_borrow_tracker) = &ecx.machine.borrow_tracker {
-            // The body of this loop needs `global_borrow_tracker` immutably
-            // so we can't move this code inside the following `end_call`.
-            for (alloc_id, tag) in &frame
-                .extra
-                .borrow_tracker
-                .as_ref()
-                .expect("we should have borrow tracking data")
-                .protected_tags
-            {
-                // Just because the tag is protected doesn't guarantee that
-                // the allocation still exists (weak protectors allow deallocations)
-                // so we must check that the allocation exists.
-                // If it does exist, then we have the guarantee that the
-                // pointer is readable, and the implicit read access inserted
-                // will never cause UB on the pointer itself.
-                let (_, _, kind) = ecx.get_alloc_info(*alloc_id);
-                if matches!(kind, AllocKind::LiveData) {
-                    let alloc_extra = ecx.get_alloc_extra(*alloc_id).unwrap();
-                    let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap();
-                    alloc_borrow_tracker.release_protector(
-                        &ecx.machine,
-                        global_borrow_tracker,
-                        *tag,
-                    )?;
-                }
-            }
-            global_borrow_tracker.borrow_mut().end_call(&frame.extra);
+        if ecx.machine.borrow_tracker.is_some() {
+            ecx.on_stack_pop(frame)?;
         }
         Ok(())
     }

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

@@ -27,6 +27,8 @@ fn is_dyn_sym(name: &str, target_os: &str) -> bool {
         // `signal` is set up as a weak symbol in `init_extern_statics` (on Android) so we might as
         // well allow it in `dlsym`.
         "signal" => true,
+        // needed at least on macOS to avoid file-based fallback in getrandom
+        "getentropy" => true,
         // Give specific OSes a chance to allow their symbols.
         _ =>
             match target_os {
@@ -250,6 +252,37 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 this.write_scalar(result, dest)?;
             }
 
+            "reallocarray" => {
+                // Currently this function does not exist on all Unixes, e.g. on macOS.
+                if !matches!(&*this.tcx.sess.target.os, "linux" | "freebsd") {
+                    throw_unsup_format!(
+                        "`reallocarray` is not supported on {}",
+                        this.tcx.sess.target.os
+                    );
+                }
+                let [ptr, nmemb, size] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let ptr = this.read_pointer(ptr)?;
+                let nmemb = this.read_target_usize(nmemb)?;
+                let size = this.read_target_usize(size)?;
+                // reallocarray checks a possible overflow and returns ENOMEM
+                // if that happens.
+                //
+                // Linux: https://www.unix.com/man-page/linux/3/reallocarray/
+                // FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=reallocarray
+                match nmemb.checked_mul(size) {
+                    None => {
+                        let einval = this.eval_libc("ENOMEM");
+                        this.set_last_error(einval)?;
+                        this.write_null(dest)?;
+                    }
+                    Some(len) => {
+                        let res = this.realloc(ptr, len, MiriMemoryKind::C)?;
+                        this.write_pointer(res, dest)?;
+                    }
+                }
+            }
+
             // Dynamic symbol loading
             "dlsym" => {
                 let [handle, symbol] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
@@ -525,6 +558,34 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let result = this.getpid()?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
+            "getentropy" => {
+                // This function is non-standard but exists with the same signature and behavior on
+                // Linux, macOS, and FreeBSD.
+                if !matches!(&*this.tcx.sess.target.os, "linux" | "macos" | "freebsd") {
+                    throw_unsup_format!(
+                        "`getentropy` is not supported on {}",
+                        this.tcx.sess.target.os
+                    );
+                }
+
+                let [buf, bufsize] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let buf = this.read_pointer(buf)?;
+                let bufsize = this.read_target_usize(bufsize)?;
+
+                // getentropy sets errno to EIO when the buffer size exceeds 256 bytes.
+                // FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=getentropy&sektion=3&format=html
+                // Linux: https://man7.org/linux/man-pages/man3/getentropy.3.html
+                // macOS: https://keith.github.io/xcode-man-pages/getentropy.2.html
+                if bufsize > 256 {
+                    let err = this.eval_libc("EIO");
+                    this.set_last_error(err)?;
+                    this.write_scalar(Scalar::from_i32(-1), dest)?
+                } else {
+                    this.gen_random(buf, bufsize)?;
+                    this.write_scalar(Scalar::from_i32(0), dest)?;
+                }
+            }
 
             // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
             // These shims are enabled only when the caller is in the standard library.
@@ -594,7 +655,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 this.write_int(super::UID, dest)?;
             }
 
-            "getpwuid_r" if this.frame_in_std() => {
+            "getpwuid_r"
+            if this.frame_in_std() => {
                 let [uid, pwd, buf, buflen, result] =
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 this.check_no_isolation("`getpwuid_r`")?;

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

@@ -47,6 +47,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     this.read_scalar(len)?,
                 )?;
             }
+            "getrandom" => {
+                let [ptr, len, flags] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let ptr = this.read_pointer(ptr)?;
+                let len = this.read_target_usize(len)?;
+                let _flags = this.read_scalar(flags)?.to_i32()?;
+                // flags on freebsd does not really matter
+                // in practice, GRND_RANDOM does not particularly draw from /dev/random
+                // since it is the same as to /dev/urandom.
+                // GRND_INSECURE is only an alias of GRND_NONBLOCK, which
+                // does not affect the RNG.
+                // https://man.freebsd.org/cgi/man.cgi?query=getrandom&sektion=2&n=1
+                this.gen_random(ptr, len)?;
+                this.write_scalar(Scalar::from_target_usize(len, this), dest)?;
+            }
 
             // errno
             "__error" => {

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

@@ -6,8 +6,8 @@ use shims::foreign_items::EmulateForeignItemResult;
 use shims::unix::fs::EvalContextExt as _;
 use shims::unix::thread::EvalContextExt as _;
 
-pub fn is_dyn_sym(name: &str) -> bool {
-    matches!(name, "getentropy")
+pub fn is_dyn_sym(_name: &str) -> bool {
+    false
 }
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
@@ -113,18 +113,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 this.write_scalar(result, dest)?;
             }
 
-            // Random generation related shims
-            "getentropy" => {
-                let [buf, bufsize] =
-                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                let buf = this.read_pointer(buf)?;
-                let bufsize = this.read_target_usize(bufsize)?;
-
-                this.gen_random(buf, bufsize)?;
-
-                this.write_scalar(Scalar::from_i32(0), dest)?; // KERN_SUCCESS
-            }
-
             // Access to command-line arguments
             "_NSGetArgc" => {
                 let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

src/tools/miri/test_dependencies/Cargo.toml

@@ -12,8 +12,8 @@ edition = "2021"
 libc = "0.2"
 num_cpus = "1.10.1"
 
-getrandom_1 = { package = "getrandom", version = "0.1" }
-getrandom = { version = "0.2", features = ["js"] }
+getrandom_01 = { package = "getrandom", version = "0.1" }
+getrandom_02 = { package = "getrandom", version = "0.2", features = ["js"] }
 rand = { version = "0.8", features = ["small_rng"] }
 
 [target.'cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))'.dependencies]

src/tools/miri/tests/compiletest.rs

@@ -79,6 +79,11 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) ->
         program.args.push(flag);
     }
 
+    // Add a test env var to do environment communication tests.
+    program.envs.push(("MIRI_ENV_VAR_TEST".into(), Some("0".into())));
+    // Let the tests know where to store temp files (they might run for a different target, which can make this hard to find).
+    program.envs.push(("MIRI_TEMP".into(), Some(env::temp_dir().into())));
+
     let mut config = Config {
         target: Some(target.to_owned()),
         stderr_filters: STDERR.clone(),
@@ -232,11 +237,6 @@ fn main() -> Result<()> {
         }
     }
 
-    // Add a test env var to do environment communication tests.
-    env::set_var("MIRI_ENV_VAR_TEST", "0");
-    // Let the tests know where to store temp files (they might run for a different target, which can make this hard to find).
-    env::set_var("MIRI_TEMP", env::temp_dir());
-
     ui(Mode::Pass, "tests/pass", &target, WithoutDependencies)?;
     ui(Mode::Pass, "tests/pass-dep", &target, WithDependencies)?;
     ui(Mode::Panic, "tests/panic", &target, WithDependencies)?;

src/tools/miri/tests/pass-dep/getrandom.rs

@@ -0,0 +1,10 @@
+// mac-os `getrandom_01` does some pointer shenanigans
+//@compile-flags: -Zmiri-permissive-provenance
+
+/// Test direct calls of getrandom 0.1 and 0.2.
+/// Make sure they work even with isolation enabled (i.e., we do not hit a file-based fallback path).
+fn main() {
+    let mut data = vec![0; 16];
+    getrandom_01::getrandom(&mut data).unwrap();
+    getrandom_02::getrandom(&mut data).unwrap();
+}

src/tools/miri/tests/pass-dep/getrandom_1.rs

@@ -1,10 +1,13 @@
 //@compile-flags: -Zmiri-strict-provenance
-use rand::{rngs::SmallRng, Rng, SeedableRng};
+use rand::prelude::*;
 
+// Test using the `rand` crate to generate randomness.
 fn main() {
-    // Test `getrandom` directly.
-    let mut data = vec![0; 16];
-    getrandom::getrandom(&mut data).unwrap();
+    // Fully deterministic seeding.
+    let mut rng = SmallRng::seed_from_u64(42);
+    let _val = rng.gen::<i32>();
+    let _val = rng.gen::<isize>();
+    let _val = rng.gen::<i128>();
 
     // Try seeding with "real" entropy.
     let mut rng = SmallRng::from_entropy();