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.

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

rustup for error reform #1219

Merged
merged 6 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
660326e9791d5caf3186b14521498c2584a494ab
57e1da59cd0761330b4ea8d47b16340a78eeafa9
80 changes: 44 additions & 36 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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`.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
)
}
Expand Down Expand Up @@ -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
)
};
Expand Down Expand Up @@ -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())
})?)
}

Expand All @@ -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())
})?)
}
8 changes: 2 additions & 6 deletions src/intptrcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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)
Expand All @@ -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))
}
}
})
Expand Down
2 changes: 1 addition & 1 deletion src/shims/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
}
}
Expand Down
55 changes: 16 additions & 39 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?));
}
_ => {}
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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(())
}
}
36 changes: 13 additions & 23 deletions src/shims/foreign_items/posix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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!(
Expand All @@ -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`).
Expand Down Expand Up @@ -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)
}
}

Expand Down
6 changes: 2 additions & 4 deletions src/shims/foreign_items/posix/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)? {
Expand Down
9 changes: 1 addition & 8 deletions src/shims/foreign_items/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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" => {
Expand Down
Loading