Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid Scalar::is_null_ptr, it is going away #825

Merged
merged 2 commits into from
Jul 5, 2019
Merged

avoid Scalar::is_null_ptr, it is going away #825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07d5e99
avoid Scalar::is_null_ptr, it is going away
RalfJung Jul 5, 2019
698b311
fix NULL in TLS dtors
RalfJung Jul 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Tag>) -> 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<NonNullScalar>
fn test_null(&self, val: Scalar<Tag>) -> InterpResult<'tcx, Option<Scalar<Tag>>> {
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(
Expand Down
35 changes: 13 additions & 22 deletions src/shims/foreign_items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
ptr: Scalar<Tag>,
) -> 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,
Expand All @@ -66,7 +66,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
) -> InterpResult<'tcx, Scalar<Tag>> {
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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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'=') {
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
Expand All @@ -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)?;
Expand Down Expand Up @@ -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
Expand All @@ -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)?;
Expand Down Expand Up @@ -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<ScalarMaybeUndef<Tag>>> {
Expand Down
33 changes: 20 additions & 13 deletions src/shims/tls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@ use rustc::{ty, ty::layout::HasDataLayout, mir};
use crate::{
InterpResult, InterpError, StackPopCleanup,
MPlaceTy, Scalar, Tag,
HelpersEvalContextExt,
};

pub type TlsKey = u128;

#[derive(Copy, Clone, Debug)]
pub struct TlsEntry<'tcx> {
pub(crate) data: Scalar<Tag>, // 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<Scalar<Tag>>,
pub(crate) dtor: Option<ty::Instance<'tcx>>,
}

Expand All @@ -38,14 +42,13 @@ impl<'tcx> TlsData<'tcx> {
pub fn create_tls_key(
&mut self,
dtor: Option<ty::Instance<'tcx>>,
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,
},
);
Expand All @@ -63,17 +66,21 @@ impl<'tcx> TlsData<'tcx> {
}
}

pub fn load_tls(&mut self, key: TlsKey) -> InterpResult<'tcx, Scalar<Tag>> {
pub fn load_tls(
&mut self,
key: TlsKey,
cx: &impl HasDataLayout,
) -> InterpResult<'tcx, Scalar<Tag>> {
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<Tag>) -> InterpResult<'tcx> {
pub fn store_tls(&mut self, key: TlsKey, new_data: Option<Scalar<Tag>>) -> InterpResult<'tcx> {
match self.keys.get_mut(&key) {
Some(&mut TlsEntry { ref mut data, .. }) => {
trace!("TLS key {} stored: {:?}", key, new_data);
Expand Down Expand Up @@ -105,7 +112,6 @@ impl<'tcx> TlsData<'tcx> {
fn fetch_tls_dtor(
&mut self,
key: Option<TlsKey>,
cx: &impl HasDataLayout,
) -> Option<(ty::Instance<'tcx>, Scalar<Tag>, TlsKey)> {
use std::collections::Bound::*;

Expand All @@ -117,10 +123,10 @@ 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(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;
}
}
Expand All @@ -133,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)?;
Expand All @@ -157,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`.
Expand Down