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

mem::size_of_val returns wrong value for DSTs with certain field layouts #35815

Closed
apasel422 opened this issue Aug 19, 2016 · 6 comments · Fixed by #36027
Closed

mem::size_of_val returns wrong value for DSTs with certain field layouts #35815

apasel422 opened this issue Aug 19, 2016 · 6 comments · Fixed by #36027
Assignees

Comments

@apasel422
Copy link
Contributor

apasel422 commented Aug 19, 2016

On stable, beta, and nightly, the follow code prints 16 followed by 24:

struct Foo<T: ?Sized> {
    a: i64,
    b: bool,
    c: T,
}

fn main() {
    let a = Box::new(Foo {a: 1, b: false, c: 1u32});
    println!("{}", std::mem::size_of_val(&*a));

    let a: Box<Foo<Send>> = a;
    println!("{}", std::mem::size_of_val(&*a));
}

CC #27023

@apasel422 apasel422 changed the title mem::size_of_val returns wrong value for DSTs with certain alignments mem::size_of_val returns wrong value for DSTs with certain field layouts Aug 19, 2016
@eddyb
Copy link
Member

eddyb commented Aug 19, 2016

This is probably for drop flags, in which case it should get fixed by #35764

@apasel422
Copy link
Contributor Author

Reduced:

struct Foo<T: ?Sized> {
    a: i64,
    b: bool,
    c: T,
}

fn main() {
    let a: &Foo<i32> = &Foo { a: 1, b: false, c: 2i32 };
    println!("{}", std::mem::size_of_val(a));

    let a: &Foo<Send> = a;
    println!("{}", std::mem::size_of_val(a));
}

@durka
Copy link
Contributor

durka commented Aug 20, 2016

@eddyb where would there be a drop flag here?

@eddyb
Copy link
Member

eddyb commented Aug 20, 2016

@durka My point isn't that there is, but that the code assumes all trait objects have drop flags. I could be wrong though, it might be a bug in the alignment logic.

@eddyb
Copy link
Member

eddyb commented Aug 20, 2016

Not fixed by #35764, will have to investigate further.

@eddyb
Copy link
Member

eddyb commented Aug 20, 2016

Uhhhh, the computation starts with:

rustc_trans::glue: DST Foo<std::marker::Send> sizing_type: { i64, i8 }
rustc_trans::glue: DST Foo<std::marker::Send> statically sized prefix size: 16 align: 8

Which is completely wrong, because you want no trailing padding, i.e. size should be 9.

EDIT: getting a field pointer might also be broken, needs looking into.
EDIT2: there's code to handle the "minimum size" needed to compute the field offset, so that works.

@eddyb eddyb self-assigned this Aug 20, 2016
bors added a commit that referenced this issue Aug 28, 2016
rustc_trans: don't round up the DST prefix size to its alignment.

Fixes #35815 by using `ty::layout` and `min_size` to compute the size of the DST prefix.
`ty::layout::Struct::min_size` is not rounded up to alignment, which could be smaller for the DST field.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants