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

Miri misses UB in place expressions when there are "_" patterns #2360

Closed
RalfJung opened this issue Jul 12, 2022 · 5 comments · Fixed by #3150
Closed

Miri misses UB in place expressions when there are "_" patterns #2360

RalfJung opened this issue Jul 12, 2022 · 5 comments · Fixed by #3150
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) S-blocked Status: blocked on something happening somewhere else

Comments

@RalfJung
Copy link
Member

RalfJung commented Jul 12, 2022

The following example should both be UB, but Miri finds neither of it: (this got fixed by rust-lang/rust#104844)

fn main() {
    let ptr = 32 as *const u8;
    unsafe { let _ = *ptr; }
}

This is blocked on rust-lang/rust#99180.

Here's another variant: (that is not fixed yet, will be fixed by rust-lang/rust#103208)

fn main() {
    let ptr = 32 as *const u8;
    unsafe {
        match *ptr { _ => {} }
    }
}
@RalfJung RalfJung added C-bug Category: This is a bug. A-interpreter Area: affects the core interpreter I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) S-blocked Status: blocked on something happening somewhere else labels Jul 12, 2022
@RalfJung
Copy link
Member Author

rust-lang/rust#102256 preserves these place expressions in MIR, which is a step towards fixing this.

@JakobDegen

We currently spec FakeRead as being a semantic nop; we could make an exception for the ForWildcard case, but it might be cleaner to have a new StatementKind::PlaceComputation or something like that

Would anything go wrong if we spec'ed them all to be evaluating the place expression and then discarding it (i.e., not actually doing a read, never producing a value)?

@JakobDegen
Copy link
Contributor

Can't be sure without trying, but I experimented for a bit and wasn't able to come up with any problems.

@RalfJung
Copy link
Member Author

Now that rust-lang/rust#104844 landed, this is actually fixed, we just need tests.

@RalfJung RalfJung mentioned this issue Apr 24, 2023
@RalfJung
Copy link
Member Author

RalfJung commented Apr 24, 2023

Actually only the let _ version is fixed. match with a _ pattern does not seem to be preserved in MIR.

@cjgillot is that expected?

EDIT: Oh I see, rust-lang/rust#103208 seems to be taking care of this.

@RalfJung
Copy link
Member Author

rust-lang/rust#103208 landed. Due to rust-lang/reference#1387, the original example is not UB any more, but similar things are, so this will be fixed after adding a test.

bors added a commit that referenced this issue Oct 30, 2023
make sure we catch UB in match place even with _ pattern

Fixes #2360
bors added a commit that referenced this issue Oct 30, 2023
make sure we catch UB in match place even with _ pattern

Fixes #2360
@bors bors closed this as completed in ea641f8 Oct 30, 2023
RalfJung pushed a commit to RalfJung/rust that referenced this issue Nov 4, 2023
make sure we catch UB in match place even with _ pattern

Fixes rust-lang/miri#2360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) S-blocked Status: blocked on something happening somewhere else
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants