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

fix Miri offset_from #66083

Merged
merged 5 commits into from
Nov 5, 2019
Merged
Show file tree
Hide file tree
Changes from 4 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
23 changes: 20 additions & 3 deletions src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,26 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

"ptr_offset_from" => {
let a = self.read_immediate(args[0])?.to_scalar()?.to_ptr()?;
let b = self.read_immediate(args[1])?.to_scalar()?.to_ptr()?;
let isize_layout = self.layout_of(self.tcx.types.isize)?;
let a = self.read_immediate(args[0])?.to_scalar()?;
let b = self.read_immediate(args[1])?.to_scalar()?;

// Special case: if both scalars are *equal integers*
// and not NULL, their offset is 0.
Copy link
Contributor

Choose a reason for hiding this comment

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

why is the null case important? The docs state nothing of the kind. Or is the "valid object" thing the problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, since wrapping_offset_from is safe, and you can supply null pointers to it... I guess this should be allowed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed this is about the "valid object" part. There's never a valid object at 0, not even one of size 0, so 0 is never a valid input here.

wrapping_offset_from does not have the "valid object" requirement.

// This is the dual to the special exception for offset-by-0
// in the inbounds pointer offset operation.
if a.is_bits() && b.is_bits() {
let a = a.to_usize(self)?;
let b = b.to_usize(self)?;
if a == b && a != 0 {
self.write_scalar(Scalar::from_int(0, isize_layout.size), dest)?;
return Ok(true);
}
}

// General case: we need two pointers.
let a = self.force_ptr(a)?;
let b = self.force_ptr(b)?;
if a.alloc_id != b.alloc_id {
throw_ub_format!(
"ptr_offset_from cannot compute offset of pointers into different \
Expand All @@ -266,7 +284,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
BinOp::Sub, a_offset, b_offset,
)?;
let pointee_layout = self.layout_of(substs.type_at(0))?;
let isize_layout = self.layout_of(self.tcx.types.isize)?;
let val = ImmTy::from_scalar(val, isize_layout);
let size = ImmTy::from_int(pointee_layout.size.bytes(), isize_layout);
self.exact_div(val, size, dest)?;
Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/consts/offset_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,14 @@ pub const OVERFLOW: isize = {
unsafe { (base_ptr as *const u8).offset_from(field_ptr) }
};

pub const OFFSET_EQUAL_INTS: isize = {
let ptr = 1 as *const u8;
unsafe { ptr.offset_from(ptr) }
};

fn main() {
assert_eq!(OFFSET, 0);
assert_eq!(OFFSET_2, 1);
assert_eq!(OVERFLOW, -1);
assert_eq!(OFFSET_EQUAL_INTS, 0);
}
18 changes: 15 additions & 3 deletions src/test/ui/consts/offset_from_ub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,25 @@ pub const NOT_PTR: usize = {
unsafe { (42 as *const u8).offset_from(&5u8) as usize }
};

pub const NOT_MULTIPLE_OF_SIZE: usize = {
pub const NOT_MULTIPLE_OF_SIZE: isize = {
//~^ NOTE
let data = [5u8, 6, 7];
let base_ptr = data.as_ptr();
let field_ptr = &data[1] as *const u8 as *const u16;
let offset = unsafe { field_ptr.offset_from(base_ptr as *const u16) };
offset as usize
unsafe { field_ptr.offset_from(base_ptr as *const u16) }
};

pub const OFFSET_FROM_NULL: isize = {
//~^ NOTE
let ptr = 0 as *const u8;
unsafe { ptr.offset_from(ptr) }
};

pub const DIFFERENT_INT: isize = { // offset_from with two different integers: like DIFFERENT_ALLOC
//~^ NOTE
let ptr1 = 8 as *const u8;
let ptr2 = 16 as *const u8;
unsafe { ptr2.offset_from(ptr1) }
};

fn main() {}
47 changes: 42 additions & 5 deletions src/test/ui/consts/offset_from_ub.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,55 @@ LL | intrinsics::ptr_offset_from(self, origin)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| exact_div: 1 cannot be divided by 2 without remainder
| inside call to `std::ptr::<impl *const u16>::offset_from` at $DIR/offset_from_ub.rs:33:27
| inside call to `std::ptr::<impl *const u16>::offset_from` at $DIR/offset_from_ub.rs:33:14
|
::: $DIR/offset_from_ub.rs:28:1
|
LL | / pub const NOT_MULTIPLE_OF_SIZE: usize = {
LL | / pub const NOT_MULTIPLE_OF_SIZE: isize = {
LL | |
LL | | let data = [5u8, 6, 7];
LL | | let base_ptr = data.as_ptr();
... |
LL | | offset as usize
LL | | let field_ptr = &data[1] as *const u8 as *const u16;
LL | | unsafe { field_ptr.offset_from(base_ptr as *const u16) }
LL | | };
| |__-

error: any use of this value will cause an error
--> $SRC_DIR/libcore/ptr/mod.rs:LL:COL
|
LL | intrinsics::ptr_offset_from(self, origin)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| invalid use of NULL pointer
| inside call to `std::ptr::<impl *const u8>::offset_from` at $DIR/offset_from_ub.rs:39:14
|
::: $DIR/offset_from_ub.rs:36:1
|
LL | / pub const OFFSET_FROM_NULL: isize = {
LL | |
LL | | let ptr = 0 as *const u8;
LL | | unsafe { ptr.offset_from(ptr) }
LL | | };
| |__-

error: any use of this value will cause an error
--> $SRC_DIR/libcore/ptr/mod.rs:LL:COL
|
LL | intrinsics::ptr_offset_from(self, origin)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| a memory access tried to interpret some bytes as a pointer
| inside call to `std::ptr::<impl *const u8>::offset_from` at $DIR/offset_from_ub.rs:46:14
|
::: $DIR/offset_from_ub.rs:42:1
|
LL | / pub const DIFFERENT_INT: isize = { // offset_from with two different integers: like DIFFERENT_ALLOC
LL | |
LL | | let ptr1 = 8 as *const u8;
LL | | let ptr2 = 16 as *const u8;
LL | | unsafe { ptr2.offset_from(ptr1) }
LL | | };
| |__-

error: aborting due to 3 previous errors
error: aborting due to 5 previous errors