C string function shims: consistently treat "invalid" pointers as UB

Author: RalfJung

src/shims/foreign_items.rs

@@ -690,6 +690,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let right = this.read_pointer(right)?;
                 let n = Size::from_bytes(this.read_target_usize(n)?);
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(left)?;
+                this.ptr_get_alloc_id(right)?;
+
                 let result = {
                     let left_bytes = this.read_bytes_ptr_strip_provenance(left, n)?;
                     let right_bytes = this.read_bytes_ptr_strip_provenance(right, n)?;
@@ -714,6 +718,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
                 let val = val as u8;
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(ptr)?;
+
                 if let Some(idx) = this
                     .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
                     .iter()
@@ -738,6 +745,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
                 let val = val as u8;
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(ptr)?;
+
                 let idx = this
                     .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
                     .iter()
@@ -752,6 +762,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "strlen" => {
                 let [ptr] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let ptr = this.read_pointer(ptr)?;
+                // This reads at least 1 byte, so we are already enforcing that this is a valid pointer.
                 let n = this.read_c_str(ptr)?.len();
                 this.write_scalar(
                     Scalar::from_target_usize(u64::try_from(n).unwrap(), this),
@@ -791,6 +802,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 // pointer provenance is preserved by this implementation of `strcpy`.
                 // That is probably overly cautious, but there also is no fundamental
                 // reason to have `strcpy` destroy pointer provenance.
+                // This reads at least 1 byte, so we are already enforcing that this is a valid pointer.
                 let n = this.read_c_str(ptr_src)?.len().checked_add(1).unwrap();
                 this.mem_copy(
                     ptr_src,

tests/fail/shims/memchr_null.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: memory access failed: null pointer is a dangling pointer (it has no provenance)
+error: Undefined Behavior: out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
   --> $DIR/memchr_null.rs:LL:CC
    |
 LL |         libc::memchr(ptr::null(), 0, 0);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: null pointer is a dangling pointer (it has no provenance)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/shims/memcmp_null.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: memory access failed: null pointer is a dangling pointer (it has no provenance)
+error: Undefined Behavior: out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
   --> $DIR/memcmp_null.rs:LL:CC
    |
 LL |         libc::memcmp(ptr::null(), ptr::null(), 0);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: null pointer is a dangling pointer (it has no provenance)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/shims/memcmp_zero.rs

@@ -0,0 +1,13 @@
+//@ignore-target-windows: No libc on Windows
+//@compile-flags: -Zmiri-permissive-provenance
+
+// C says that passing "invalid" pointers is UB for all string functions.
+// It is unclear whether `(int*)42` is "invalid", but there is no actually
+// a `char` living at that address, so arguably it cannot be a valid pointer.
+// Hence this is UB.
+fn main() {
+    let ptr = 42 as *const u8;
+    unsafe {
+        libc::memcmp(ptr.cast(), ptr.cast(), 0); //~ERROR: dangling
+    }
+}

tests/fail/shims/memcmp_zero.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: out-of-bounds pointer use: 0x2a[noalloc] is a dangling pointer (it has no provenance)
+  --> $DIR/memcmp_zero.rs:LL:CC
+   |
+LL |         libc::memcmp(ptr.cast(), ptr.cast(), 0);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: 0x2a[noalloc] is a dangling pointer (it has no provenance)
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/memcmp_zero.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

tests/fail/shims/memrchr_null.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: memory access failed: null pointer is a dangling pointer (it has no provenance)
+error: Undefined Behavior: out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
   --> $DIR/memrchr_null.rs:LL:CC
    |
 LL |         libc::memrchr(ptr::null(), 0, 0);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: null pointer is a dangling pointer (it has no provenance)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information