Auto merge of #2485 - 5225225:memalign, r=RalfJung

Breaking posix_memalign precondition is not UB

The `size==0` test here might be overtesting, but I figured might as well test it and leave a comment saying it is fine to remove it if the implementation changes.

Fixes #2099

Author: bors

src/shims/unix/foreign_items.rs

@@ -186,27 +186,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let align = this.read_scalar(align)?.to_machine_usize(this)?;
                 let size = this.read_scalar(size)?.to_machine_usize(this)?;
                 // Align must be power of 2, and also at least ptr-sized (POSIX rules).
-                if !align.is_power_of_two() {
-                    throw_ub_format!("posix_memalign: alignment must be a power of two, but is {}", align);
-                }
-                if align < this.pointer_size().bytes() {
-                    throw_ub_format!(
-                        "posix_memalign: alignment must be at least the size of a pointer, but is {}",
-                        align,
-                    );
-                }
-
-                if size == 0 {
-                    this.write_null(&ret.into())?;
+                // But failure to adhere to this is not UB, it's an error condition.
+                if !align.is_power_of_two() || align < this.pointer_size().bytes() {
+                    let einval = this.eval_libc_i32("EINVAL")?;
+                    this.write_int(einval, dest)?;
                 } else {
-                    let ptr = this.allocate_ptr(
-                        Size::from_bytes(size),
-                        Align::from_bytes(align).unwrap(),
-                        MiriMemoryKind::C.into(),
-                    )?;
-                    this.write_pointer(ptr, &ret.into())?;
+                    if size == 0 {
+                        this.write_null(&ret.into())?;
+                    } else {
+                        let ptr = this.allocate_ptr(
+                            Size::from_bytes(size),
+                            Align::from_bytes(align).unwrap(),
+                            MiriMemoryKind::C.into(),
+                        )?;
+                        this.write_pointer(ptr, &ret.into())?;
+                    }
+                    this.write_null(dest)?;
                 }
-                this.write_null(dest)?;
             }
 
             // Dynamic symbol loading

tests/pass/posix_memalign.rs

@@ -0,0 +1,83 @@
+//@ignore-target-windows: No libc on Windows
+
+#![feature(rustc_private)]
+#![feature(pointer_is_aligned)]
+#![feature(strict_provenance)]
+
+use core::ptr;
+
+fn main() {
+    // A normal allocation.
+    unsafe {
+        let mut ptr: *mut libc::c_void = ptr::null_mut();
+        let align = 8;
+        let size = 64;
+        assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0);
+        assert!(!ptr.is_null());
+        assert!(ptr.is_aligned_to(align));
+        ptr.cast::<u8>().write_bytes(1, size);
+        libc::free(ptr);
+    }
+
+    // Align > size.
+    unsafe {
+        let mut ptr: *mut libc::c_void = ptr::null_mut();
+        let align = 64;
+        let size = 8;
+        assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0);
+        assert!(!ptr.is_null());
+        assert!(ptr.is_aligned_to(align));
+        ptr.cast::<u8>().write_bytes(1, size);
+        libc::free(ptr);
+    }
+
+    // Size not multiple of align
+    unsafe {
+        let mut ptr: *mut libc::c_void = ptr::null_mut();
+        let align = 16;
+        let size = 31;
+        assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0);
+        assert!(!ptr.is_null());
+        assert!(ptr.is_aligned_to(align));
+        ptr.cast::<u8>().write_bytes(1, size);
+        libc::free(ptr);
+    }
+
+    // Size == 0
+    unsafe {
+        let mut ptr: *mut libc::c_void = ptr::null_mut();
+        let align = 64;
+        let size = 0;
+        assert_eq!(libc::posix_memalign(&mut ptr, align, size), 0);
+        // We are not required to return null if size == 0, but we currently do.
+        // It's fine to remove this assert if we start returning non-null pointers.
+        assert!(ptr.is_null());
+        assert!(ptr.is_aligned_to(align));
+        // Regardless of what we return, it must be `free`able.
+        libc::free(ptr);
+    }
+
+    // Non-power of 2 align
+    unsafe {
+        let mut ptr: *mut libc::c_void = ptr::invalid_mut(0x1234567);
+        let align = 15;
+        let size = 8;
+        assert_eq!(libc::posix_memalign(&mut ptr, align, size), libc::EINVAL);
+        // The pointer is not modified on failure, posix_memalign(3) says:
+        // > On Linux (and other systems), posix_memalign() does  not  modify  memptr  on failure.
+        // > A requirement standardizing this behavior was added in POSIX.1-2008 TC2.
+        assert_eq!(ptr.addr(), 0x1234567);
+    }
+
+    // Too small align (smaller than ptr)
+    unsafe {
+        let mut ptr: *mut libc::c_void = ptr::invalid_mut(0x1234567);
+        let align = std::mem::size_of::<usize>() / 2;
+        let size = 8;
+        assert_eq!(libc::posix_memalign(&mut ptr, align, size), libc::EINVAL);
+        // The pointer is not modified on failure, posix_memalign(3) says:
+        // > On Linux (and other systems), posix_memalign() does  not  modify  memptr  on failure.
+        // > A requirement standardizing this behavior was added in POSIX.1-2008 TC2.
+        assert_eq!(ptr.addr(), 0x1234567);
+    }
+}