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

Check that offset is not too big, check projection offset to be inbounds #447

Closed
RalfJung opened this issue Aug 30, 2018 · 3 comments · Fixed by rust-lang/rust#63075 or #863
Closed
Labels
A-validation Area: This affects enforcing the validity invariant, and related UB checking C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings)

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 30, 2018

According to rust-lang/rust#53676, there is an upper bound to what you can do with offset. miri should check that.

Related question: Shouldn't the place_field method in the miri engine check pointer_offset_inbounds? That will be a heavy perf hit, but I think we might currently be missing out on some UB.

@RalfJung
Copy link
Member Author

Or maybe projections can never leave the bounds... I have not been able to exploit this.

We currently miss the UB in

use std::mem;

fn main() {
    let slice: &[u8; 5] = &[0; 5];
    let slice: &[u8; 10] = unsafe { mem::transmute(slice) };
    let _x = &slice[7];
}

but IMHO this is a case for validating values during computation, not for adding bounds checks to projections.

@RalfJung
Copy link
Member Author

Also see #437

@RalfJung RalfJung added the C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement label Nov 17, 2018
@RalfJung RalfJung added the A-validation Area: This affects enforcing the validity invariant, and related UB checking label Mar 8, 2019
@RalfJung
Copy link
Member Author

See the discussion at rust-lang/nomicon#149 (comment) and rust-lang/nomicon#149 (comment): for now we probably don't actually need to do any checks on the offsets here if we instead check that when we dereference (*ptr), the pointer is not dangling. That's more UB and so a good conservative start.

It will be interesting to see if there is code in our test suite / libstd violating this.

Also, here's another example:

fn main() {
    let local = 5u8;
    let ptr = (&local as *const u8).wrapping_sub(1) as *const (u8, u8);
    let _ref = unsafe { &(*ptr).1 };
}

And with rust-lang/rfcs#2582, here's another one:

fn main() {
    let ptr: *const (u8, u8) = 16usize as *const _;
    let ptr2 = &raw const (*ptr).1;
}

@RalfJung RalfJung added the I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) label Aug 3, 2019
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
Miri: Check that a ptr is aligned and inbounds already when evaluating `*`

This syncs Miri with what the Nomicon and the Reference say, and resolves rust-lang/miri#447.

Also this would not have worked without rust-lang#62982 due to new cycles. ;)

r? @oli-obk
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
Miri: Check that a ptr is aligned and inbounds already when evaluating `*`

This syncs Miri with what the Nomicon and the Reference say, and resolves rust-lang/miri#447.

Also this would not have worked without rust-lang#62982 due to new cycles. ;)

r? @oli-obk
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
Miri: Check that a ptr is aligned and inbounds already when evaluating `*`

This syncs Miri with what the Nomicon and the Reference say, and resolves rust-lang/miri#447.

Also this would not have worked without rust-lang#62982 due to new cycles. ;)

r? @oli-obk
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
Miri: Check that a ptr is aligned and inbounds already when evaluating `*`

This syncs Miri with what the Nomicon and the Reference say, and resolves rust-lang/miri#447.

Also this would not have worked without rust-lang#62982 due to new cycles. ;)

r? @oli-obk
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
Miri: Check that a ptr is aligned and inbounds already when evaluating `*`

This syncs Miri with what the Nomicon and the Reference say, and resolves rust-lang/miri#447.

Also this would not have worked without rust-lang#62982 due to new cycles. ;)

r? @oli-obk
bors added a commit that referenced this issue Aug 15, 2019
adjust tests for eager pointer checks on deref

The Miri side of rust-lang/rust#63075.

Fixes #447.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-validation Area: This affects enforcing the validity invariant, and related UB checking C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings)
Projects
None yet
1 participant