Skip to content

Move last error into memory #977

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

Merged
merged 4 commits into from
Oct 8, 2019
Merged

Move last error into memory #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
905c70c
Rustfmt
pvdrz Oct 7, 2019
11d7be9
Move last_error into memory
pvdrz Oct 3, 2019
887d748
Add __errno_location shim
pvdrz Oct 3, 2019
459c65a
Add method to consume io::Error
pvdrz Oct 7, 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
71 changes: 44 additions & 27 deletions src/eval.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@
use rand::rngs::StdRng;
use rand::SeedableRng;

use syntax::source_map::DUMMY_SP;
use rustc::ty::{self, TyCtxt};
use rustc::ty::layout::{LayoutOf, Size, Align};
use rustc::hir::def_id::DefId;
use rustc::ty::layout::{Align, LayoutOf, Size};
use rustc::ty::{self, TyCtxt};
use syntax::source_map::DUMMY_SP;

use crate::{
InterpResult, InterpError, InterpCx, StackPopCleanup, struct_error,
Scalar, Tag, Pointer, FnVal,
MemoryExtra, MiriMemoryKind, Evaluator, TlsEvalContextExt, HelpersEvalContextExt,
EnvVars,
struct_error, EnvVars, Evaluator, FnVal, HelpersEvalContextExt, InterpCx, InterpError,
InterpResult, MemoryExtra, MiriMemoryKind, Pointer, Scalar, StackPopCleanup, Tag,
TlsEvalContextExt,
};

/// Configuration needed to spawn a Miri instance.
Expand Down Expand Up @@ -40,7 +39,10 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
tcx.at(syntax::source_map::DUMMY_SP),
ty::ParamEnv::reveal_all(),
Evaluator::new(config.communicate),
MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate),
MemoryExtra::new(
StdRng::seed_from_u64(config.seed.unwrap_or(0)),
config.validate,
),
);
// Complete initialization.
EnvVars::init(&mut ecx, config.excluded_env_vars);
Expand All @@ -50,9 +52,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
let main_mir = ecx.load_mir(main_instance.def, None)?;

if !main_mir.return_ty().is_unit() || main_mir.arg_count != 0 {
throw_unsup_format!(
"miri does not support main functions without `fn()` type signatures"
);
throw_unsup_format!("miri does not support main functions without `fn()` type signatures");
}

let start_id = tcx.lang_items().start_fn().unwrap();
Expand All @@ -62,9 +62,10 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
ecx.tcx.tcx,
ty::ParamEnv::reveal_all(),
start_id,
ecx.tcx.mk_substs(
::std::iter::once(ty::subst::GenericArg::from(main_ret_ty)))
).unwrap();
ecx.tcx
.mk_substs(::std::iter::once(ty::subst::GenericArg::from(main_ret_ty))),
)
.unwrap();
let start_mir = ecx.load_mir(start_instance.def, None)?;

if start_mir.arg_count != 3 {
Expand All @@ -91,7 +92,9 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
let mut args = ecx.frame().body.args_iter();

// First argument: pointer to `main()`.
let main_ptr = ecx.memory_mut().create_fn_alloc(FnVal::Instance(main_instance));
let main_ptr = ecx
.memory_mut()
.create_fn_alloc(FnVal::Instance(main_instance));
let dest = ecx.local_place(args.next().unwrap())?;
ecx.write_scalar(Scalar::Ptr(main_ptr), dest)?;

Expand Down Expand Up @@ -124,16 +127,23 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
// Add `0` terminator.
let mut arg = arg.into_bytes();
arg.push(0);
argvs.push(ecx.memory_mut().allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Static.into()));
argvs.push(
ecx.memory_mut()
.allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Static.into()),
);
}
// Make an array with all these pointers, in the Miri memory.
let argvs_layout = ecx.layout_of(ecx.tcx.mk_array(ecx.tcx.mk_imm_ptr(ecx.tcx.types.u8), argvs.len() as u64))?;
let argvs_layout = ecx.layout_of(
ecx.tcx
.mk_array(ecx.tcx.mk_imm_ptr(ecx.tcx.types.u8), argvs.len() as u64),
)?;
let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Env.into());
for (idx, arg) in argvs.into_iter().enumerate() {
let place = ecx.mplace_field(argvs_place, idx as u64)?;
ecx.write_scalar(Scalar::Ptr(arg), place.into())?;
}
ecx.memory_mut().mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
ecx.memory_mut()
.mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
// Write a pointer to that place as the argument.
let argv = argvs_place.ptr;
ecx.write_scalar(argv, dest)?;
Expand All @@ -145,7 +155,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
}
// Store command line as UTF-16 for Windows `GetCommandLineW`.
{
let tcx = &{ecx.tcx.tcx};
let tcx = &{ ecx.tcx.tcx };
let cmd_utf16: Vec<u16> = cmd.encode_utf16().collect();
let cmd_ptr = ecx.memory_mut().allocate(
Size::from_bytes(cmd_utf16.len() as u64 * 2),
Expand All @@ -168,16 +178,22 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
}
}

assert!(args.next().is_none(), "start lang item has more arguments than expected");
assert!(
args.next().is_none(),
"start lang item has more arguments than expected"
);

// Set the last_error to 0
let errno_layout = ecx.layout_of(ecx.tcx.types.u32)?;
let errno_place = ecx.allocate(errno_layout, MiriMemoryKind::Static.into());
ecx.write_scalar(Scalar::from_u32(0), errno_place.into())?;
let errno_ptr = ecx.check_mplace_access(errno_place.into(), Some(Size::from_bits(32)))?;
ecx.machine.last_error = errno_ptr;

Ok(ecx)
}

pub fn eval_main<'tcx>(
tcx: TyCtxt<'tcx>,
main_id: DefId,
config: MiriConfig,
) {
pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) {
let mut ecx = match create_ecx(tcx, main_id, config) {
Ok(ecx) => ecx,
Err(mut err) => {
Expand Down Expand Up @@ -228,8 +244,9 @@ pub fn eval_main<'tcx>(
// We iterate with indices because we need to look at the next frame (the caller).
for idx in 0..frames.len() {
let frame_info = &frames[idx];
let call_site_is_local = frames.get(idx+1).map_or(false,
|caller_info| caller_info.instance.def_id().is_local());
let call_site_is_local = frames.get(idx + 1).map_or(false, |caller_info| {
caller_info.instance.def_id().is_local()
});
if call_site_is_local {
err.span_note(frame_info.call_site, &frame_info.to_string());
} else {
Expand Down
71 changes: 45 additions & 26 deletions src/machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
//! Global machine state as well as implementation of the interpreter engine
//! `Machine` trait.

use std::rc::Rc;
use std::borrow::Cow;
use std::cell::RefCell;
use std::rc::Rc;

use rand::rngs::StdRng;

use syntax::attr;
use syntax::symbol::sym;
use rustc::hir::def_id::DefId;
use rustc::ty::{self, Ty, TyCtxt, layout::{Size, LayoutOf}};
use rustc::mir;
use rustc::ty::{
self,
layout::{LayoutOf, Size},
Ty, TyCtxt,
};
use syntax::attr;
use syntax::symbol::sym;

use crate::*;

// Some global facts about the emulated machine.
pub const PAGE_SIZE: u64 = 4*1024; // FIXME: adjust to target architecture
pub const STACK_ADDR: u64 = 32*PAGE_SIZE; // not really about the "stack", but where we start assigning integer addresses to allocations
pub const STACK_SIZE: u64 = 16*PAGE_SIZE; // whatever
pub const PAGE_SIZE: u64 = 4 * 1024; // FIXME: adjust to target architecture
pub const STACK_ADDR: u64 = 32 * PAGE_SIZE; // not really about the "stack", but where we start assigning integer addresses to allocations
pub const STACK_SIZE: u64 = 16 * PAGE_SIZE; // whatever
pub const NUM_CPUS: u64 = 1;

/// Extra memory kinds
Expand Down Expand Up @@ -88,7 +92,7 @@ pub struct Evaluator<'tcx> {
pub(crate) cmd_line: Option<Pointer<Tag>>,

/// Last OS error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that this is a memory location, the comment should say e.g. how big it is (seems to always be 4 bytes?). Also, this was previously used for Windows stuff, errno_location is a Linux thing; can the two really share the same field?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a comment about its size. It should work for windows in principle because windows uses an u32 and linux uses an i32.

I think only disadvantage of adding this is that we have the indirection of having to get the location pointer and then get the pointeé. But it shouldn't affect the capablities of setting and getting the error in either of the two platforms.

pub(crate) last_error: u32,
pub(crate) last_error: Option<Pointer<Tag>>,

/// TLS state.
pub(crate) tls: TlsData<'tcx>,
Expand All @@ -109,7 +113,7 @@ impl<'tcx> Evaluator<'tcx> {
argc: None,
argv: None,
cmd_line: None,
last_error: 0,
last_error: None,
tls: TlsData::default(),
communicate,
file_handler: Default::default(),
Expand Down Expand Up @@ -146,7 +150,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
type PointerTag = Tag;
type ExtraFnVal = Dlsym;

type MemoryMap = MonoHashMap<AllocId, (MemoryKind<MiriMemoryKind>, Allocation<Tag, Self::AllocExtra>)>;
type MemoryMap = MonoHashMap<
AllocId,
(
MemoryKind<MiriMemoryKind>,
Allocation<Tag, Self::AllocExtra>,
),
>;

const STATIC_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::Static);

Expand Down Expand Up @@ -264,8 +274,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
}

#[inline(always)]
fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx>
{
fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
// We are not interested in detecting loops.
Ok(())
}
Expand All @@ -275,7 +284,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
id: AllocId,
alloc: Cow<'b, Allocation>,
kind: Option<MemoryKind<Self::MemoryKinds>>,
) -> (Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>, Self::PointerTag) {
) -> (
Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>,
Self::PointerTag,
) {
let kind = kind.expect("we set our STATIC_KIND so this cannot be None");
let alloc = alloc.into_owned();
let (stacks, base_tag) = if !memory_extra.validate {
Expand All @@ -291,12 +303,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
};
let mut stacked_borrows = memory_extra.stacked_borrows.borrow_mut();
let alloc: Allocation<Tag, Self::AllocExtra> = alloc.with_tags_and_extra(
|alloc| if !memory_extra.validate {
Tag::Untagged
} else {
// Only statics may already contain pointers at this point
assert_eq!(kind, MiriMemoryKind::Static.into());
stacked_borrows.static_base_ptr(alloc)
|alloc| {
if !memory_extra.validate {
Tag::Untagged
} else {
// Only statics may already contain pointers at this point
assert_eq!(kind, MiriMemoryKind::Static.into());
stacked_borrows.static_base_ptr(alloc)
}
},
AllocExtra {
stacked_borrows: stacks,
Expand All @@ -306,14 +320,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
}

#[inline(always)]
fn tag_static_base_pointer(
memory_extra: &MemoryExtra,
id: AllocId,
) -> Self::PointerTag {
fn tag_static_base_pointer(memory_extra: &MemoryExtra, id: AllocId) -> Self::PointerTag {
if !memory_extra.validate {
Tag::Untagged
} else {
memory_extra.stacked_borrows.borrow_mut().static_base_ptr(id)
memory_extra
.stacked_borrows
.borrow_mut()
.static_base_ptr(id)
}
}

Expand All @@ -325,7 +339,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
) -> InterpResult<'tcx> {
if !Self::enforce_validity(ecx) {
// No tracking.
Ok(())
Ok(())
} else {
ecx.retag(kind, place)
}
Expand All @@ -343,7 +357,12 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
ecx: &mut InterpCx<'mir, 'tcx, Self>,
extra: stacked_borrows::CallId,
) -> InterpResult<'tcx> {
Ok(ecx.memory().extra.stacked_borrows.borrow_mut().end_call(extra))
Ok(ecx
.memory()
.extra
.stacked_borrows
.borrow_mut()
.end_call(extra))
}

#[inline(always)]
Expand Down
10 changes: 4 additions & 6 deletions src/shims/env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
.write_bytes(tcx, buf, &bytes)?;
return Ok(Scalar::Ptr(buf));
}
this.machine.last_error = this
.eval_path_scalar(&["libc", "ERANGE"])?
.unwrap()
.to_u32()?;
let erange = this.eval_libc("ERANGE")?;
this.set_last_error(erange)?;
}
Err(e) => this.machine.last_error = e.raw_os_error().unwrap() as u32,
Err(e) => this.consume_io_error(e)?,
}
Ok(Scalar::ptr_null(&*this.tcx))
}
Expand All @@ -172,7 +170,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
match env::set_current_dir(path) {
Ok(()) => Ok(0),
Err(e) => {
this.machine.last_error = e.raw_os_error().unwrap() as u32;
this.consume_io_error(e)?;
Ok(-1)
}
}
Expand Down
Loading