handle rewrite

Author: beepster4096

src/shims/windows/foreign_items.rs

@@ -9,7 +9,7 @@ use rustc_target::spec::abi::Abi;
 use crate::thread::Time;
 use crate::*;
 use shims::foreign_items::EmulateByNameResult;
-use shims::windows::handle::Handle;
+use shims::windows::handle::{EvalContextExt as _, Handle};
 use shims::windows::sync::EvalContextExt as _;
 use shims::windows::thread::EvalContextExt as _;
 
@@ -393,10 +393,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let [handle] =
                     this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
 
-                match Handle::from_scalar(this.read_scalar(handle)?.check_init()?, this)? {
-                    Some(Handle::Thread(thread)) => this.detach_thread(thread)?,
-                    _ => throw_ub_format!("invalid handle"),
-                };
+                this.CloseHandle(handle)?;
 
                 this.write_scalar(Scalar::from_u32(1), dest)?;
             }

src/shims/windows/handle.rs

@@ -12,107 +12,151 @@ enum RealHandle {
 }
 
 impl RealHandle {
-    fn discriminant(self) -> u64 {
+    const USABLE_BITS: u32 = 31;
+
+    const THREAD_DISCRIMINANT: u32 = 1;
+
+    fn discriminant(self) -> u32 {
         match self {
             // can't use zero here because all zero handle is invalid
-            Self::Thread(_) => 1,
+            Self::Thread(_) => Self::THREAD_DISCRIMINANT,
         }
     }
 
-    fn data(self) -> u64 {
+    fn data(self) -> u32 {
         match self {
-            Self::Thread(thread) => thread.to_u32() as u64,
+            Self::Thread(thread) => thread.to_u32(),
         }
     }
 
-    fn packed_disc_size() -> u64 {
-        (variant_count::<Self>().log2() + 1) as u64
+    fn packed_disc_size() -> u32 {
+        // log2(x) + 1 is how many bits it takes to store x
+        // because the discriminants start at 1, the variant count is equal to the highest discriminant
+        variant_count::<Self>().log2() + 1
     }
 
-    fn to_packed(self, bits: u64) -> u64 {
-        // top bit and lower 2 bits need to be clear
-        let usable_bits = (bits - 3).min(32);
-
+    /// This function packs the discriminant and data values into a 31-bit space.
+    /// None of this layout is guaranteed to applications by Windows or Miri.
+    /// The sign bit is not used to avoid overlapping any pseudo-handles.
+    fn to_packed(self) -> i32 {
         let disc_size = Self::packed_disc_size();
-        let data_size = usable_bits - disc_size;
+        let data_size = Self::USABLE_BITS - disc_size;
 
         let discriminant = self.discriminant();
         let data = self.data();
 
-        assert!(discriminant < 2u64.pow(disc_size as u32));
-        assert!(data < 2u64.pow(data_size as u32));
+        // make sure the discriminant fits into `disc_size` bits
+        assert!(discriminant < 2u32.pow(disc_size));
 
-        (discriminant << data_size | data) << 2
+        // make sure the data fits into `data_size` bits
+        assert!(data < 2u32.pow(data_size));
+
+        // packs the data into the lower `data_size` bits
+        // and packs the discriminant right above the data
+        (discriminant << data_size | data) as i32
     }
 
     fn new(discriminant: u32, data: u32) -> Option<Self> {
         match discriminant {
-            1 => Some(Self::Thread(data.into())),
+            Self::THREAD_DISCRIMINANT => Some(Self::Thread(data.into())),
             _ => None,
         }
     }
 
-    fn from_packed(handle: u64, bits: u64) -> Option<Self> {
-        let usable_bits = (bits - 3).min(32);
+    /// see docs for `to_packed`
+    fn from_packed(handle: i32) -> Option<Self> {
+        let handle_bits = handle as u32;
 
         let disc_size = Self::packed_disc_size();
-        let data_size = usable_bits - disc_size;
-        let data_mask = 2u64.pow(data_size as u32) - 1;
+        let data_size = Self::USABLE_BITS - disc_size;
+
+        // the lower `data_size` bits of this mask are 1
+        let data_mask = 2u32.pow(data_size) - 1;
 
-        let discriminant = handle >> data_size >> 2;
-        let data = handle >> 2 & data_mask;
+        // the discriminant is stored right above the lower `data_size` bits
+        let discriminant = handle_bits >> data_size;
 
-        Self::new(discriminant.try_into().unwrap(), data.try_into().unwrap())
+        // the data is stored in the lower `data_size` bits
+        let data = handle_bits & data_mask;
+
+        Self::new(discriminant, data)
     }
 }
 
 /// Miri representation of a Windows `HANDLE`
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub enum Handle {
     Null, // = 0
+
     // pseudo-handles
-    CurrentThread, // = -2
+    // The lowest pseudo-handle is -6, so miri pseduo-handles start at -7 to break code hardcoding these values
+    CurrentThread, // = -7
+
     // real handles
     Thread(ThreadId),
 }
 
 impl Handle {
-    fn to_packed(self, bits: u64) -> i64 {
+    const CURRENT_THREAD_VALUE: i32 = -7;
+
+    fn to_packed(self) -> i32 {
         match self {
             Self::Null => 0,
-            Self::CurrentThread => -2,
-            Self::Thread(thread) => RealHandle::Thread(thread).to_packed(bits) as i64,
+            Self::CurrentThread => Self::CURRENT_THREAD_VALUE,
+            Self::Thread(thread) => RealHandle::Thread(thread).to_packed(),
         }
     }
 
     pub fn to_scalar(self, cx: &impl HasDataLayout) -> Scalar<Tag> {
-        let bits = cx.data_layout().pointer_size.bits();
-
-        let handle = self.to_packed(bits);
+        // 64-bit handles are sign extended 32-bit handles
+        // see https://docs.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication
+        let handle = self.to_packed().into();
 
         Scalar::from_machine_isize(handle, cx)
     }
 
-    fn from_packed(handle: i64, bits: u64) -> Option<Self> {
+    fn from_packed(handle: i64) -> Option<Self> {
+        let current_thread_val = Self::CURRENT_THREAD_VALUE as i64;
+
         if handle == 0 {
             Some(Self::Null)
-        } else if handle == -2 {
+        } else if handle == current_thread_val {
             Some(Self::CurrentThread)
-        } else {
-            match RealHandle::from_packed(handle as u64, bits)? {
+        } else if let Ok(handle) = handle.try_into() {
+            match RealHandle::from_packed(handle)? {
                 RealHandle::Thread(id) => Some(Self::Thread(id)),
             }
+        } else {
+            // if a handle doesn't fit in an i32, it isn't valid.
+            None
         }
     }
 
     pub fn from_scalar<'tcx>(
         handle: Scalar<Tag>,
         cx: &impl HasDataLayout,
     ) -> InterpResult<'tcx, Option<Self>> {
-        let bits = cx.data_layout().pointer_size.bits();
-
         let handle = handle.to_machine_isize(cx)?;
 
-        Ok(Self::from_packed(handle, bits))
+        Ok(Self::from_packed(handle))
+    }
+}
+
+impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
+
+#[allow(non_snake_case)]
+pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
+    fn CloseHandle(&mut self, handle_op: &OpTy<'tcx, Tag>) -> InterpResult<'tcx> {
+        let this = self.eval_context_mut();
+
+        match Handle::from_scalar(this.read_scalar(handle_op)?.check_init()?, this)? {
+            Some(Handle::Thread(thread)) => this.detach_thread(thread)?,
+            _ =>
+                throw_machine_stop!(TerminationInfo::Abort(
+                    "invalid handle passed to `CloseHandle`".into()
+                )),
+        };
+
+        Ok(())
     }
 }