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

Regression in beta: unsafe-free, pure Rust pgm. with no deps segfaults #36401

Closed
osa1 opened this issue Sep 11, 2016 · 8 comments
Closed

Regression in beta: unsafe-free, pure Rust pgm. with no deps segfaults #36401

osa1 opened this issue Sep 11, 2016 · 8 comments
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html regression-from-stable-to-beta Performance or correctness regression from stable to beta.

Comments

@osa1
Copy link
Contributor

osa1 commented Sep 11, 2016

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Key {
    AltArrow(Arrow),
    Arrow(Arrow),
    Ctrl(char),
    Char(char),
    Esc,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Arrow { Left, Right, Up, Down }

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Event {
    Key(Key),
    String(String),
    Resize,
    Unknown(Vec<u8>),
}

static XTERM_SINGLE_BYTES : [(u8, Event); 5] =
    [ (1,  Event::Key(Key::Ctrl('a'))),
      (5,  Event::Key(Key::Ctrl('e'))),
      (23, Event::Key(Key::Ctrl('w'))),
      (11, Event::Key(Key::Ctrl('k'))),
      (4,  Event::Key(Key::Ctrl('d'))),
    ];

fn main() {
    println!("{:?}", XTERM_SINGLE_BYTES);
}

(can be tested here: https://is.gd/EkJ3by)

When run with beta or nightly, this program segfaults.

I tried to simplify this a little bit, but my attempts either turned segfault into a stack overflow, or made it working. Here are some changes I've tried:

  • I tried [(u8, char)] for XTERM_SINGLE_BYTES and it worked fine.
  • I tried removing Event::Keys (e.g. [(u8, Key)]) and it worked.
  • I removed some of the constructors from Event enum, most of the time the error turned into a stack overflow.

In my original program I'm iterating the array, and for the u8 part I'm getting random bytes instead of bytes I put in the array. I'm using rustc 1.13.0-nightly (70598e0 2016-09-03).

@durka
Copy link
Contributor

durka commented Sep 11, 2016

Minimized a bit:

#[derive(Debug)]
pub enum Event {
    Key(u8),

    Resize,

    Unknown(Vec<u8>),
}

static XTERM_SINGLE_BYTES : [(u8, Event); 1] =
    [ (1,  Event::Resize)
    ];

fn main() {
    println!("{:?}", XTERM_SINGLE_BYTES);
}

This incorrectly prints [(1, Key(0))].

I wonder if it has something to do with the nullable pointer layout optimization, as the issue goes away if the Vec variant is removed.

@TimNN
Copy link
Contributor

TimNN commented Sep 11, 2016

With -Z orbit, this was introduced between nightly-2016-05-08 and nightly-2016-05-09 (Changes).

(Edit: I missed a nightly in the original version of this comment.)

@osa1
Copy link
Contributor Author

osa1 commented Sep 11, 2016

Thanks @TimNN, as a workaround until this is fixed I switched to nightly-2016-05-08 and it works nicely.

@bluss bluss added I-wrong A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html labels Sep 11, 2016
@durka
Copy link
Contributor

durka commented Sep 11, 2016

In that case I'm suspicious of #33130 cc @eddyb

@TimNN
Copy link
Contributor

TimNN commented Sep 11, 2016

The Vec is not necessary to reproduce,

#[derive(Debug)]
pub enum Event {
    Key(u8),
    Resize,
    Unknown(i16),
}

as long as the types wrapped are of different sizes.

Also, I can reproduce this only as long as the size of the integer used in the static is smaller than than the size of the larger "wrapped" integer (in this case, the wrapped integer is i16, so only i8 and u8 will reproduce this).

@arielb1
Copy link
Contributor

arielb1 commented Sep 11, 2016

Looks like missing alignment:

@_ZN8rust_out18XTERM_SINGLE_BYTES17heee0b00158a3d910E = internal constant { { i8, { i8, [3 x i8] } } } { { i8, { i8, [3 x i8] } } { i8 1, { i8, [3 x i8] } { i8 1, [3 x i8] undef } } }, align 2

@nagisa
Copy link
Member

nagisa commented Sep 11, 2016

@arielb1 where did you get this? With original test case on play.rlo has align 8 for all stable, beta and nightly, which seems to be the expected value?

@arielb1
Copy link
Contributor

arielb1 commented Sep 11, 2016

The u16 variant by @TimNN. The problem is not the align 2 but the { i8, { i8, [3 x i8] } }

@arielb1 arielb1 added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Sep 11, 2016
bors added a commit that referenced this issue Sep 12, 2016
use `adt::trans_const` when translating constant closures and tuples

The previous way dropped padding on the floor.

Fixes #36401

r? @eddyb
pmatos pushed a commit to LinkiTools/rust that referenced this issue Sep 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

No branches or pull requests

6 participants