Auto merge of #2151 - RalfJung:numbers, r=oli-obk

enable number validity checking and ptr::invalid checking by default

This removes the `-Zmiri-check-number-validity` flag, enabling its effects by default. (We don't error when the flag is passed, for backwards compatibility.) We also enable by default that transmuting an integer to a pointer now creates a pointer with `None` provenance, which is invalid to dereference (and, in the case of a function pointer, invalid to call). I did this together since it is all related to ptr2int/int2ptr transmutation.

Two new flags are added to optionally take back these stricter checks:
- `-Zmiri-allow-uninit-numbers` makes Miri accept uninit data in integers and floats
- `-Zmiri-allow-ptr-int-transmute` makes Miri accept pointers (provenance data) in integers and floats, *and* makes Miri treat int2ptr transmutes as equivalent to a cast.

The flag names make sense IMO, but they are somewhat inconsistent with our existing flags since we usually call things `-Zmiri-disable-$CHECK` rather than `-Zmiri-allow-$THING`. But `-Zmiri-disable-uninit-number-check` sounds silly?

(Whenever I say "transmute" this includes union and pointer based type punning.)
Cc `@saethlin` I hope this won't break everything?^^ I think the most risky part is the int2ptr transmute aspect, in particular around function pointers where no `as` casts are possible. The correct pattern is to first cast to a raw ptr and then transmute that to a fn ptr. We should probably document this better, in the `transmute` documentation and maybe in the documentation for the `fn()` type. I should run this PR against the std test suite before we land it.
r? `@oli-obk`

- [x] Ensure stdlib docs recommend "usize -> raw ptr -> fn ptr" for int-to-fnptr casts: rust-lang/rust#97321
- [x] Run the stdlib test suite

README.md

@@ -44,8 +44,7 @@ in your program, and cannot run all programs:
   positives here, so if your program runs fine in Miri right now that is by no
   means a guarantee that it is UB-free when these questions get answered.
 
-    In particular, Miri does currently not check that integers/floats are
-  initialized or that references point to valid data.
+    In particular, Miri does currently not check that references point to valid data.
 * If the program relies on unspecified details of how data is laid out, it will
   still run fine in Miri -- but might break (including causing UB) on different
   compiler versions or different platforms.
@@ -302,10 +301,15 @@ The remaining flags are for advanced use only, and more likely to change or be r
 Some of these are **unsound**, which means they can lead
 to Miri failing to detect cases of undefined behavior in a program.
 
-* `-Zmiri-check-number-validity` enables checking of integer and float validity
-  (e.g., they must be initialized and not carry pointer provenance) as part of
-  enforcing validity invariants. This has no effect when
+* `-Zmiri-allow-uninit-numbers` disables the check to ensure that number types (integer and float
+  types) always hold initialized data. (They must still be initialized when any actual operation,
+  such as arithmetic, is performed.) Using this flag is **unsound**. This has no effect when
   `-Zmiri-disable-validation` is present.
+* `-Zmiri-allow-ptr-int-transmute` makes Miri more accepting of transmutation between pointers and
+  integers via `mem::transmute` or union/pointer type punning. This has two effects: it disables the
+  check against integers storing a pointer (i.e., data with provenance), thus allowing
+  pointer-to-integer transmutation, and it treats integer-to-pointer transmutation as equivalent to
+  a cast. Using this flag is **unsound**.
 * `-Zmiri-disable-abi-check` disables checking [function ABI]. Using this flag
   is **unsound**.
 * `-Zmiri-disable-alignment-check` disables checking pointer alignment, so you

src/bin/miri.rs

@@ -335,7 +335,16 @@ fn main() {
                     miri_config.check_alignment = miri::AlignmentCheck::Symbolic;
                 }
                 "-Zmiri-check-number-validity" => {
-                    miri_config.check_number_validity = true;
+                    eprintln!(
+                        "WARNING: the flag `-Zmiri-check-number-validity` no longer has any effect \
+                        since it is now enabled by default"
+                    );
+                }
+                "-Zmiri-allow-uninit-numbers" => {
+                    miri_config.allow_uninit_numbers = true;
+                }
+                "-Zmiri-allow-ptr-int-transmute" => {
+                    miri_config.allow_ptr_int_transmute = true;
                 }
                 "-Zmiri-disable-abi-check" => {
                     miri_config.check_abi = false;
@@ -386,7 +395,6 @@ fn main() {
                 "-Zmiri-strict-provenance" => {
                     miri_config.provenance_mode = ProvenanceMode::Strict;
                     miri_config.tag_raw = true;
-                    miri_config.check_number_validity = true;
                 }
                 "-Zmiri-permissive-provenance" => {
                     miri_config.provenance_mode = ProvenanceMode::Permissive;

src/eval.rs

@@ -77,8 +77,10 @@ pub struct MiriConfig {
     pub stacked_borrows: bool,
     /// Controls alignment checking.
     pub check_alignment: AlignmentCheck,
-    /// Controls integer and float validity (e.g., initialization) checking.
-    pub check_number_validity: bool,
+    /// Controls integer and float validity initialization checking.
+    pub allow_uninit_numbers: bool,
+    /// Controls how we treat ptr2int and int2ptr transmutes.
+    pub allow_ptr_int_transmute: bool,
     /// Controls function [ABI](Abi) checking.
     pub check_abi: bool,
     /// Action for an op requiring communication with the host.
@@ -126,7 +128,8 @@ impl Default for MiriConfig {
             validate: true,
             stacked_borrows: true,
             check_alignment: AlignmentCheck::Int,
-            check_number_validity: false,
+            allow_uninit_numbers: false,
+            allow_ptr_int_transmute: false,
             check_abi: true,
             isolated_op: IsolatedOp::Reject(RejectOpWith::Abort),
             ignore_leaks: false,

src/intptrcast.rs

@@ -118,19 +118,12 @@ impl<'mir, 'tcx> GlobalStateInner {
     ) -> Pointer<Option<Tag>> {
         trace!("Transmuting 0x{:x} to a pointer", addr);
 
-        let global_state = ecx.machine.intptrcast.borrow();
-
-        match global_state.provenance_mode {
-            ProvenanceMode::Legacy => {
-                // In legacy mode, we have to support int2ptr transmutes,
-                // so just pretend they do the same thing as a cast.
-                Self::ptr_from_addr_cast(ecx, addr)
-            }
-            ProvenanceMode::Permissive | ProvenanceMode::Strict => {
-                // Both of these modes consider transmuted pointers to be "invalid" (`None`
-                // provenance).
-                Pointer::new(None, Size::from_bytes(addr))
-            }
+        if ecx.machine.allow_ptr_int_transmute {
+            // When we allow transmutes, treat them like casts.
+            Self::ptr_from_addr_cast(ecx, addr)
+        } else {
+            // We consider transmuted pointers to be "invalid" (`None` provenance).
+            Pointer::new(None, Size::from_bytes(addr))
         }
     }
 

src/machine.rs

@@ -255,13 +255,19 @@ pub struct Evaluator<'mir, 'tcx> {
     /// Whether to enforce the validity invariant.
     pub(crate) validate: bool,
 
-    /// Whether to enforce validity (e.g., initialization) of integers and floats.
-    pub(crate) enforce_number_validity: bool,
+    /// Whether to allow uninitialized numbers (integers and floats).
+    pub(crate) allow_uninit_numbers: bool,
+
+    /// Whether to allow ptr2int transmutes, and whether to allow *dereferencing* the result of an
+    /// int2ptr transmute.
+    pub(crate) allow_ptr_int_transmute: bool,
 
     /// Whether to enforce [ABI](Abi) of function calls.
     pub(crate) enforce_abi: bool,
 
+    /// The table of file descriptors.
     pub(crate) file_handler: shims::posix::FileHandler,
+    /// The table of directory descriptors.
     pub(crate) dir_handler: shims::posix::DirHandler,
 
     /// The "time anchor" for this machine's monotone clock (for `Instant` simulation).
@@ -351,7 +357,8 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             tls: TlsData::default(),
             isolated_op: config.isolated_op,
             validate: config.validate,
-            enforce_number_validity: config.check_number_validity,
+            allow_uninit_numbers: config.allow_uninit_numbers,
+            allow_ptr_int_transmute: config.allow_ptr_int_transmute,
             enforce_abi: config.check_abi,
             file_handler: FileHandler::new(config.mute_stdout_stderr),
             dir_handler: Default::default(),
@@ -493,12 +500,12 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
 
     #[inline(always)]
     fn enforce_number_init(ecx: &MiriEvalContext<'mir, 'tcx>) -> bool {
-        ecx.machine.enforce_number_validity
+        !ecx.machine.allow_uninit_numbers
     }
 
     #[inline(always)]
     fn enforce_number_no_provenance(ecx: &MiriEvalContext<'mir, 'tcx>) -> bool {
-        ecx.machine.enforce_number_validity
+        !ecx.machine.allow_ptr_int_transmute
     }
 
     #[inline(always)]

src/shims/posix/sync.rs

@@ -405,8 +405,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         // To catch double-destroys, we de-initialize the mutexattr.
         // This is technically not right and might lead to false positives. For example, the below
-        // code is *likely* sound, even assuming uninit numbers are UB, but miri with
-        // -Zmiri-check-number-validity complains
+        // code is *likely* sound, even assuming uninit numbers are UB, but Miri complains.
         //
         // let mut x: MaybeUninit<libc::pthread_mutexattr_t> = MaybeUninit::zeroed();
         // libc::pthread_mutexattr_init(x.as_mut_ptr());

tests/compile-fail/provenance/ptr_int_unexposed.rs

@@ -1,4 +1,4 @@
-// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows
+// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows -Zmiri-allow-ptr-int-transmute
 
 fn main() {
     let x: i32 = 3;

tests/compile-fail/provenance/ptr_invalid.rs

@@ -1,4 +1,3 @@
-// compile-flags: -Zmiri-permissive-provenance
 #![feature(strict_provenance)]
 
 // Ensure that a `ptr::invalid` ptr is truly invalid.

tests/compile-fail/stacked_borrows/illegal_read3.rs

@@ -1,3 +1,4 @@
+// compile-flags: -Zmiri-allow-ptr-int-transmute
 // A callee may not read the destination of our `&mut` without us noticing.
 // Thise code got carefully checked to not introduce any reborrows
 // that are not explicit in the source. Let's hope the compiler does not break this later!

tests/compile-fail/transmute-pair-uninit.rs

@@ -1,3 +1,4 @@
+// compile-flags: -Zmiri-allow-uninit-numbers
 #![feature(core_intrinsics)]
 
 use std::mem;

tests/compile-fail/uninit_byte_read.rs

@@ -1,3 +1,4 @@
+// compile-flags: -Zmiri-allow-uninit-numbers
 fn main() {
     let v: Vec<u8> = Vec::with_capacity(10);
     let undef = unsafe { *v.get_unchecked(5) };

tests/compile-fail/validity/invalid_enum_tag_256variants_uninit.rs

@@ -1,3 +1,4 @@
+// compile-flags: -Zmiri-allow-uninit-numbers
 #![allow(unused, deprecated, invalid_value)]
 
 #[derive(Copy, Clone)]

tests/compile-fail/validity/ptr_integer_array_transmute.rs

@@ -1,5 +1,3 @@
-// compile-flags: -Zmiri-check-number-validity
-
 fn main() {
     let r = &mut 42;
     let _i: [usize; 1] = unsafe { std::mem::transmute(r) }; //~ ERROR encountered a pointer, but expected plain (non-pointer) bytes

tests/compile-fail/validity/ptr_integer_transmute.rs

@@ -1,5 +1,3 @@
-// compile-flags: -Zmiri-check-number-validity
-
 fn main() {
     let r = &mut 42;
     let _i: usize = unsafe { std::mem::transmute(r) }; //~ ERROR expected plain (non-pointer) bytes

tests/compile-fail/validity/uninit_float.rs

@@ -1,5 +1,3 @@
-// compile-flags: -Zmiri-check-number-validity
-
 // This test is adapted from https://github.com/rust-lang/miri/issues/1340#issue-600900312.
 
 fn main() {

tests/compile-fail/validity/uninit_integer.rs

@@ -1,5 +1,3 @@
-// compile-flags: -Zmiri-check-number-validity
-
 // This test is from https://github.com/rust-lang/miri/issues/1340#issue-600900312.
 
 fn main() {

tests/compile-fail/validity/uninit_integer_signed.rs

@@ -1,5 +1,3 @@
-// compile-flags: -Zmiri-check-number-validity
-
 // This test is adapted from https://github.com/rust-lang/miri/issues/1340#issue-600900312.
 
 fn main() {

tests/run-pass/concurrency/libc_pthread_cond.rs

@@ -1,6 +1,6 @@
 // ignore-windows: No libc on Windows
 // ignore-apple: pthread_condattr_setclock is not supported on MacOS.
-// compile-flags: -Zmiri-disable-isolation -Zmiri-check-number-validity
+// compile-flags: -Zmiri-disable-isolation
 
 #![feature(rustc_private)]
 

tests/run-pass/intptrcast.rs

@@ -1,3 +1,5 @@
+// compile-flags: -Zmiri-allow-ptr-int-transmute
+
 // This returns a miri pointer at type usize, if the argument is a proper pointer
 fn transmute_ptr_to_int<T>(x: *const T) -> usize {
     unsafe { std::mem::transmute(x) }

tests/run-pass/libc.rs

@@ -197,17 +197,17 @@ fn test_prctl_thread_name() {
     use libc::c_long;
     unsafe {
         let mut buf = [255; 10];
-        assert_eq!(libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr() as c_long, 0 as c_long, 0 as c_long, 0 as c_long), 0);
+        assert_eq!(libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr(), 0 as c_long, 0 as c_long, 0 as c_long), 0);
         assert_eq!(b"<unnamed>\0", &buf);
         let thread_name = CString::new("hello").expect("CString::new failed");
-        assert_eq!(libc::prctl(libc::PR_SET_NAME, thread_name.as_ptr() as c_long, 0 as c_long, 0 as c_long, 0 as c_long), 0);
+        assert_eq!(libc::prctl(libc::PR_SET_NAME, thread_name.as_ptr(), 0 as c_long, 0 as c_long, 0 as c_long), 0);
         let mut buf = [255; 6];
-        assert_eq!(libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr() as c_long, 0 as c_long, 0 as c_long, 0 as c_long), 0);
+        assert_eq!(libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr(), 0 as c_long, 0 as c_long, 0 as c_long), 0);
         assert_eq!(b"hello\0", &buf);
         let long_thread_name = CString::new("01234567890123456789").expect("CString::new failed");
-        assert_eq!(libc::prctl(libc::PR_SET_NAME, long_thread_name.as_ptr() as c_long, 0 as c_long, 0 as c_long, 0 as c_long), 0);
+        assert_eq!(libc::prctl(libc::PR_SET_NAME, long_thread_name.as_ptr(), 0 as c_long, 0 as c_long, 0 as c_long), 0);
         let mut buf = [255; 16];
-        assert_eq!(libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr() as c_long, 0 as c_long, 0 as c_long, 0 as c_long), 0);
+        assert_eq!(libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr(), 0 as c_long, 0 as c_long, 0 as c_long), 0);
         assert_eq!(b"012345678901234\0", &buf);
     }
 }

tests/run-pass/move-uninit-primval.rs

@@ -1,3 +1,4 @@
+// compile-flags: -Zmiri-allow-uninit-numbers
 #![allow(deprecated)]
 
 struct Foo {

tests/run-pass/partially-uninit.rs

@@ -1,5 +1,3 @@
-// compile-flags: -Zmiri-check-number-validity
-
 use std::mem::{self, MaybeUninit};
 
 #[repr(C)]

tests/run-pass/ptr_offset.rs

@@ -61,7 +61,9 @@ fn ptr_offset() {
     unsafe {
         let p = f as fn() -> i32 as usize;
         let x = (p as *mut u32).offset(0) as usize;
-        let f: fn() -> i32 = mem::transmute(x);
+        // *cast* to ptr, then transmute to fn ptr.
+        // (transmuting int to [fn]ptr causes trouble.)
+        let f: fn() -> i32 = mem::transmute(x as *const ());
         assert_eq!(f(), 42);
     }
 }

tests/run-pass/tag-align-dyn-u64.rs

@@ -25,7 +25,8 @@ fn mk_rec() -> Rec {
 }
 
 fn is_u64_aligned(u: &Tag<u64>) -> bool {
-    let p: usize = unsafe { mem::transmute(u) };
+    let p: *const () = unsafe { mem::transmute(u) };
+    let p = p as usize;
     let u64_align = std::mem::align_of::<u64>();
     return (p & (u64_align - 1)) == 0;
 }

tests/run-pass/transmute_fat.rs

@@ -1,13 +1,14 @@
 // Stacked Borrows disallows this becuase the reference is never cast to a raw pointer.
-// compile-flags: -Zmiri-disable-stacked-borrows
+// compile-flags: -Zmiri-disable-stacked-borrows -Zmiri-allow-ptr-int-transmute
 
 fn main() {
     // If we are careful, we can exploit data layout...
     let raw = unsafe {
         std::mem::transmute::<&[u8], [usize; 2]>(&[42])
     };
     let ptr = raw[0] + raw[1];
-    let ptr = ptr as *const u8;
+    // We transmute both ways, to really test allow-ptr-int-transmute.
+    let ptr: *const u8 = unsafe { std::mem::transmute(ptr) };
     // The pointer is one-past-the end, but we decrement it into bounds before using it
     assert_eq!(unsafe { *ptr.offset(-1) }, 42);
 }

tests/run-pass/uninit_number_ignored.rs

@@ -1,5 +1,5 @@
+// compile-flags: -Zmiri-allow-uninit-numbers
 // This test is adapted from https://github.com/rust-lang/miri/issues/1340#issue-600900312.
-// This test passes because -Zmiri-check-number-validity is not passed.
 
 fn main() {
     let _val1 = unsafe { std::mem::MaybeUninit::<usize>::uninit().assume_init() };