From a5baa150e3362ae83bda62dbbe73b0f5e2211364 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sun, 5 May 2024 22:38:19 +0200
Subject: [PATCH] getrandom: test with and without isolation

also add some comments for why we keep certain old obscure APIs supported
---
 .../src/shims/unix/linux/foreign_items.rs     | 33 +++++++------------
 .../miri/src/shims/windows/foreign_items.rs   |  3 ++
 src/tools/miri/tests/pass-dep/getrandom.rs    |  3 +-
 3 files changed, 16 insertions(+), 23 deletions(-)

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 ecf82f26a5528..7cd749a41072e 100644
--- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs
+++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs
@@ -117,6 +117,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     // `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)`
                     // is called if a `HashMap` is created the regular way (e.g. HashMap<K, V>).
                     id if id == sys_getrandom => {
+                        // Used by getrandom 0.1
                         // The first argument is the syscall id, so skip over it.
                         if args.len() < 4 {
                             throw_ub_format!(
@@ -124,7 +125,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                                 args.len()
                             );
                         }
-                        getrandom(this, &args[1], &args[2], &args[3], dest)?;
+
+                        let ptr = this.read_pointer(&args[1])?;
+                        let len = this.read_target_usize(&args[2])?;
+                        // The only supported flags are GRND_RANDOM and GRND_NONBLOCK,
+                        // neither of which have any effect on our current PRNG.
+                        // See <https://github.com/rust-lang/rust/pull/79196> for a discussion of argument sizes.
+                        let _flags = this.read_scalar(&args[3])?.to_i32();
+
+                        this.gen_random(ptr, len)?;
+                        this.write_scalar(Scalar::from_target_usize(len, this), dest)?;
                     }
                     // `futex` is used by some synchronization primitives.
                     id if id == sys_futex => {
@@ -196,24 +206,3 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Ok(EmulateItemResult::NeedsJumping)
     }
 }
-
-// Shims the linux `getrandom` syscall.
-fn getrandom<'tcx>(
-    this: &mut MiriInterpCx<'_, 'tcx>,
-    ptr: &OpTy<'tcx, Provenance>,
-    len: &OpTy<'tcx, Provenance>,
-    flags: &OpTy<'tcx, Provenance>,
-    dest: &MPlaceTy<'tcx, Provenance>,
-) -> InterpResult<'tcx> {
-    let ptr = this.read_pointer(ptr)?;
-    let len = this.read_target_usize(len)?;
-
-    // The only supported flags are GRND_RANDOM and GRND_NONBLOCK,
-    // neither of which have any effect on our current PRNG.
-    // See <https://github.com/rust-lang/rust/pull/79196> for a discussion of argument sizes.
-    let _flags = this.read_scalar(flags)?.to_i32();
-
-    this.gen_random(ptr, len)?;
-    this.write_scalar(Scalar::from_target_usize(len, this), dest)?;
-    Ok(())
-}
diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs
index dba5b7a906f91..28dad974771d6 100644
--- a/src/tools/miri/src/shims/windows/foreign_items.rs
+++ b/src/tools/miri/src/shims/windows/foreign_items.rs
@@ -513,6 +513,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 throw_machine_stop!(TerminationInfo::Exit { code: code.into(), leak_check: false });
             }
             "SystemFunction036" => {
+                // used by getrandom 0.1
                 // This is really 'RtlGenRandom'.
                 let [ptr, len] =
                     this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
@@ -522,6 +523,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 this.write_scalar(Scalar::from_bool(true), dest)?;
             }
             "ProcessPrng" => {
+                // used by `std`
                 let [ptr, len] =
                     this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
                 let ptr = this.read_pointer(ptr)?;
@@ -530,6 +532,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 this.write_scalar(Scalar::from_i32(1), dest)?;
             }
             "BCryptGenRandom" => {
+                // used by getrandom 0.2
                 let [algorithm, ptr, len, flags] =
                     this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
                 let algorithm = this.read_scalar(algorithm)?;
diff --git a/src/tools/miri/tests/pass-dep/getrandom.rs b/src/tools/miri/tests/pass-dep/getrandom.rs
index c0d9296a9a6d9..53de3af76350c 100644
--- a/src/tools/miri/tests/pass-dep/getrandom.rs
+++ b/src/tools/miri/tests/pass-dep/getrandom.rs
@@ -1,8 +1,9 @@
 // mac-os `getrandom_01` does some pointer shenanigans
 //@compile-flags: -Zmiri-permissive-provenance
+//@revisions: isolation no_isolation
+//@[no_isolation]compile-flags: -Zmiri-disable-isolation
 
 /// 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();