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

align_of for small structs does not give the actual alignment of pointers #21611

Closed
huonw opened this Issue Jan 25, 2015 · 7 comments

Comments

Projects
None yet
8 participants
@huonw
Copy link
Member

huonw commented Jan 25, 2015

#[derive(Debug, Copy)]
struct Foo {
    x: u8,
}

fn main() {
    let f = Foo { x: 1 };
    let vec = vec![f, f, f, f, f];

    println!("size: {}", std::mem::size_of::<Foo>());
    println!("align: {}", std::mem::align_of::<Foo>());
    println!("alignment offset: {}", (&vec[2] as *const _ as usize) % std::mem::align_of::<Foo>());
}

It prints size: 1 align: 8 alignment offset: 2. I would expect that every value of type T is aligned to align_of::<T>() (i.e. the % should always give 0).

On x86-64, this applies to anything with alignment < 8, including #[repr(packed)] structs.

I'm not sure it's a bug, but it does seem rather confusing.

Also, NB. this means size_of < align_of.

(min_align_of acts more like I might expect.)

@huonw huonw added the I-nominated label Jan 25, 2015

@Diggsey

This comment has been minimized.

Copy link
Contributor

Diggsey commented Jan 25, 2015

Comment from the source:

// We use the preferred alignment as the default alignment for a type. This
// appears to be what clang migrated towards as well:
//
// http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110725/044411.html

Unfortunately, the link does not give details of why that choice was made, as it's a surprising one to say the least.

@brson brson added the E-easy label Jan 29, 2015

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 29, 2015

polish issue for 1.0 milestone, P-high.

@pnkfelix pnkfelix added the P-medium label Jan 29, 2015

@pnkfelix pnkfelix added this to the 1.0 milestone Jan 29, 2015

@pnkfelix pnkfelix removed the I-nominated label Jan 29, 2015

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jan 29, 2015

Hard part is figuring out the correct implementation.

@tbu-

This comment has been minimized.

Copy link
Contributor

tbu- commented Feb 6, 2015

I don't see the bug here, min_align_of is the hard limit that must be adhered to, and align_of gives a suggestion. It would be very bad to take an alignment of 8 for a 1-byte struct in a vector.

@huonw

This comment has been minimized.

Copy link
Member Author

huonw commented Feb 6, 2015

@tbu- that's the point, the conveniently named "align_of" is a dangerous value to use in many situations, while min_align_of is conservative and always correct (although may be less efficient than necessary in a few scenarios).

E.g. C++11's alignof acts like min_align_of, this prints 1:

#include<cstdio>

struct foo { char x; };
int main() {
    printf("%lu\n", alignof(struct foo));
}
@lilyball

This comment has been minimized.

Copy link
Contributor

lilyball commented Feb 7, 2015

I've always been confused by what the heck align_of is. I know the module is already marked stable, but I really feel like it should be renamed to preferred_align_of and min_align_of be changed to align_of. Every single time I've ever had a reason to look at alignment, minimum alignment turns out to be what I was interested in. May as well make it the default function.

Similarly, the documentation on what is currently align_of claims that this is the alignment that, if adhered to, guarantees the type will function properly. Surely that's actually the minimum alignment. The docs should be changed instead to explain that this is the preferred alignment, and hopefully give a short explanation as to what the heck that actually means.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Apr 2, 2015

I think sizeof and alignof (the defaults) should do whatever C++ does.

huonw added a commit to huonw/rust that referenced this issue May 20, 2015

Make `align_of` behave like `min_align_of`.
This removes a footgun, since it is a reasonable assumption to make that
pointers to `T` will be aligned to `align_of::<T>()`. This also matches
the behaviour of C/C++. `min_align_of` is now deprecated.

Closes rust-lang#21611.

@steveklabnik steveklabnik removed this from the 1.0 milestone May 21, 2015

huonw added a commit to huonw/rust that referenced this issue Jun 26, 2015

Make `align_of` behave like `min_align_of`.
This removes a footgun, since it is a reasonable assumption to make that
pointers to `T` will be aligned to `align_of::<T>()`. This also matches
the behaviour of C/C++. `min_align_of` is now deprecated.

Closes rust-lang#21611.

bors added a commit that referenced this issue Jun 26, 2015

Auto merge of #25646 - huonw:align, r=alexcrichton
This removes a footgun, since it is a reasonable assumption to make that
pointers to `T` will be aligned to `align_of::<T>()`. This also matches
the behaviour of C/C++. `min_align_of` is now deprecated.

Closes #21611.

@bors bors closed this in #25646 Jun 26, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 1, 2015

Make `align_of` behave like `min_align_of`.
This removes a footgun, since it is a reasonable assumption to make that
pointers to `T` will be aligned to `align_of::<T>()`. This also matches
the behaviour of C/C++. `min_align_of` is now deprecated.

Closes rust-lang#21611.

jroesch added a commit to jroesch/rust that referenced this issue Jul 21, 2015

Make `align_of` behave like `min_align_of`.
This removes a footgun, since it is a reasonable assumption to make that
pointers to `T` will be aligned to `align_of::<T>()`. This also matches
the behaviour of C/C++. `min_align_of` is now deprecated.

Closes rust-lang#21611.

thepowersgang added a commit to thepowersgang/rust that referenced this issue Jul 25, 2015

Make `align_of` behave like `min_align_of`.
This removes a footgun, since it is a reasonable assumption to make that
pointers to `T` will be aligned to `align_of::<T>()`. This also matches
the behaviour of C/C++. `min_align_of` is now deprecated.

Closes rust-lang#21611.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.