Auto merge of #1219 - RalfJung:error-cleanup, r=RalfJung

rustup for error reform

This is the Miri side of rust-lang/rust#69839.

Author: bors

rust-version

@@ -1 +1 @@
-660326e9791d5caf3186b14521498c2584a494ab
+57e1da59cd0761330b4ea8d47b16340a78eeafa9

src/helpers.rs

@@ -18,7 +18,7 @@ use crate::*;
 impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 
 /// Gets an instance for a path.
-fn resolve_did<'mir, 'tcx>(tcx: TyCtxt<'tcx>, path: &[&str]) -> InterpResult<'tcx, DefId> {
+fn try_resolve_did<'mir, 'tcx>(tcx: TyCtxt<'tcx>, path: &[&str]) -> Option<DefId> {
     tcx.crates()
         .iter()
         .find(|&&krate| tcx.original_crate_name(krate).as_str() == path[0])
@@ -41,19 +41,47 @@ 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()
-        })
 }
 
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
     /// Gets an instance for a path.
-    fn resolve_path(&self, path: &[&str]) -> InterpResult<'tcx, ty::Instance<'tcx>> {
-        Ok(ty::Instance::mono(
-            self.eval_context_ref().tcx.tcx,
-            resolve_did(self.eval_context_ref().tcx.tcx, path)?,
-        ))
+    fn resolve_path(&self, path: &[&str]) -> ty::Instance<'tcx> {
+        let did = try_resolve_did(self.eval_context_ref().tcx.tcx, path)
+            .unwrap_or_else(|| panic!("failed to find required Rust item: {:?}", path));
+        ty::Instance::mono(self.eval_context_ref().tcx.tcx, did)
+    }
+
+    /// Evaluates the scalar at the specified path. Returns Some(val)
+    /// if the path could be resolved, and None otherwise
+    fn eval_path_scalar(
+        &mut self,
+        path: &[&str],
+    ) -> InterpResult<'tcx, ScalarMaybeUndef<Tag>> {
+        let this = self.eval_context_mut();
+        let instance = this.resolve_path(path);
+        let cid = GlobalId { instance, promoted: None };
+        let const_val = this.const_eval_raw(cid)?;
+        let const_val = this.read_scalar(const_val.into())?;
+        return Ok(const_val);
+    }
+
+    /// Helper function to get a `libc` constant as a `Scalar`.
+    fn eval_libc(&mut self, name: &str) -> InterpResult<'tcx, Scalar<Tag>> {
+        self.eval_context_mut()
+            .eval_path_scalar(&["libc", name])?
+            .not_undef()
+    }
+
+    /// Helper function to get a `libc` constant as an `i32`.
+    fn eval_libc_i32(&mut self, name: &str) -> InterpResult<'tcx, i32> {
+        self.eval_libc(name)?.to_i32()
+    }
+
+    /// Helper function to get the `TyLayout` of a `libc` type
+    fn libc_ty_layout(&mut self, name: &str) -> InterpResult<'tcx, TyLayout<'tcx>> {
+        let this = self.eval_context_mut();
+        let ty = this.resolve_path(&["libc", name]).monomorphic_ty(*this.tcx);
+        this.layout_of(ty)
     }
 
     /// Write a 0 of the appropriate size to `dest`.
@@ -98,7 +126,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         if this.machine.communicate {
             // Fill the buffer using the host's rng.
             getrandom::getrandom(&mut data)
-                .map_err(|err| err_unsup_format!("getrandom failed: {}", err))?;
+                .map_err(|err| err_unsup_format!("host getrandom failed: {}", err))?;
         } else {
             let rng = this.memory.extra.rng.get_mut();
             rng.fill_bytes(&mut data);
@@ -313,26 +341,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
     }
 
-    /// Helper function to get a `libc` constant as a `Scalar`.
-    fn eval_libc(&mut self, name: &str) -> InterpResult<'tcx, Scalar<Tag>> {
-        self.eval_context_mut()
-            .eval_path_scalar(&["libc", name])?
-            .ok_or_else(|| err_unsup_format!("Path libc::{} cannot be resolved.", name))?
-            .not_undef()
-    }
-
-    /// Helper function to get a `libc` constant as an `i32`.
-    fn eval_libc_i32(&mut self, name: &str) -> InterpResult<'tcx, i32> {
-        self.eval_libc(name)?.to_i32()
-    }
-
-    /// Helper function to get the `TyLayout` of a `libc` type
-    fn libc_ty_layout(&mut self, name: &str) -> InterpResult<'tcx, TyLayout<'tcx>> {
-        let this = self.eval_context_mut();
-        let ty = this.resolve_path(&["libc", name])?.monomorphic_ty(*this.tcx);
-        this.layout_of(ty)
-    }
-
     // Writes several `ImmTy`s contiguosly into memory. This is useful when you have to pack
     // different values into a struct.
     fn write_packed_immediates(
@@ -360,7 +368,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     fn check_no_isolation(&self, name: &str) -> InterpResult<'tcx> {
         if !self.eval_context_ref().machine.communicate {
             throw_unsup_format!(
-                "`{}` not available when isolation is enabled. Pass the flag `-Zmiri-disable-isolation` to disable it.",
+                "`{}` not available when isolation is enabled (pass the flag `-Zmiri-disable-isolation` to disable isolation)",
                 name,
             )
         }
@@ -416,13 +424,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 AlreadyExists => "EEXIST",
                 WouldBlock => "EWOULDBLOCK",
                 _ => {
-                    throw_unsup_format!("The {} error cannot be transformed into a raw os error", e)
+                    throw_unsup_format!("io error {} cannot be transformed into a raw os error", e)
                 }
             })?
         } else {
             // FIXME: we have to implement the Windows equivalent of this.
             throw_unsup_format!(
-                "Setting the last OS error from an io::Error is unsupported for {}.",
+                "setting the last OS error from an io::Error is unsupported for {}.",
                 target.target_os
             )
         };
@@ -531,7 +539,7 @@ pub fn immty_from_int_checked<'tcx>(
 ) -> InterpResult<'tcx, ImmTy<'tcx, Tag>> {
     let int = int.into();
     Ok(ImmTy::try_from_int(int, layout).ok_or_else(|| {
-        err_unsup_format!("Signed value {:#x} does not fit in {} bits", int, layout.size.bits())
+        err_unsup_format!("signed value {:#x} does not fit in {} bits", int, layout.size.bits())
     })?)
 }
 
@@ -541,6 +549,6 @@ pub fn immty_from_uint_checked<'tcx>(
 ) -> InterpResult<'tcx, ImmTy<'tcx, Tag>> {
     let int = int.into();
     Ok(ImmTy::try_from_uint(int, layout).ok_or_else(|| {
-        err_unsup_format!("Signed value {:#x} does not fit in {} bits", int, layout.size.bits())
+        err_unsup_format!("unsigned value {:#x} does not fit in {} bits", int, layout.size.bits())
     })?)
 }

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/dlsym.rs

@@ -15,7 +15,7 @@ impl Dlsym {
         Ok(match name {
             "getentropy" => Some(GetEntropy),
             "__pthread_get_minstack" => None,
-            _ => throw_unsup_format!("Unsupported dlsym: {}", name),
+            _ => throw_unsup_format!("unsupported dlsym: {}", name),
         })
     }
 }

src/shims/foreign_items.rs

@@ -165,7 +165,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     .expect("No panic runtime found!");
                 let panic_runtime = tcx.crate_name(*panic_runtime);
                 let start_panic_instance =
-                    this.resolve_path(&[&*panic_runtime.as_str(), link_name])?;
+                    this.resolve_path(&[&*panic_runtime.as_str(), link_name]);
                 return Ok(Some(&*this.load_mir(start_panic_instance.def, None)?));
             }
             _ => {}
@@ -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(
@@ -455,26 +436,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             _ => match this.tcx.sess.target.target.target_os.as_str() {
                 "linux" | "macos" => return posix::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),
                 "windows" => return windows::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),
-                target => throw_unsup_format!("The {} target platform is not supported", target),
+                target => throw_unsup_format!("the {} target platform is not supported", target),
             }
         };
 
         Ok(true)
     }
 
-    /// Evaluates the scalar at the specified path. Returns Some(val)
-    /// if the path could be resolved, and None otherwise
-    fn eval_path_scalar(
-        &mut self,
-        path: &[&str],
-    ) -> InterpResult<'tcx, Option<ScalarMaybeUndef<Tag>>> {
-        let this = self.eval_context_mut();
-        if let Ok(instance) = this.resolve_path(path) {
-            let cid = GlobalId { instance, promoted: None };
-            let const_val = this.const_eval_raw(cid)?;
-            let const_val = this.read_scalar(const_val.into())?;
-            return Ok(Some(const_val));
+    /// 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);
         }
-        return Ok(None);
+        Ok(())
     }
 }

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`).
@@ -294,28 +289,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
                 trace!("sysconf() called with name {}", name);
                 // TODO: Cache the sysconf integers via Miri's global cache.
-                let paths = &[
-                    (&["libc", "_SC_PAGESIZE"], Scalar::from_int(PAGE_SIZE, dest.layout.size)),
-                    (&["libc", "_SC_GETPW_R_SIZE_MAX"], Scalar::from_int(-1, dest.layout.size)),
-                    (
-                        &["libc", "_SC_NPROCESSORS_ONLN"],
-                        Scalar::from_int(NUM_CPUS, dest.layout.size),
-                    ),
+                let sysconfs = &[
+                    ("_SC_PAGESIZE", Scalar::from_int(PAGE_SIZE, dest.layout.size)),
+                    ("_SC_GETPW_R_SIZE_MAX", Scalar::from_int(-1, dest.layout.size)),
+                    ("_SC_NPROCESSORS_ONLN", Scalar::from_int(NUM_CPUS, dest.layout.size)),
                 ];
                 let mut result = None;
-                for &(path, path_value) in paths {
-                    if let Some(val) = this.eval_path_scalar(path)? {
-                        let val = val.to_i32()?;
-                        if val == name {
-                            result = Some(path_value);
-                            break;
-                        }
+                for &(sysconf_name, value) in sysconfs {
+                    let sysconf_name = this.eval_libc_i32(sysconf_name)?;
+                    if sysconf_name == name {
+                        result = Some(value);
+                        break;
                     }
                 }
                 if let Some(result) = result {
                     this.write_scalar(result, dest)?;
                 } else {
-                    throw_unsup_format!("Unimplemented sysconf name: {}", name)
+                    throw_unsup_format!("unimplemented sysconf name: {}", name)
                 }
             }
 

src/shims/foreign_items/posix/linux.rs

@@ -56,13 +56,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
             "syscall" => {
                 let sys_getrandom = this
-                    .eval_path_scalar(&["libc", "SYS_getrandom"])?
-                    .expect("Failed to get libc::SYS_getrandom")
+                    .eval_libc("SYS_getrandom")?
                     .to_machine_usize(this)?;
 
                 let sys_statx = this
-                    .eval_path_scalar(&["libc", "SYS_statx"])?
-                    .expect("Failed to get libc::SYS_statx")
+                    .eval_libc("SYS_statx")?
                     .to_machine_usize(this)?;
 
                 match this.read_scalar(args[0])?.to_machine_usize(this)? {

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" => {