addressing comments

Author: devnexen

src/shims/alloc.rs

@@ -64,6 +64,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Align::from_bytes(prev_power_of_two(size)).unwrap()
     }
 
+    fn min_align(&self, align: u64, size: u64) -> Align {
+        let mut align = Align::from_bytes(align).unwrap();
+        let min_align = self.malloc_align(size);
+        if align < min_align {
+            align = min_align
+        }
+        align
+    }
+
     /// Emulates calling the internal __rust_* allocator functions
     fn emulate_allocator(
         &mut self,

src/shims/unix/foreign_items.rs

@@ -3,6 +3,7 @@ use std::str;
 
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_span::Symbol;
+use rustc_target::abi::Size;
 use rustc_target::spec::abi::Abi;
 
 use crate::shims::alloc::EvalContextExt as _;
@@ -303,36 +304,32 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let align = this.read_target_usize(align)?;
                 let size = this.read_target_usize(size)?;
 
-                // The size must be a power of the alignment.
-                // The specs says nothing about error handling other than null
-                // pointer but aligned_alloc Linux/macOs implementations set EINVAL. Note that
-                // on FreeBSD setting a size not being a power of alignment is UB but we do not
-                // want it.
+                // The size must be a multiple of the alignment.
+                // Alignment must be a power of 2, and "supported by the implementation".
+                // If any of these are violated, we have to return NULL.
+                // All fundamental alignments must be supported.
+                // We decide that our implementation supports all alignments.
                 //
+                // macOS and Illumos are buggy in that they require the alignment
+                // to be at least the size of a pointer. We do not emulate those platform bugs.
+                //
+                // Linux also sets errno to EINVAL, but that's non-standard behavior that we do not
+                // emulate/
+                // FreeBSD says some of these cases are UB but that's violating the C standard.
                 // http://en.cppreference.com/w/cpp/memory/c/aligned_alloc
                 // Linux: https://linux.die.net/man/3/aligned_alloc
                 // FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=aligned_alloc&apropos=0&sektion=3&manpath=FreeBSD+9-current&format=html
                 match size.checked_rem(align) {
-                    Some(0) => {
-                        if !align.is_power_of_two() {
-                            let _ = this.set_last_error(this.eval_libc("EINVAL"));
-                            this.write_null(dest)?;
-                        } else {
-                            let mut align = Align::from_bytes(align).unwrap();
-                            let min_align = this.malloc_align(size);
-                            if align < min_align {
-                                align = min_align;
-                            }
+                    Some(0) if align.is_power_of_two() => {
+                            let align = this.min_align(align, size);
                             let ptr = this.allocate_ptr(
                                 Size::from_bytes(size),
                                 align,
                                 MiriMemoryKind::C.into(),
                             )?;
                             this.write_pointer(ptr, dest)?;
-                        }
                     },
                     _ => {
-                        let _ = this.set_last_error(this.eval_libc("EINVAL"));
                         this.write_null(dest)?;
                     }
                 }