From 93c62a4912c7350924ec942aed37ff1cc363352f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 23:12:44 +0200 Subject: [PATCH 1/3] move tls.rs into shims module --- src/lib.rs | 3 +-- src/shims/mod.rs | 1 + src/{ => shims}/tls.rs | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{ => shims}/tls.rs (100%) diff --git a/src/lib.rs b/src/lib.rs index 50cf32f852..31e7070777 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,6 @@ extern crate rustc_target; mod shims; mod operator; mod helpers; -mod tls; mod range_map; mod mono_hash_map; mod stacked_borrows; @@ -31,8 +30,8 @@ pub use rustc_mir::interpret::{self, AllocMap, PlaceTy}; pub use crate::shims::{EvalContextExt as ShimsEvalContextExt}; pub use crate::shims::foreign_items::EvalContextExt as ForeignItemsEvalContextExt; pub use crate::shims::intrinsics::EvalContextExt as IntrinsicsEvalContextExt; +pub use crate::shims::tls::{EvalContextExt as TlsEvalContextExt, TlsData}; pub use crate::operator::EvalContextExt as OperatorEvalContextExt; -pub use crate::tls::{EvalContextExt as TlsEvalContextExt, TlsData}; pub use crate::range_map::RangeMap; pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt}; pub use crate::mono_hash_map::MonoHashMap; diff --git a/src/shims/mod.rs b/src/shims/mod.rs index 0fc23e8119..3258cf3d9c 100644 --- a/src/shims/mod.rs +++ b/src/shims/mod.rs @@ -1,5 +1,6 @@ pub mod foreign_items; pub mod intrinsics; +pub mod tls; use rustc::{ty, mir}; diff --git a/src/tls.rs b/src/shims/tls.rs similarity index 100% rename from src/tls.rs rename to src/shims/tls.rs From 07d5e9917cb17d8c8aa2c132919e985841ed24bc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 09:56:42 +0200 Subject: [PATCH 2/3] avoid Scalar::is_null_ptr, it is going away --- src/helpers.rs | 22 ++++++++++++++++++++++ src/shims/foreign_items.rs | 35 +++++++++++++---------------------- src/shims/tls.rs | 20 +++++++++++++------- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 16451fb872..3503af4369 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -43,6 +43,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx }) } + /// Write a 0 of the appropriate size to `dest`. + fn write_null(&mut self, dest: PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> { + self.eval_context_mut().write_scalar(Scalar::from_int(0, dest.layout.size), dest) + } + + /// Test if this immediate equals 0. + fn is_null(&self, val: Scalar) -> InterpResult<'tcx, bool> { + let this = self.eval_context_ref(); + let null = Scalar::from_int(0, this.memory().pointer_size()); + this.ptr_eq(val, null) + } + + /// Turn a Scalar into an Option + fn test_null(&self, val: Scalar) -> InterpResult<'tcx, Option>> { + let this = self.eval_context_ref(); + Ok(if this.is_null(val)? { + None + } else { + Some(val) + }) + } + /// Visits the memory covered by `place`, sensitive to freezing: the 3rd parameter /// will be true if this is frozen, false if this is in an `UnsafeCell`. fn visit_freeze_sensitive( diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index c9b10e02c9..d43374f1bc 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -49,7 +49,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ptr: Scalar, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - if !ptr.is_null_ptr(this) { + if !this.is_null(ptr)? { this.memory_mut().deallocate( ptr.to_ptr()?, None, @@ -66,7 +66,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let align = this.min_align(); - if old_ptr.is_null_ptr(this) { + if this.is_null(old_ptr)? { if new_size == 0 { Ok(Scalar::from_int(0, this.pointer_size())) } else { @@ -427,7 +427,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let mut success = None; { let name_ptr = this.read_scalar(args[0])?.not_undef()?; - if !name_ptr.is_null_ptr(this) { + if !this.is_null(name_ptr)? { let name_ptr = name_ptr.to_ptr()?; let name = this .memory() @@ -455,7 +455,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let name_ptr = this.read_scalar(args[0])?.not_undef()?; let value_ptr = this.read_scalar(args[1])?.to_ptr()?; let value = this.memory().get(value_ptr.alloc_id)?.read_c_str(tcx, value_ptr)?; - if !name_ptr.is_null_ptr(this) { + if !this.is_null(name_ptr)? { let name_ptr = name_ptr.to_ptr()?; let name = this.memory().get(name_ptr.alloc_id)?.read_c_str(tcx, name_ptr)?; if !name.is_empty() && !name.contains(&b'=') { @@ -638,14 +638,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let key_ptr = this.read_scalar(args[0])?.not_undef()?; // Extract the function type out of the signature (that seems easier than constructing it ourselves). - let dtor = match this.read_scalar(args[1])?.not_undef()? { - Scalar::Ptr(dtor_ptr) => Some(this.memory().get_fn(dtor_ptr)?), - Scalar::Raw { data: 0, size } => { - // NULL pointer - assert_eq!(size as u64, this.memory().pointer_size().bytes()); - None - }, - Scalar::Raw { .. } => return err!(ReadBytesAsPointer), + let dtor = match this.test_null(this.read_scalar(args[1])?.not_undef()?)? { + Some(dtor_ptr) => Some(this.memory().get_fn(dtor_ptr.to_ptr()?)?), + None => None, }; // Figure out how large a pthread TLS key actually is. @@ -657,7 +652,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, tcx) as u128; + 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) { return err!(OutOfTls); } @@ -682,13 +677,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } "pthread_getspecific" => { let key = this.read_scalar(args[0])?.to_bits(args[0].layout.size)?; - let ptr = this.machine.tls.load_tls(key)?; + let ptr = this.machine.tls.load_tls(key, tcx)?; this.write_scalar(ptr, dest)?; } "pthread_setspecific" => { let key = this.read_scalar(args[0])?.to_bits(args[0].layout.size)?; let new_ptr = this.read_scalar(args[1])?.not_undef()?; - this.machine.tls.store_tls(key, new_ptr)?; + this.machine.tls.store_tls(key, this.test_null(new_ptr)?)?; // Return success (`0`). this.write_null(dest)?; @@ -842,7 +837,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, tcx) as u128; + 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 @@ -853,13 +848,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } "TlsGetValue" => { let key = this.read_scalar(args[0])?.to_u32()? as u128; - let ptr = this.machine.tls.load_tls(key)?; + let ptr = this.machine.tls.load_tls(key, tcx)?; this.write_scalar(ptr, dest)?; } "TlsSetValue" => { let key = this.read_scalar(args[0])?.to_u32()? as u128; let new_ptr = this.read_scalar(args[1])?.not_undef()?; - this.machine.tls.store_tls(key, new_ptr)?; + this.machine.tls.store_tls(key, this.test_null(new_ptr)?)?; // Return success (`1`). this.write_scalar(Scalar::from_int(1, dest.layout.size), dest)?; @@ -936,10 +931,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(()) } - fn write_null(&mut self, dest: PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> { - self.eval_context_mut().write_scalar(Scalar::from_int(0, dest.layout.size), dest) - } - /// 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>> { diff --git a/src/shims/tls.rs b/src/shims/tls.rs index ddc301447c..6d6a71b8eb 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -12,7 +12,10 @@ pub type TlsKey = u128; #[derive(Copy, Clone, Debug)] pub struct TlsEntry<'tcx> { - pub(crate) data: Scalar, // Will eventually become a map from thread IDs to `Scalar`s, if we ever support more than one thread. + /// The data for this key. None is used to represent NULL. + /// (We normalize this early to avoid having to do a NULL-ptr-test each time we access the data.) + /// Will eventually become a map from thread IDs to `Scalar`s, if we ever support more than one thread. + pub(crate) data: Option>, pub(crate) dtor: Option>, } @@ -38,14 +41,13 @@ impl<'tcx> TlsData<'tcx> { pub fn create_tls_key( &mut self, dtor: Option>, - cx: &impl HasDataLayout, ) -> TlsKey { let new_key = self.next_key; self.next_key += 1; self.keys.insert( new_key, TlsEntry { - data: Scalar::ptr_null(cx).into(), + data: None, dtor, }, ); @@ -63,17 +65,21 @@ impl<'tcx> TlsData<'tcx> { } } - pub fn load_tls(&mut self, key: TlsKey) -> InterpResult<'tcx, Scalar> { + pub fn load_tls( + &mut self, + key: TlsKey, + cx: &impl HasDataLayout, + ) -> InterpResult<'tcx, Scalar> { match self.keys.get(&key) { Some(&TlsEntry { data, .. }) => { trace!("TLS key {} loaded: {:?}", key, data); - Ok(data) + Ok(data.unwrap_or_else(|| Scalar::ptr_null(cx).into())) } None => err!(TlsOutOfBounds), } } - pub fn store_tls(&mut self, key: TlsKey, new_data: Scalar) -> InterpResult<'tcx> { + pub fn store_tls(&mut self, key: TlsKey, new_data: Option>) -> InterpResult<'tcx> { match self.keys.get_mut(&key) { Some(&mut TlsEntry { ref mut data, .. }) => { trace!("TLS key {} stored: {:?}", key, new_data); @@ -117,7 +123,7 @@ impl<'tcx> TlsData<'tcx> { for (&key, &mut TlsEntry { ref mut data, dtor }) in thread_local.range_mut((start, Unbounded)) { - if !data.is_null_ptr(cx) { + if let Some(ref mut data) = *data { if let Some(dtor) = dtor { let ret = Some((dtor, *data, key)); *data = Scalar::ptr_null(cx); From 698b311a596b5d141b0c42f1b5400adc1f11150f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jul 2019 10:08:57 +0200 Subject: [PATCH 3/3] fix NULL in TLS dtors --- src/shims/tls.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/shims/tls.rs b/src/shims/tls.rs index 6d6a71b8eb..9a22c03bf2 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -6,6 +6,7 @@ use rustc::{ty, ty::layout::HasDataLayout, mir}; use crate::{ InterpResult, InterpError, StackPopCleanup, MPlaceTy, Scalar, Tag, + HelpersEvalContextExt, }; pub type TlsKey = u128; @@ -111,7 +112,6 @@ impl<'tcx> TlsData<'tcx> { fn fetch_tls_dtor( &mut self, key: Option, - cx: &impl HasDataLayout, ) -> Option<(ty::Instance<'tcx>, Scalar, TlsKey)> { use std::collections::Bound::*; @@ -123,10 +123,10 @@ impl<'tcx> TlsData<'tcx> { for (&key, &mut TlsEntry { ref mut data, dtor }) in thread_local.range_mut((start, Unbounded)) { - if let Some(ref mut data) = *data { + if let Some(data_scalar) = *data { if let Some(dtor) = dtor { - let ret = Some((dtor, *data, key)); - *data = Scalar::ptr_null(cx); + let ret = Some((dtor, data_scalar, key)); + *data = None; return ret; } } @@ -139,10 +139,11 @@ impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tc pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { fn run_tls_dtors(&mut self) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let mut dtor = this.machine.tls.fetch_tls_dtor(None, &*this.tcx); + let mut dtor = this.machine.tls.fetch_tls_dtor(None); // FIXME: replace loop by some structure that works with stepping while let Some((instance, ptr, key)) = dtor { trace!("Running TLS dtor {:?} on {:?}", instance, ptr); + assert!(!this.is_null(ptr).unwrap(), "Data can't be NULL when dtor is called!"); // TODO: Potentially, this has to support all the other possible instances? // See eval_fn_call in interpret/terminator/mod.rs let mir = this.load_mir(instance.def)?; @@ -163,9 +164,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // step until out of stackframes this.run()?; - dtor = match this.machine.tls.fetch_tls_dtor(Some(key), &*this.tcx) { + dtor = match this.machine.tls.fetch_tls_dtor(Some(key)) { dtor @ Some(_) => dtor, - None => this.machine.tls.fetch_tls_dtor(None, &*this.tcx), + None => this.machine.tls.fetch_tls_dtor(None), }; } // FIXME: On a windows target, call `unsafe extern "system" fn on_tls_callback`.