Auto merge of #2727 - RalfJung:flags-and-libc, r=RalfJung

make flag checks reobust against multi-bit flags

and make eval_libc functions ICE on any problem

Author: bors

src/tools/miri/src/helpers.rs

@@ -138,55 +138,77 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             .unwrap_or_else(|| panic!("failed to find required Rust item: {path:?}"))
     }
 
-    /// Evaluates the scalar at the specified path. Returns Some(val)
-    /// if the path could be resolved, and None otherwise
-    fn eval_path_scalar(&self, path: &[&str]) -> InterpResult<'tcx, Scalar<Provenance>> {
+    /// Evaluates the scalar at the specified path.
+    fn eval_path_scalar(&self, path: &[&str]) -> Scalar<Provenance> {
         let this = self.eval_context_ref();
         let instance = this.resolve_path(path, Namespace::ValueNS);
         let cid = GlobalId { instance, promoted: None };
         // We don't give a span -- this isn't actually used directly by the program anyway.
-        let const_val = this.eval_global(cid, None)?;
+        let const_val = this
+            .eval_global(cid, None)
+            .unwrap_or_else(|err| panic!("failed to evaluate required Rust item: {path:?}\n{err}"));
         this.read_scalar(&const_val.into())
+            .unwrap_or_else(|err| panic!("failed to read required Rust item: {path:?}\n{err}"))
     }
 
     /// Helper function to get a `libc` constant as a `Scalar`.
-    fn eval_libc(&self, name: &str) -> InterpResult<'tcx, Scalar<Provenance>> {
+    fn eval_libc(&self, name: &str) -> Scalar<Provenance> {
         self.eval_path_scalar(&["libc", name])
     }
 
     /// Helper function to get a `libc` constant as an `i32`.
-    fn eval_libc_i32(&self, name: &str) -> InterpResult<'tcx, i32> {
+    fn eval_libc_i32(&self, name: &str) -> i32 {
         // TODO: Cache the result.
-        self.eval_libc(name)?.to_i32()
+        self.eval_libc(name).to_i32().unwrap_or_else(|_err| {
+            panic!("required libc item has unexpected type (not `i32`): {name}")
+        })
+    }
+
+    /// Helper function to get a `libc` constant as an `u32`.
+    fn eval_libc_u32(&self, name: &str) -> u32 {
+        // TODO: Cache the result.
+        self.eval_libc(name).to_u32().unwrap_or_else(|_err| {
+            panic!("required libc item has unexpected type (not `u32`): {name}")
+        })
     }
 
     /// Helper function to get a `windows` constant as a `Scalar`.
-    fn eval_windows(&self, module: &str, name: &str) -> InterpResult<'tcx, Scalar<Provenance>> {
+    fn eval_windows(&self, module: &str, name: &str) -> Scalar<Provenance> {
         self.eval_context_ref().eval_path_scalar(&["std", "sys", "windows", module, name])
     }
 
+    /// Helper function to get a `windows` constant as a `u32`.
+    fn eval_windows_u32(&self, module: &str, name: &str) -> u32 {
+        // TODO: Cache the result.
+        self.eval_windows(module, name).to_u32().unwrap_or_else(|_err| {
+            panic!("required Windows item has unexpected type (not `u32`): {module}::{name}")
+        })
+    }
+
     /// Helper function to get a `windows` constant as a `u64`.
-    fn eval_windows_u64(&self, module: &str, name: &str) -> InterpResult<'tcx, u64> {
+    fn eval_windows_u64(&self, module: &str, name: &str) -> u64 {
         // TODO: Cache the result.
-        self.eval_windows(module, name)?.to_u64()
+        self.eval_windows(module, name).to_u64().unwrap_or_else(|_err| {
+            panic!("required Windows item has unexpected type (not `u64`): {module}::{name}")
+        })
     }
 
     /// Helper function to get the `TyAndLayout` of a `libc` type
-    fn libc_ty_layout(&self, name: &str) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
+    fn libc_ty_layout(&self, name: &str) -> TyAndLayout<'tcx> {
         let this = self.eval_context_ref();
         let ty = this
             .resolve_path(&["libc", name], Namespace::TypeNS)
             .ty(*this.tcx, ty::ParamEnv::reveal_all());
-        this.layout_of(ty)
+        this.layout_of(ty).unwrap()
     }
 
     /// Helper function to get the `TyAndLayout` of a `windows` type
-    fn windows_ty_layout(&self, name: &str) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
+    fn windows_ty_layout(&self, name: &str) -> TyAndLayout<'tcx> {
         let this = self.eval_context_ref();
         let ty = this
             .resolve_path(&["std", "sys", "windows", "c", name], Namespace::TypeNS)
             .ty(*this.tcx, ty::ParamEnv::reveal_all());
-        this.layout_of(ty)
+        this.layout_of(ty).unwrap()
     }
 
     /// Project to the given *named* field of the mplace (which must be a struct or union type).
@@ -609,14 +631,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         if target.families.iter().any(|f| f == "unix") {
             for &(name, kind) in UNIX_IO_ERROR_TABLE {
                 if err_kind == kind {
-                    return this.eval_libc(name);
+                    return Ok(this.eval_libc(name));
                 }
             }
             throw_unsup_format!("io error {:?} cannot be translated into a raw os error", err_kind)
         } else if target.families.iter().any(|f| f == "windows") {
             // FIXME: we have to finish implementing the Windows equivalent of this.
             use std::io::ErrorKind::*;
-            this.eval_windows(
+            Ok(this.eval_windows(
                 "c",
                 match err_kind {
                     NotFound => "ERROR_FILE_NOT_FOUND",
@@ -627,7 +649,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                             err_kind
                         ),
                 },
-            )
+            ))
         } else {
             throw_unsup_format!(
                 "converting io::Error into errnum is unsupported for OS {}",
@@ -647,7 +669,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         if target.families.iter().any(|f| f == "unix") {
             let errnum = errnum.to_i32()?;
             for &(name, kind) in UNIX_IO_ERROR_TABLE {
-                if errnum == this.eval_libc_i32(name)? {
+                if errnum == this.eval_libc_i32(name) {
                     return Ok(Some(kind));
                 }
             }

src/tools/miri/src/shims/env.rs

@@ -170,7 +170,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 ))
             }
             None => {
-                let envvar_not_found = this.eval_windows("c", "ERROR_ENVVAR_NOT_FOUND")?;
+                let envvar_not_found = this.eval_windows("c", "ERROR_ENVVAR_NOT_FOUND");
                 this.set_last_error(envvar_not_found)?;
                 Scalar::from_u32(0) // return zero upon failure
             }
@@ -240,7 +240,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             Ok(0) // return zero on success
         } else {
             // name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
-            let einval = this.eval_libc("EINVAL")?;
+            let einval = this.eval_libc("EINVAL");
             this.set_last_error(einval)?;
             Ok(-1)
         }
@@ -274,15 +274,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?;
                 this.update_environ()?;
             }
-            Ok(this.eval_windows("c", "TRUE")?)
+            Ok(this.eval_windows("c", "TRUE"))
         } else {
             let value = this.read_os_str_from_wide_str(value_ptr)?;
             let var_ptr = alloc_env_var_as_wide_str(&name, &value, this)?;
             if let Some(var) = this.machine.env_vars.map.insert(name, var_ptr) {
                 this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?;
             }
             this.update_environ()?;
-            Ok(this.eval_windows("c", "TRUE")?)
+            Ok(this.eval_windows("c", "TRUE"))
         }
     }
 
@@ -306,7 +306,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             Ok(0)
         } else {
             // name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
-            let einval = this.eval_libc("EINVAL")?;
+            let einval = this.eval_libc("EINVAL");
             this.set_last_error(einval)?;
             Ok(-1)
         }
@@ -335,7 +335,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 if this.write_path_to_c_str(&cwd, buf, size)?.0 {
                     return Ok(buf);
                 }
-                let erange = this.eval_libc("ERANGE")?;
+                let erange = this.eval_libc("ERANGE");
                 this.set_last_error(erange)?;
             }
             Err(e) => this.set_last_error_from_io_error(e.kind())?,
@@ -411,14 +411,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             this.reject_in_isolation("`SetCurrentDirectoryW`", reject_with)?;
             this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
 
-            return this.eval_windows("c", "FALSE");
+            return Ok(this.eval_windows("c", "FALSE"));
         }
 
         match env::set_current_dir(path) {
-            Ok(()) => this.eval_windows("c", "TRUE"),
+            Ok(()) => Ok(this.eval_windows("c", "TRUE")),
             Err(e) => {
                 this.set_last_error_from_io_error(e.kind())?;
-                this.eval_windows("c", "FALSE")
+                Ok(this.eval_windows("c", "FALSE"))
             }
         }
     }

src/tools/miri/src/shims/time.rs

@@ -36,26 +36,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 // Linux further distinguishes regular and "coarse" clocks, but the "coarse" version
                 // is just specified to be "faster and less precise", so we implement both the same way.
                 absolute_clocks = vec![
-                    this.eval_libc_i32("CLOCK_REALTIME")?,
-                    this.eval_libc_i32("CLOCK_REALTIME_COARSE")?,
+                    this.eval_libc_i32("CLOCK_REALTIME"),
+                    this.eval_libc_i32("CLOCK_REALTIME_COARSE"),
                 ];
                 // The second kind is MONOTONIC clocks for which 0 is an arbitrary time point, but they are
                 // never allowed to go backwards. We don't need to do any additonal monotonicity
                 // enforcement because std::time::Instant already guarantees that it is monotonic.
                 relative_clocks = vec![
-                    this.eval_libc_i32("CLOCK_MONOTONIC")?,
-                    this.eval_libc_i32("CLOCK_MONOTONIC_COARSE")?,
+                    this.eval_libc_i32("CLOCK_MONOTONIC"),
+                    this.eval_libc_i32("CLOCK_MONOTONIC_COARSE"),
                 ];
             }
             "macos" => {
-                absolute_clocks = vec![this.eval_libc_i32("CLOCK_REALTIME")?];
-                relative_clocks = vec![this.eval_libc_i32("CLOCK_MONOTONIC")?];
+                absolute_clocks = vec![this.eval_libc_i32("CLOCK_REALTIME")];
+                relative_clocks = vec![this.eval_libc_i32("CLOCK_MONOTONIC")];
                 // Some clocks only seem to exist in the aarch64 version of the target.
                 if this.tcx.sess.target.arch == "aarch64" {
                     // `CLOCK_UPTIME_RAW` supposed to not increment while the system is asleep... but
                     // that's not really something a program running inside Miri can tell, anyway.
                     // We need to support it because std uses it.
-                    relative_clocks.push(this.eval_libc_i32("CLOCK_UPTIME_RAW")?);
+                    relative_clocks.push(this.eval_libc_i32("CLOCK_UPTIME_RAW"));
                 }
             }
             target => throw_unsup_format!("`clock_gettime` is not supported on target OS {target}"),
@@ -68,7 +68,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             this.machine.clock.now().duration_since(this.machine.clock.anchor())
         } else {
             // Unsupported clock.
-            let einval = this.eval_libc("EINVAL")?;
+            let einval = this.eval_libc("EINVAL");
             this.set_last_error(einval)?;
             return Ok(Scalar::from_i32(-1));
         };
@@ -94,7 +94,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         // Using tz is obsolete and should always be null
         let tz = this.read_pointer(tz_op)?;
         if !this.ptr_is_null(tz)? {
-            let einval = this.eval_libc("EINVAL")?;
+            let einval = this.eval_libc("EINVAL");
             this.set_last_error(einval)?;
             return Ok(-1);
         }
@@ -118,9 +118,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.assert_target_os("windows", "GetSystemTimeAsFileTime");
         this.check_no_isolation("`GetSystemTimeAsFileTime`")?;
 
-        let NANOS_PER_SEC = this.eval_windows_u64("time", "NANOS_PER_SEC")?;
-        let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC")?;
-        let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH")?;
+        let NANOS_PER_SEC = this.eval_windows_u64("time", "NANOS_PER_SEC");
+        let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC");
+        let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH");
         let NANOS_PER_INTERVAL = NANOS_PER_SEC / INTERVALS_PER_SEC;
         let SECONDS_TO_UNIX_EPOCH = INTERVALS_TO_UNIX_EPOCH / INTERVALS_PER_SEC;
 
@@ -226,7 +226,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let duration = match this.read_timespec(&this.deref_operand(req_op)?)? {
             Some(duration) => duration,
             None => {
-                let einval = this.eval_libc("EINVAL")?;
+                let einval = this.eval_libc("EINVAL");
                 this.set_last_error(einval)?;
                 return Ok(-1);
             }

src/tools/miri/src/shims/tls.rs

@@ -303,12 +303,12 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             return Ok(());
         }
         let thread_callback =
-            this.eval_windows("thread_local_key", "p_thread_callback")?.to_pointer(this)?;
+            this.eval_windows("thread_local_key", "p_thread_callback").to_pointer(this)?;
         let thread_callback = this.get_ptr_fn(thread_callback)?.as_instance()?;
 
         // FIXME: Technically, the reason should be `DLL_PROCESS_DETACH` when the main thread exits
         // but std treats both the same.
-        let reason = this.eval_windows("c", "DLL_THREAD_DETACH")?;
+        let reason = this.eval_windows("c", "DLL_THREAD_DETACH");
 
         // The signature of this function is `unsafe extern "system" fn(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID)`.
         // FIXME: `h` should be a handle to the current module and what `pv` should be is unknown

src/tools/miri/src/shims/unix/foreign_items.rs

@@ -196,7 +196,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 // Align must be power of 2, and also at least ptr-sized (POSIX rules).
                 // 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")?;
+                    let einval = this.eval_libc_i32("EINVAL");
                     this.write_int(einval, dest)?;
                 } else {
                     if size == 0 {
@@ -243,7 +243,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 ];
                 let mut result = None;
                 for &(sysconf_name, value) in sysconfs {
-                    let sysconf_name = this.eval_libc_i32(sysconf_name)?;
+                    let sysconf_name = this.eval_libc_i32(sysconf_name);
                     if sysconf_name == name {
                         result = Some(value(this));
                         break;
@@ -480,7 +480,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     None => format!("<unknown errnum in strerror_r: {errnum}>"),
                 };
                 let (complete, _) = this.write_os_str_to_c_str(OsStr::new(&formatted), buf, buflen)?;
-                let ret = if complete { 0 } else { this.eval_libc_i32("ERANGE")? };
+                let ret = if complete { 0 } else { this.eval_libc_i32("ERANGE") };
                 this.write_int(ret, dest)?;
             }
             "getpid" => {
@@ -495,7 +495,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             if this.frame_in_std() => {
                 let [_attr, guard_size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let guard_size = this.deref_operand(guard_size)?;
-                let guard_size_layout = this.libc_ty_layout("size_t")?;
+                let guard_size_layout = this.libc_ty_layout("size_t");
                 this.write_scalar(Scalar::from_uint(this.machine.page_size, guard_size_layout.size), &guard_size.into())?;
 
                 // Return success (`0`).
@@ -589,7 +589,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     this.write_null(dest)?;
                 } else {
                     this.write_null(&result.into())?;
-                    this.write_scalar(this.eval_libc("ERANGE")?, dest)?;
+                    this.write_scalar(this.eval_libc("ERANGE"), dest)?;
                 }
             }