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

MIR generates Unreachable in fewer situations than before #41446

Closed
oli-obk opened this issue Apr 21, 2017 · 8 comments
Closed

MIR generates Unreachable in fewer situations than before #41446

oli-obk opened this issue Apr 21, 2017 · 8 comments
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Apr 21, 2017

rust-lang/miri#157 (comment) for details

Either the Unreachable statement should be removed from MIR or it should be generated at all locations that produce a value of an empty type

@arielb1
Copy link
Contributor

arielb1 commented Apr 21, 2017

unreachable is still use:

fn foo(x: !) -> u32 { x }

MIR:

fn foo(_1: !) -> u32 {
    let mut _0: u32;                     // return pointer
    scope 1 {
        let _2: !;                       // "x" in scope 1 at <anon>:2:8: 2:9
    }
    let mut _3: !;

    bb0: {
        StorageLive(_2);                 // scope 0 at <anon>:2:8: 2:9
        _2 = _1;                         // scope 0 at <anon>:2:8: 2:9
        _3 = _2;                         // scope 1 at <anon>:2:23: 2:24
        unreachable;                     // scope 1 at <anon>:2:23: 2:24
    }
}

should be generated at all locations that produce a value of an empty type

Having a value of an empty type is UB, so rustc can generate whatever type-checking MIR it wants. You could write a PR that makes uses of ! be translated into UB.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 21, 2017

Why is this case more special than the deref of pointers? It could just be a return. I think there should be a concrete strategy for where to insert unreachable.

@nagisa
Copy link
Member

nagisa commented Apr 21, 2017

Unreachable is here to stay. It will be necessary for EBBs at the very least.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 21, 2017

Then I don't see the harm in producing it more frequently. If Rust turns UB into explicit unreachable, we can actually add lints that help with writing correct unsafe code.

@arielb1
Copy link
Contributor

arielb1 commented Apr 21, 2017

Why is this case more special than the deref of pointers? It could just be a return.

Because then borrowck/dropck/whoever would complain that there is a return without assigning anything to the return value.

Then I don't see the harm in producing it more frequently.

MIR construction intentionally tries to be simple unless there is a good reason to make it complicated.

@oli-obk oli-obk changed the title MIR does not generate Unreachable anymore MIR generates Unreachable in fewer situations than before Apr 22, 2017
@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 22, 2017

MIR construction intentionally tries to be simple unless there is a good reason to make it complicated

Makes sense, but it could be done in a MIR pass. Should be rather trivial (converting all reads and writes on uninhabited types into Unreachable).

@Mark-Simulacrum Mark-Simulacrum added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 22, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 27, 2017
bors added a commit that referenced this issue Mar 20, 2018
Eager unreachable blocks for `!` type

This change optimises the generation of MIR in the presence of `!`. Blocks are now immediately marked as unreachable if:
- A function argument is derived from the value of type `!`.
- A match arm binds a value whose type is derived from `!`.
- An rvalue is created whose type is derived from `!`.

This avoids unnecessary MIR generation and also fixes #43061 and #41446.

r? @nikomatsakis
@steveklabnik
Copy link
Member

Triage: MIR produced today is

fn foo(_1: !) -> u32 {
    debug x => _1;                       // in scope 0 at src/lib.rs:3:8: 3:9
    let mut _0: u32;                     // return place in scope 0 at src/lib.rs:3:17: 3:20

    bb0: {
        unreachable;                     // scope 0 at src/lib.rs:3:23: 3:24
    }
}

@oli-obk
Copy link
Contributor Author

oli-obk commented May 7, 2020

I believe this issue is fixed.

#![feature(never_type)]
#![allow(unreachable_code)]

fn main() {
    let y = &5;
    let x: ! = unsafe {
        *(y as *const _ as *const !) //~ ERROR entered unreachable code
    };
    f(x)
}

#[inline(never)]
fn f(x: !) -> ! { x }

produces

fn f(_1: !) -> ! {
    debug x => _1;                       // in scope 0 at src/main.rs:13:6: 13:7
    let mut _0: !;                       // return place in scope 0 at src/main.rs:13:15: 13:16

    bb0: {
        unreachable;                     // scope 0 at src/main.rs:13:19: 13:20
    }
}

fn main() -> () {
    let mut _0: ();                      // return place in scope 0 at src/main.rs:4:11: 4:11
    let _1: &i32;                        // in scope 0 at src/main.rs:5:9: 5:10
    let mut _3: *const !;                // in scope 0 at src/main.rs:7:10: 7:37
    let mut _4: *const i32;              // in scope 0 at src/main.rs:7:11: 7:24
    let mut _5: *const i32;              // in scope 0 at src/main.rs:7:11: 7:24
    let mut _6: &i32;                    // in scope 0 at src/main.rs:5:13: 5:15
    scope 1 {
        debug y => _1;                   // in scope 1 at src/main.rs:5:9: 5:10
        let _2: ! as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 1 at src/main.rs:6:9: 6:10
        scope 2 {
            debug x => _2;               // in scope 2 at src/main.rs:6:9: 6:10
        }
        scope 3 {
        }
    }

    bb0: {
        StorageLive(_1);                 // scope 0 at src/main.rs:5:9: 5:10
        _6 = const main::promoted[0];    // scope 0 at src/main.rs:5:13: 5:15
                                         // ty::Const
                                         // + ty: &i32
                                         // + val: Unevaluated(DefId(0:3 ~ playground[c63f]::main[0]), [], Some(promoted[0]))
                                         // mir::Constant
                                         // + span: src/main.rs:5:13: 5:15
                                         // + literal: Const { ty: &i32, val: Unevaluated(DefId(0:3 ~ playground[c63f]::main[0]), [], Some(promoted[0])) }
        _1 = _6;                         // scope 0 at src/main.rs:5:13: 5:15
        StorageLive(_2);                 // scope 1 at src/main.rs:6:9: 6:10
        StorageLive(_3);                 // scope 3 at src/main.rs:7:10: 7:37
        StorageLive(_4);                 // scope 3 at src/main.rs:7:11: 7:24
        StorageLive(_5);                 // scope 3 at src/main.rs:7:11: 7:24
        _5 = &raw const (*_1);           // scope 3 at src/main.rs:7:11: 7:12
        _4 = _5;                         // scope 3 at src/main.rs:7:11: 7:24
        _3 = move _4 as *const ! (Misc); // scope 3 at src/main.rs:7:10: 7:37
        StorageDead(_4);                 // scope 3 at src/main.rs:7:36: 7:37
        unreachable;                     // scope 3 at src/main.rs:7:9: 7:37
    }
}

promoted[0] in main: &i32 = {
    let mut _0: &i32;                    // return place in scope 0 at src/main.rs:5:13: 5:15
    let mut _1: i32;                     // in scope 0 at src/main.rs:5:14: 5:15
    scope 1 {
        scope 2 {
        }
        scope 3 {
        }
    }

    bb0: {
        _1 = const 5i32;                 // scope 0 at src/main.rs:5:14: 5:15
                                         // ty::Const
                                         // + ty: i32
                                         // + val: Value(Scalar(0x00000005))
                                         // mir::Constant
                                         // + span: src/main.rs:5:14: 5:15
                                         // + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) }
        _0 = &_1;                        // scope 0 at src/main.rs:5:13: 5:15
        return;                          // scope 0 at src/main.rs:5:13: 5:15
    }
}

so the function call to f is removed and replaced by an unreachable terminator, which is exactly what the original miri issue was about.

@oli-obk oli-obk closed this as completed May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants