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

'a: while break 'a{} results in illigal instruction #50856

Closed
DutchGhost opened this issue May 18, 2018 · 11 comments · Fixed by #51049
Closed

'a: while break 'a{} results in illigal instruction #50856

DutchGhost opened this issue May 18, 2018 · 11 comments · Fixed by #51049
Assignees
Labels
A-type-system Area: Type system C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@DutchGhost
Copy link
Contributor

Having the following main function results in an illigal instruction on stable, beta and nighlty.
It should be noted that when n is not used after the loop, the illigal instruction does not occur
https://play.rust-lang.org/?gist=96a11fd41327de4bf4e2c371dd7c1660&version=stable&mode=debug

fn main() {
    let mut n = 0; 
    'a: while {break 'a; true} {
        n += 1;
    }
    n += 2;
}

Interestingly putting the same while-loop in a function works fine:
https://play.rust-lang.org/?gist=ed60d2118b970be39e09bc8653a25345&version=stable&mode=debug

@DutchGhost DutchGhost changed the title break in condition for while loop results in illigal instruction break in while-loop condition results in illigal instruction in main() May 18, 2018
@DutchGhost
Copy link
Contributor Author

DutchGhost commented May 18, 2018

Update:
A more simple version that also results in an illigal instruction:
https://play.rust-lang.org/?gist=489a23ba854eea0b1e435b447ba81dc1&version=stable&mode=debug

fn main() {
   'a: while {break 'a; true} { }
   1;
}

after some more trying, it can be shortened to this (and works for functions other than in main, as shown here: https://play.rust-lang.org/?gist=79ea8fe2b5a1b96ba49089e1b4df1254&version=nightly&mode=debug) :

fn main() {
    'a: while break 'a{};
}

adding a return; statement at the end of main seems to 'fix' the problem

@DutchGhost DutchGhost changed the title break in while-loop condition results in illigal instruction in main() 'a: while break 'a{} results in illigal instruction May 18, 2018
@kennytm kennytm added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness C-bug Category: This is a bug. labels May 18, 2018
@kennytm
Copy link
Member

kennytm commented May 18, 2018

Checking the last example with godbolt,

  • 1.25 and above: results in undefined instruction (this issue)
  • 1.18 to 1.24: ICE with broken MIR
  • 1.17: becomes no-op
  • 1.16 and below: Error "use of undeclared label 'a"

@kennytm kennytm added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label May 18, 2018
@kennytm
Copy link
Member

kennytm commented May 18, 2018

Err the last example is even featured in E0590 (introduced in #39864), but why this seems not tested at all 😒.


Edit: It generates an unreachable at MIR level.

// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
fn main() -> (){
    let mut _0: ();                      // return place

    bb0: {                              
        unreachable;                     // bb0[0]: scope 0 at src/main.rs:1:11: 3:2
    }
}
The initial MIR already contains an unreachable.
// MIR for `main`
// source = MirSource { def_id: DefId(0/0:3 ~ 3[317d]::main[0]), promoted: None }
// pass_name = mir_map
// disambiguator = 0

fn main() -> (){
    let mut _0: ();                      // return place
    let mut _1: !;
    let mut _2: ();
    let mut _3: bool;
    let mut _4: !;
    let mut _5: ();

    bb0: {                              
        goto -> bb1;                     // bb0[0]: scope 0 at 3.rs:2:5: 2:25
    }

    bb1: {                              
        StorageLive(_3);                 // bb1[0]: scope 0 at 3.rs:2:15: 2:23
        _2 = ();                         // bb1[1]: scope 0 at 3.rs:2:15: 2:23
        goto -> bb4;                     // bb1[2]: scope 0 at 3.rs:2:15: 2:23
    }

    bb2: {                              
        _2 = ();                         // bb2[0]: scope 0 at 3.rs:2:5: 2:25
        StorageDead(_3);                 // bb2[1]: scope 0 at 3.rs:2:24: 2:25
        unreachable;                     // bb2[2]: scope 0 at 3.rs:1:11: 3:2
    }

    bb3: {                               // cleanup
        resume;                          // bb3[0]: scope 0 at 3.rs:1:1: 3:2
    }

    bb4: {                              
        goto -> bb5;                     // bb4[0]: scope 0 at 3.rs:2:15: 2:23
    }

    bb5: {                              
        goto -> bb6;                     // bb5[0]: scope 0 at 3.rs:2:15: 2:23
    }

    bb6: {                              
        goto -> bb2;                     // bb6[0]: scope 0 at 3.rs:2:15: 2:23
    }

    bb7: {                              
        _4 = ();                         // bb7[0]: scope 0 at 3.rs:2:15: 2:23
        unreachable;                     // bb7[1]: scope 0 at 3.rs:2:15: 2:23
    }

    bb8: {                              
        StorageDead(_4);                 // bb8[0]: scope 0 at 3.rs:2:22: 2:23
        switchInt(move _3) -> [false: bb2, otherwise: bb9]; // bb8[1]: scope 0 at 3.rs:2:5: 2:25
    }

    bb9: {                              
        _5 = ();                         // bb9[0]: scope 0 at 3.rs:2:23: 2:25
        goto -> bb1;                     // bb9[1]: scope 0 at 3.rs:2:5: 2:25
    }

    bb10: {                             
        StorageDead(_1);                 // bb10[0]: scope 0 at 3.rs:3:1: 3:2
        goto -> bb11;                    // bb10[1]: scope 0 at 3.rs:3:2: 3:2
    }

    bb11: {                             
        return;                          // bb11[0]: scope 0 at 3.rs:3:2: 3:2
    }
}
-Zunpretty=hir,typed
#[prelude_import]
use std::prelude::v1::*;
#[macro_use]
extern crate std;
fn main() ({ ('a: while ((break 'a  as !)) { } as ()); } as !)

@varkor
Copy link
Member

varkor commented May 18, 2018

Err the last example is even featured in E0590 (introduced in #39864), but why this seems not tested at all 😒.

It's because in the example that is tested, there's no semicolon:

fn main() {
    'foo: while break 'foo {} // ok
}
fn main() {
    'foo: while break 'foo {}; // uh-oh
}

Why that makes a difference, I haven't determined yet.

@kennytm
Copy link
Member

kennytm commented May 19, 2018

The problem is, for some reason, the type checker inferred 'a: while break 'a {}; to have type !, and thus the rest of the block is unreachable.

// compile-pass, but shouldn't.
#![feature(never_type)]
fn main() {
    let _: ! = { 'a: while break 'a {}; };
}

However, without the ; the inferred type is just () as expected.

@kennytm kennytm added the A-type-system Area: Type system label May 19, 2018
@varkor
Copy link
Member

varkor commented May 19, 2018

Maybe slightly off-topic, but why are references to the label even allowed in the condition, rather than solely the body? You don't achieve extra expressiveness, as it's equivalent to returning false in the condition, and it makes things more complicated.

@DutchGhost
Copy link
Contributor Author

having code like this results in an ICE:

const ARR: [(); 0] = [ 'a: while break 'a {}; 0];

fn main() {
    
}
error: internal compiler error: librustc_mir/transform/qualify_consts.rs:273: multiple assignments to _1
 --> src/main.rs:1:24
  |
1 | const ARR: [(); 0] = [ 'a: while break 'a {}; 0];
  |                        ^^^^^^^^^^^^^^^^^^^^^

thread 'rustc' panicked at 'Box<Any>', librustc_errors/lib.rs:488:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: aborting due to previous error


note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.26.0 (a77568041 2018-05-07) running on x86_64-unknown-linux-gnu

note: compiler flags: -C codegen-units=1 -C debuginfo=2 --crate-type bin

note: some of the compiler flags provided by cargo are hidden

error: Could not compile `playground`.

https://play.rust-lang.org/?gist=56a8ce6081021416d64f4a056aeae8ca&version=stable&mode=debug

@nagisa
Copy link
Member

nagisa commented May 20, 2018

Maybe slightly off-topic, but why are references to the label even allowed in the condition, rather than solely the body? You don't achieve extra expressiveness, as it's equivalent to returning false in the condition, and it makes things more complicated.

Because it is consistent with the desugaring of the while loop to a

'a: loop {
    if { break 'a } { break 'a }
}

which is a valid and fairly sensible code.

@nikomatsakis
Copy link
Contributor

I think the bug in in the type-checker then: it should not be giving 'a: while (break 'a) the type of !, clear. @varkor — you interested in fixing? I can give a few tips...

@nikomatsakis nikomatsakis added P-high High priority P-medium Medium priority and removed P-high High priority labels May 24, 2018
@nikomatsakis
Copy link
Contributor

Marking as P-medium: technically a regression, but a very old one, and kind of a corner case. Still, I'll try to leave some notes on how to fix.

@varkor
Copy link
Member

varkor commented May 24, 2018

@nikomatsakis: yeah, I can have a look into this. Might be a little delayed, but definitely within the next couple of weeks.

@varkor varkor self-assigned this May 24, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 26, 2018
…omatsakis

Fix behaviour of divergence in while loop conditions

This fixes `'a: while break 'a {};` being treated as diverging, by tracking break expressions in the same way as in `loop` expressions.

Fixes rust-lang#50856.

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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