adjust for error reform

Author: RalfJung

src/helpers.rs

@@ -42,8 +42,7 @@ fn resolve_did<'mir, 'tcx>(tcx: TyCtxt<'tcx>, path: &[&str]) -> InterpResult<'tc
             None
         })
         .ok_or_else(|| {
-            let path = path.iter().map(|&s| s.to_owned()).collect();
-            err_unsup!(PathNotFound(path)).into()
+            err_unsup_format!("failed to find required Rust item: {:?}", path).into()
         })
 }
 

src/intptrcast.rs

@@ -43,10 +43,6 @@ impl<'mir, 'tcx> GlobalState {
         int: u64,
         memory: &Memory<'mir, 'tcx, Evaluator<'tcx>>,
     ) -> InterpResult<'tcx, Pointer<Tag>> {
-        if int == 0 {
-            throw_unsup!(InvalidNullPointerUsage);
-        }
-
         let global_state = memory.extra.intptrcast.borrow();
         let pos = global_state.int_to_ptr_map.binary_search_by_key(&int, |(addr, _)| *addr);
 
@@ -57,7 +53,7 @@ impl<'mir, 'tcx> GlobalState {
                 // zero. The pointer is untagged because it was created from a cast
                 Pointer::new_with_tag(alloc_id, Size::from_bytes(0), Tag::Untagged)
             }
-            Err(0) => throw_unsup!(DanglingPointerDeref),
+            Err(0) => throw_ub!(InvalidIntPointerUsage(int)),
             Err(pos) => {
                 // This is the largest of the adresses smaller than `int`,
                 // i.e. the greatest lower bound (glb)
@@ -69,7 +65,7 @@ impl<'mir, 'tcx> GlobalState {
                     // This pointer is untagged because it was created from a cast
                     Pointer::new_with_tag(alloc_id, Size::from_bytes(offset), Tag::Untagged)
                 } else {
-                    throw_unsup!(DanglingPointerDeref)
+                    throw_ub!(InvalidIntPointerUsage(int))
                 }
             }
         })

src/shims/foreign_items.rs

@@ -222,12 +222,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             "__rust_alloc" => {
                 let size = this.read_scalar(args[0])?.to_machine_usize(this)?;
                 let align = this.read_scalar(args[1])?.to_machine_usize(this)?;
-                if size == 0 {
-                    throw_unsup!(HeapAllocZeroBytes);
-                }
-                if !align.is_power_of_two() {
-                    throw_unsup!(HeapAllocNonPowerOfTwoAlignment(align));
-                }
+                Self::check_alloc_request(size, align)?;
                 let ptr = this.memory.allocate(
                     Size::from_bytes(size),
                     Align::from_bytes(align).unwrap(),
@@ -238,12 +233,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             "__rust_alloc_zeroed" => {
                 let size = this.read_scalar(args[0])?.to_machine_usize(this)?;
                 let align = this.read_scalar(args[1])?.to_machine_usize(this)?;
-                if size == 0 {
-                    throw_unsup!(HeapAllocZeroBytes);
-                }
-                if !align.is_power_of_two() {
-                    throw_unsup!(HeapAllocNonPowerOfTwoAlignment(align));
-                }
+                Self::check_alloc_request(size, align)?;
                 let ptr = this.memory.allocate(
                     Size::from_bytes(size),
                     Align::from_bytes(align).unwrap(),
@@ -257,12 +247,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let ptr = this.read_scalar(args[0])?.not_undef()?;
                 let old_size = this.read_scalar(args[1])?.to_machine_usize(this)?;
                 let align = this.read_scalar(args[2])?.to_machine_usize(this)?;
-                if old_size == 0 {
-                    throw_unsup!(HeapAllocZeroBytes);
-                }
-                if !align.is_power_of_two() {
-                    throw_unsup!(HeapAllocNonPowerOfTwoAlignment(align));
-                }
+                // No need to check old_size/align; we anyway check that they match the allocation.
                 let ptr = this.force_ptr(ptr)?;
                 this.memory.deallocate(
                     ptr,
@@ -274,12 +259,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let old_size = this.read_scalar(args[1])?.to_machine_usize(this)?;
                 let align = this.read_scalar(args[2])?.to_machine_usize(this)?;
                 let new_size = this.read_scalar(args[3])?.to_machine_usize(this)?;
-                if old_size == 0 || new_size == 0 {
-                    throw_unsup!(HeapAllocZeroBytes);
-                }
-                if !align.is_power_of_two() {
-                    throw_unsup!(HeapAllocNonPowerOfTwoAlignment(align));
-                }
+                Self::check_alloc_request(new_size, align)?;
+                // No need to check old_size; we anyway check that they match the allocation.
                 let ptr = this.force_ptr(this.read_scalar(args[0])?.not_undef()?)?;
                 let align = Align::from_bytes(align).unwrap();
                 let new_ptr = this.memory.reallocate(
@@ -462,6 +443,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         Ok(true)
     }
 
+    /// Check some basic requirements for this allocation request:
+    /// non-zero size, power-of-two alignment.
+    fn check_alloc_request(size: u64, align: u64) -> InterpResult<'tcx> {
+        if size == 0 {
+            throw_ub_format!("creating allocation with size 0");
+        }
+        if !align.is_power_of_two() {
+            throw_ub_format!("creating allocation with non-power-of-two alignment {}", align);
+        }
+        Ok(())
+    }
+
     /// Evaluates the scalar at the specified path. Returns Some(val)
     /// if the path could be resolved, and None otherwise
     fn eval_path_scalar(

src/shims/foreign_items/posix.rs

@@ -138,7 +138,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let size = this.read_scalar(args[2])?.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_unsup!(HeapAllocNonPowerOfTwoAlignment(align));
+                    throw_ub_format!("posix_memalign: alignment must be a power of two, but is {}", align);
                 }
                 if align < this.pointer_size().bytes() {
                     throw_ub_format!(
@@ -185,7 +185,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 };
 
                 // Figure out how large a pthread TLS key actually is.
-                // This is `libc::pthread_key_t`.
+                // To this end, deref the argument type. This is `libc::pthread_key_t`.
                 let key_type = args[0].layout.ty
                     .builtin_deref(true)
                     .ok_or_else(|| err_ub_format!(
@@ -195,12 +195,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let key_layout = this.layout_of(key_type)?;
 
                 // Create key and write it into the memory where `key_ptr` wants it.
-                let key = this.machine.tls.create_tls_key(dtor) as u128;
-                if key_layout.size.bits() < 128 && key >= (1u128 << key_layout.size.bits() as u128)
-                {
-                    throw_unsup!(OutOfTls);
-                }
-
+                let key = this.machine.tls.create_tls_key(dtor, key_layout.size)?;
                 this.write_scalar(Scalar::from_uint(key, key_layout.size), key_place.into())?;
 
                 // Return success (`0`).

src/shims/foreign_items/windows.rs

@@ -154,14 +154,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 // This just creates a key; Windows does not natively support TLS destructors.
 
                 // Create key and return it.
-                let key = this.machine.tls.create_tls_key(None) as u128;
-
-                // Figure out how large a TLS key actually is. This is `c::DWORD`.
-                if dest.layout.size.bits() < 128
-                    && key >= (1u128 << dest.layout.size.bits() as u128)
-                {
-                    throw_unsup!(OutOfTls);
-                }
+                let key = this.machine.tls.create_tls_key(None, dest.layout.size)?;
                 this.write_scalar(Scalar::from_uint(key, dest.layout.size), dest)?;
             }
             "TlsGetValue" => {

src/shims/tls.rs

@@ -2,7 +2,7 @@
 
 use std::collections::BTreeMap;
 
-use rustc::{ty, ty::layout::HasDataLayout};
+use rustc::{ty, ty::layout::{Size, HasDataLayout}};
 use rustc_target::abi::LayoutOf;
 
 use crate::{HelpersEvalContextExt, InterpResult, MPlaceTy, Scalar, StackPopCleanup, Tag};
@@ -37,12 +37,18 @@ impl<'tcx> Default for TlsData<'tcx> {
 }
 
 impl<'tcx> TlsData<'tcx> {
-    pub fn create_tls_key(&mut self, dtor: Option<ty::Instance<'tcx>>) -> TlsKey {
+    /// Generate a new TLS key with the given destructor.
+    /// `max_size` determines the integer size the key has to fit in.
+    pub fn create_tls_key(&mut self, dtor: Option<ty::Instance<'tcx>>, max_size: Size) -> InterpResult<'tcx, TlsKey> {
         let new_key = self.next_key;
         self.next_key += 1;
         self.keys.insert(new_key, TlsEntry { data: None, dtor }).unwrap_none();
         trace!("New TLS key allocated: {} with dtor {:?}", new_key, dtor);
-        new_key
+
+        if max_size.bits() < 128 && new_key >= (1u128 << max_size.bits() as u128) {
+            throw_unsup_format!("we ran out of TLS key space");
+        }
+        Ok(new_key)
     }
 
     pub fn delete_tls_key(&mut self, key: TlsKey) -> InterpResult<'tcx> {
@@ -51,7 +57,7 @@ impl<'tcx> TlsData<'tcx> {
                 trace!("TLS key {} removed", key);
                 Ok(())
             }
-            None => throw_unsup!(TlsOutOfBounds),
+            None => throw_ub_format!("removing a non-existig TLS key: {}", key),
         }
     }
 
@@ -65,7 +71,7 @@ impl<'tcx> TlsData<'tcx> {
                 trace!("TLS key {} loaded: {:?}", key, data);
                 Ok(data.unwrap_or_else(|| Scalar::ptr_null(cx).into()))
             }
-            None => throw_unsup!(TlsOutOfBounds),
+            None => throw_ub_format!("loading from a non-existing TLS key: {}", key),
         }
     }
 
@@ -76,7 +82,7 @@ impl<'tcx> TlsData<'tcx> {
                 *data = new_data;
                 Ok(())
             }
-            None => throw_unsup!(TlsOutOfBounds),
+            None => throw_ub_format!("storing to a non-existing TLS key: {}", key),
         }
     }
 

tests/compile-fail/alignment.rs

@@ -5,7 +5,7 @@ fn main() {
     let x_ptr: *mut u8 = &mut x[0];
     let y_ptr = x_ptr as *mut u64;
     unsafe {
-        *y_ptr = 42; //~ ERROR tried to access memory with alignment 1, but alignment
+        *y_ptr = 42; //~ ERROR accessing memory with alignment 1, but alignment
     }
     panic!("unreachable in miri");
 }

tests/compile-fail/atomic_unaligned.rs

@@ -7,6 +7,6 @@ fn main() {
     let zptr = &z as *const _ as *const u64;
     unsafe {
         ::std::intrinsics::atomic_load(zptr);
-        //~^ ERROR tried to access memory with alignment 4, but alignment 8 is required
+        //~^ ERROR accessing memory with alignment 4, but alignment 8 is required
     }
 }

tests/compile-fail/cast_box_int_to_fn_ptr.rs

@@ -7,5 +7,5 @@ fn main() {
         std::mem::transmute::<&Box<usize>, &fn(i32)>(&b)
     };
 
-    (*g)(42) //~ ERROR tried to treat a memory pointer as a function pointer
+    (*g)(42) //~ ERROR it does not point to a function
 }

tests/compile-fail/cast_fn_ptr1.rs

@@ -5,5 +5,5 @@ fn main() {
         std::mem::transmute::<fn(), fn(i32)>(f)
     };
 
-    g(42) //~ ERROR tried to call a function with incorrect number of arguments
+    g(42) //~ ERROR calling a function with more arguments than it expected
 }