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

Slice length becomes random between nightly-2021-03-04 and nightly-2021-03-05 #82859

Closed
juntyr opened this issue Mar 7, 2021 · 11 comments · Fixed by #83030
Closed

Slice length becomes random between nightly-2021-03-04 and nightly-2021-03-05 #82859

juntyr opened this issue Mar 7, 2021 · 11 comments · Fixed by #83030
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@juntyr
Copy link
Contributor

juntyr commented Mar 7, 2021

Code

I tried this code (thanks to @jyn514 for the minified version):

fn impl_get_slice() -> &'static [()] {
    &[()]
}

#[inline(always)]
fn get_slice() -> &'static [()] {
    let ret = (|| { impl_get_slice() })();
    ret
}

fn main() {
    let output = get_slice().len();
    println!("{}", output);
}

I expected the code to print 0, the length of the slice.

Instead, a random value is printed.

Version it worked on

It most recently worked on:

rustc --version --verbose:

rustc 1.52.0-nightly (476acbf1e 2021-03-03)
binary: rustc
commit-hash: 476acbf1e9965b5e95c90f0d7d658709812b7003
commit-date: 2021-03-03
host: x86_64-unknown-linux-gnu
release: 1.52.0-nightly
LLVM version: 11.0.1

Version with regression

rustc --version --verbose:

rustc 1.52.0-nightly (45b3c2851 2021-03-04)
binary: rustc
commit-hash: 45b3c28518e4c45dfd12bc2c4400c0d0e9639927
commit-date: 2021-03-04
host: x86_64-unknown-linux-gnu
release: 1.52.0-nightly
LLVM version: 12.0.0

@rustbot modify labels: +regression-from-nightly-to-nightly -regression-untriaged

@rustbot rustbot added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 7, 2021
@Stupremee
Copy link
Member

Stupremee commented Mar 7, 2021

Are you able to share a code snippet that reproduces your bug, even a long example is fine for now?

@nikic
Copy link
Contributor

nikic commented Mar 7, 2021

Very likely caused by LLVM upgrade.

@juntyr
Copy link
Contributor Author

juntyr commented Mar 7, 2021

Are you able to share a code snippet that reproduces your bug, even a long example is fine for now?

I'm having lunch now, but I'll see what I can do afterwards :)

@juntyr
Copy link
Contributor Author

juntyr commented Mar 7, 2021

So far, my only progress is that

for item in my_iter.iter() {
    ...
}

works whilst the following does not:

my_iter.iter().for_each(|item| {
    ...
});

Again, adding a println! inside the loop makes the second example work. I'm still trying to reduce this problem to an example which can be reproduced without including code from my project.

@juntyr
Copy link
Contributor Author

juntyr commented Mar 7, 2021

@Stupremee @nikic Ok, I finally was able to reduce this bug to its bones - here is the code (it turned out to be quite different than what I had originally expected):

pub struct S;

fn check() -> bool {
    true
}

impl S {
    fn impl_get_slice(&self, _val: &()) -> &[()] {
        &[]
    }

    #[inline(always)]
    fn get_slice(&self, val: &()) -> &[()] {
        if !check() {
            panic!("panic")
        };
        
        let run = || { self.impl_get_slice(val) };
        let ret = run();
        ret
    }
}

fn main() {
    let output;

    let s = S;
    output = s.get_slice(&()).len();

    println!("{}", output);
}

You can see that on the current nightly, this snippet prints a random value:
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=b5ae7bbbcecbbea9b03ad0b8b364cd9f
It should, as demonstrated on the release branch, print 0, the length of the slice &[]:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b5ae7bbbcecbbea9b03ad0b8b364cd9f

My best guess is that in debug mode (this bug does not occur in release), we are actually trying to read the length of our slice &[]. However, some change to codegen may have optimised out storing that length, so that we are reading garbage from the stack.

@jyn514
Copy link
Member

jyn514 commented Mar 7, 2021

Slightly smaller:

fn indirect_get_slice() -> &'static [usize] {
    &[]
}

#[inline(always)]
fn get_slice() -> &'static [usize] {
    let ret = indirect_get_slice();
    ret
}

fn main() {
    let output = get_slice().len();
    println!("{}", output);
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=22893ec68663e7c7716adea458339433

@jyn514
Copy link
Member

jyn514 commented Mar 7, 2021

My best guess is that in debug mode (this bug does not occur in release), we are actually trying to read the length of our slice &[]. However, some change to codegen may have optimised out storing that length, so that we are reading garbage from the stack.

Hmm, this also happens in debug mode for me (both with the larger and smaller example).

@Stupremee
Copy link
Member

It doesn't work in release mode. It only works in debug mode

@jyn514 jyn514 added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Mar 7, 2021
@juntyr juntyr changed the title Println-influenced behaviour between nightly-2021-03-04 and nightly-2021-03-05 Slice length becomes random between nightly-2021-03-04 and nightly-2021-03-05 Mar 7, 2021
@ghost
Copy link

ghost commented Mar 7, 2021

This requires -Cdebuginfo=2: https://godbolt.org/z/nGeWa6 -> https://godbolt.org/z/ffe6Yz

@nikic
Copy link
Contributor

nikic commented Mar 7, 2021

This is likely the same issue as #82833.

@JohnTitor JohnTitor added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 7, 2021
@JohnTitor
Copy link
Member

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@JohnTitor JohnTitor added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 7, 2021
@nikic nikic self-assigned this Mar 9, 2021
nikic added a commit to nikic/rust that referenced this issue Mar 11, 2021
@bors bors closed this as completed in 215ebc3 Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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.

6 participants