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

ensure that integers cast to pointers will never point at a valid alloc, not even the zst alloc #81

Merged
merged 19 commits into from
Nov 17, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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 src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<'tcx> Error for EvalError<'tcx> {
EvalError::DanglingPointerDeref =>
"dangling pointer was dereferenced",
EvalError::InvalidFunctionPointer =>
"tried to use a pointer as a function pointer",
"tried to use an integer pointer as a function pointer",
Copy link
Member

Choose a reason for hiding this comment

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

Why doesn't this apply to general pointers casted to function pointers any more? The name InvalidFunctionPointer seems too general for the new error message.

Copy link
Member

Choose a reason for hiding this comment

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

@oli-obk pinging to make sure you don't miss this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing changed here, the error message was outdated way before this pr, we can adjust the variant name to match that

Copy link
Member

Choose a reason for hiding this comment

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

What kind of error do we actually get when treating a regular pointer as a function pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

    let b = Box::new(42);
    let g = unsafe {
        std::mem::transmute::<&usize, &fn(i32)>(&b)
    };

    (*g)(42) //~ ERROR tried to use an integer pointer or a dangling pointer as a function pointer

We can't do transmute(42) because primvals panic in some cases right now. I think we should eliminate the expect functions and have the try functions return an EvalResult instead.

  10:     0x55e4c4ab7a18 - <core::option::Option<T>>::expect::hda65b98614e3f1e8
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/libcore/option.rs:293
  11:     0x55e4c4bf53c3 - miri::primval::PrimVal::expect_fn_ptr::h87524394959696a3
                        at /home/ws/ca8159/Projects/rust/miri/src/primval.rs:198
  12:     0x55e4c4ba9ae7 - miri::interpreter::terminator::<impl miri::interpreter::EvalContext<'a, 'tcx>>::eval_terminator::h1e9da0fab75dcb3c
                        at /home/ws/ca8159/Projects/rust/miri/src/interpreter/terminator/mod.rs:88
  13:     0x55e4c4b960c5 - miri::interpreter::step::<impl miri::interpreter::EvalContext<'a, 'tcx>>::terminator::h0a43b45ccf9ebe32
                        at /home/ws/ca8159/Projects/rust/miri/src/interpreter/step.rs:98
  14:     0x55e4c4b95659 - miri::interpreter::step::<impl miri::interpreter::EvalContext<'a, 'tcx>>::step::ha153e9fdef721ba9
                        at /home/ws/ca8159/Projects/rust/miri/src/interpreter/step.rs:69
  15:     0x55e4c4be5fb8 - miri::interpreter::eval_main::h502fc7147a3a0d58
                        at /home/ws/ca8159/Projects/rust/miri/src/interpreter/mod.rs:1664
  16:     0x55e4c4a8c843 - <miri::MiriCompilerCalls as rustc_driver::CompilerCalls<'a>>::build_controller::{{closure}}::h01b87c9edeea1ee3
                        at /home/ws/ca8159/Projects/rust/miri/src/bin/miri.rs:72

EvalError::InvalidBool =>
"invalid boolean value read",
EvalError::InvalidDiscriminant =>
Expand Down
2 changes: 1 addition & 1 deletion src/interpreter/terminator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
};
let drop_fn = self.memory.read_ptr(vtable)?;
// some values don't need to call a drop impl, so the value is null
if !drop_fn.points_to_zst() {
if drop_fn != Pointer::from_int(0) {
let (def_id, substs, ty) = self.memory.get_fn(drop_fn.alloc_id)?;
let fn_sig = self.tcx.erase_late_bound_regions_and_normalize(&ty.sig);
let real_ty = fn_sig.inputs[0];
Expand Down
6 changes: 3 additions & 3 deletions src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl Pointer {
// FIXME(solson): Integer pointers should use u64, not usize. Target pointers can be larger
// than host usize.
pub fn from_int(i: usize) -> Self {
Pointer::new(ZST_ALLOC_ID, i)
Pointer::new(NEVER_ALLOC_ID, i)
Copy link
Contributor Author

@oli-obk oli-obk Nov 10, 2016

Choose a reason for hiding this comment

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

so I did this change to fix the assume bug, which then caused drop impls to fail: https://github.com/solson/miri/pull/81/files#diff-8f4a840e817b018e1d74153888641b27L642 and int to pointer cast error messages to show up as dangling pointers

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine. I might have a plan to refactor Pointer::from_int out of existence, but I haven't taken a close enough look yet.

}

pub fn zst_ptr() -> Self {
Expand Down Expand Up @@ -290,7 +290,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
Some(alloc) => Ok(alloc),
None => match self.functions.get(&id) {
Some(_) => Err(EvalError::DerefFunctionPointer),
None if id == ZST_ALLOC_ID => Err(EvalError::InvalidMemoryAccess),
None if id == NEVER_ALLOC_ID || id == ZST_ALLOC_ID => Err(EvalError::InvalidMemoryAccess),
None => Err(EvalError::DanglingPointerDeref),
}
}
Expand All @@ -302,7 +302,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
Some(alloc) => Ok(alloc),
None => match self.functions.get(&id) {
Some(_) => Err(EvalError::DerefFunctionPointer),
None if id == ZST_ALLOC_ID => Err(EvalError::InvalidMemoryAccess),
None if id == NEVER_ALLOC_ID || id == ZST_ALLOC_ID => Err(EvalError::InvalidMemoryAccess),
None => Err(EvalError::DanglingPointerDeref),
}
}
Expand Down
3 changes: 3 additions & 0 deletions tests/run-pass/assume_bug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
vec![()].into_iter();
}