rust-lang / rust-lang/rust-bindgen

mishandling of aligned typedefs

Open
#1,753 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Rust
Stars
5.3k
Forks
829
Avg merge
1d 1h
Merged PRs (30d)
15

Description

I'm seeing some unexpected (to me at least) behavior with GCC and aligned typedefs. First I'll give an example of the behavior itself, and then I'll show how bindgen gets confused by it. All of the following is with GCC 9.3.0, bindgen 0.53.2, Arch Linux, x86_64.

Consider this example, which I'll call align.c. There are two pairs of structs, B which contains two A members, and D which contains two C members. Struct A applies an alignment attribute directly to its struct definition, while struct C applies an alignment attribute to its typedef. I had no idea this distinction was important, but apparently it is:

#include <stdio.h>
#include <stdalign.h>
#include <stddef.h>

struct A {
    char x[3];
} __attribute__ ((aligned(64)));

struct B {
    struct A a1;
    struct A a2;
    char x;
};

// C and D are just like A and B, except that the typedef is aligned
// rather than the struct itself.
__attribute__ ((aligned(64))) typedef struct {
    char x[3];
} C;

typedef struct {
    C c1;
    C c2;
    char x;
} D;

int main() {
    printf("sizeof(struct A) = %ld\n", sizeof(struct A));
    printf("alignof(struct A) = %ld\n", alignof(struct A));
    printf("sizeof(struct B) = %ld\n", sizeof(struct B));
    printf("alignof(struct B) = %ld\n", alignof(struct B));
    printf("offset of B.x = %ld\n", offsetof(struct B, x));
    printf("\n");
    printf("sizeof(C) = %ld\n", sizeof(C));
    printf("alignof(C) = %ld\n", alignof(C));
    printf("sizeof(D) = %ld\n", sizeof(D));
    printf("alignof(D) = %ld\n", alignof(D));
    printf("offset of D.x = %ld\n", offsetof(D, x));
    return 0;
}

Then gcc align.c && ./a.out prints:

sizeof(struct A) = 64
alignof(struct A) = 64
sizeof(struct B) = 192
alignof(struct B) = 64
offset of B.x = 128

sizeof(C) = 3
alignof(C) = 64
sizeof(D) = 128
alignof(D) = 64
offset of D.x = 67

A and B are doing what I expect there, but C is really surprising. Its size is not a multiple of its alignment. I didn't know that was possible in C, and it's documented as impossible in Rust.

Let's see how bindgen runs into trouble with this. Here's the relevant part of what it generates:

#[repr(C)]
#[repr(align(64))]
#[derive(Copy, Clone)]
pub struct A {
    pub x: [::std::os::raw::c_char; 3usize],
}
#[test]
fn bindgen_test_layout_A() {
    assert_eq!(
        ::std::mem::size_of::<A>(),
        64usize,
        concat!("Size of: ", stringify!(A))
    );
    assert_eq!(
        ::std::mem::align_of::<A>(),
        64usize,
        concat!("Alignment of ", stringify!(A))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<A>())).x as *const _ as usize },
        0usize,
        concat!("Offset of field: ", stringify!(A), "::", stringify!(x))
    );
}
#[repr(C)]
#[repr(align(64))]
#[derive(Copy, Clone)]
pub struct B {
    pub a1: A,
    pub a2: A,
    pub x: ::std::os::raw::c_char,
}
#[test]
fn bindgen_test_layout_B() {
    assert_eq!(
        ::std::mem::size_of::<B>(),
        192usize,
        concat!("Size of: ", stringify!(B))
    );
    assert_eq!(
        ::std::mem::align_of::<B>(),
        64usize,
        concat!("Alignment of ", stringify!(B))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<B>())).a1 as *const _ as usize },
        0usize,
        concat!("Offset of field: ", stringify!(B), "::", stringify!(a1))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<B>())).a2 as *const _ as usize },
        64usize,
        concat!("Offset of field: ", stringify!(B), "::", stringify!(a2))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<B>())).x as *const _ as usize },
        128usize,
        concat!("Offset of field: ", stringify!(B), "::", stringify!(x))
    );
}
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct C {
    pub x: [::std::os::raw::c_char; 3usize],
}
#[test]
fn bindgen_test_layout_C() {
    assert_eq!(
        ::std::mem::size_of::<C>(),
        3usize,
        concat!("Size of: ", stringify!(C))
    );
    assert_eq!(
        ::std::mem::align_of::<C>(),
        1usize,
        concat!("Alignment of ", stringify!(C))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<C>())).x as *const _ as usize },
        0usize,
        concat!("Offset of field: ", stringify!(C), "::", stringify!(x))
    );
}
#[repr(C)]
#[repr(align(64))]
#[derive(Copy, Clone)]
pub struct D {
    pub c1: C,
    pub c2: C,
    pub x: ::std::os::raw::c_char,
}
#[test]
fn bindgen_test_layout_D() {
    assert_eq!(
        ::std::mem::size_of::<D>(),
        128usize,
        concat!("Size of: ", stringify!(D))
    );
    assert_eq!(
        ::std::mem::align_of::<D>(),
        64usize,
        concat!("Alignment of ", stringify!(D))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<D>())).c1 as *const _ as usize },
        0usize,
        concat!("Offset of field: ", stringify!(D), "::", stringify!(c1))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<D>())).c2 as *const _ as usize },
        64usize,
        concat!("Offset of field: ", stringify!(D), "::", stringify!(c2))
    );
    assert_eq!(
        unsafe { &(*(::std::ptr::null::<D>())).x as *const _ as usize },
        67usize,
        concat!("Offset of field: ", stringify!(D), "::", stringify!(x))
    );
}

Most notably, one of the generated tests is a failure:

running 4 tests
test bindgen_test_layout_A ... ok
test bindgen_test_layout_B ... ok
test bindgen_test_layout_C ... ok
test bindgen_test_layout_D ... FAILED

failures:

---- bindgen_test_layout_D stdout ----
thread 'bindgen_test_layout_D' panicked at 'assertion failed: `(left == right)`
  left: `64`,
 right: `128`: Size of: D', src/lib.rs:94:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Looking it all of the test assertions, everything for A and B is consistent with GCC. For C, though, there's a discrepancy. The alignment of C is supposed to be 64, but bindgen is asserting that it's 1, and that test is passing. For D, all of the generated assertions agree with GCC, but as we saw above, the first one ends up failing. If we comment out D's failing assertions one by one, we can see that the last two also fail, because they expect the C members to be 64-byte aligned.

It's interesting that bindgen applies #[repr(align(64))] to B and D, rather than to A and C, as the C code does. It looks like it's part of a strategy to get all the alignments involved to work out, but it's missing some extra padding in D. (I do think that leaving C with the wrong alignment would lead to unsound behavior when creating arrays of C, however, regardless of whether D can be padded properly.)


Interestingly, bindgen does seem to be able to navigate a slightly different but closely related situation. If I redefine D to be:

typedef struct {
    char y1;
    C c;
    char y2;
} D;

Then it's Rust definition comes out like this:

#[repr(C)]
#[repr(align(64))]
#[derive(Copy, Clone)]
pub struct D {
    pub y1: ::std::os::raw::c_char,
    pub __bindgen_padding_0: [u64; 7usize],
    pub c: C,
    pub y2: ::std::os::raw::c_char,
}

Because of the extra padding inserted there, all of the generated tests pass! However, I do think there would still be issues with arrays of C, since they are not aligned properly without D's padding.


The code that actually triggers this issue in the real world is: https://github.com/XKCP/K12/blob/9585025bb031d15cbe3a699c364b63f519e964c4/lib/KangarooTwelve.h#L47-L54

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by compiling and running the reported align.c example with GCC, then compare the generated Rust bindings and layout tests shown in the issue. The real-world trigger is KangarooTwelve.h at lines 47-54; done means typedef-applied alignment, member offsets, sizes, and arrays are represented consistently without failing generated tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, rust
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.