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

structure layout using __aligned__ attribute is incorrect #867

Open
toshipp opened this issue Jul 29, 2017 · 4 comments
Open

structure layout using __aligned__ attribute is incorrect #867

toshipp opened this issue Jul 29, 2017 · 4 comments

Comments

@toshipp
Copy link

toshipp commented Jul 29, 2017

Input C/C++ Header

The original header is linux kernel source header; linux/target_core_user.h.

#define ALIGN_SIZE 64 /* Should be enough for most CPUs */

typedef unsigned short __u16;
typedef unsigned int __u32;

struct tcmu_mailbox {
	__u16 version;
	__u16 flags;
	__u32 cmdr_off;
	__u32 cmdr_size;

	__u32 cmd_head;

	/* Updated by user. On its own cacheline */
	__u32 cmd_tail __attribute__((__aligned__(ALIGN_SIZE)));

} __attribute__((packed));

Bindgen Invocation

$ bindgen input.h

Actual Results

pub type __u16 = ::std::os::raw::c_ushort;
pub type __u32 = ::std::os::raw::c_uint;
#[repr(C, packed)]
#[derive(Copy)]
pub struct tcmu_mailbox {
    pub version: __u16,
    pub flags: __u16,
    pub cmdr_off: __u32,
    pub cmdr_size: __u32,
    pub cmd_head: __u32,
    pub cmd_tail: __u32,
    pub __bindgen_padding_0: [u8; 108usize],
}

Expected Results

pub type __u16 = ::std::os::raw::c_ushort;
pub type __u32 = ::std::os::raw::c_uint;
#[repr(C, packed)]
#[derive(Copy)]
pub struct tcmu_mailbox {
    pub version: __u16,
    pub flags: __u16,
    pub cmdr_off: __u32,
    pub cmdr_size: __u32,
    pub cmd_head: __u32,
    pub __bindgen_padding_0: [u8; 48usize],
    pub cmd_tail: __u32,
}

In C, the offset of cmd_tail is 64, so generated rust code should also have same offset.

To check offset, I wrote C code like as;

#include <stddef.h>
#include <stdio.h>
#include "mini.h"

int main(int n, char**v) {
    printf("version %ld\n", offsetof(struct tcmu_mailbox, version));
    printf("flags %ld\n", offsetof(struct tcmu_mailbox, flags));
    printf("cmdr_off %ld\n", offsetof(struct tcmu_mailbox, cmdr_off));
    printf("cmdr_size %ld\n", offsetof(struct tcmu_mailbox, cmdr_size));
    printf("cmd_head %ld\n", offsetof(struct tcmu_mailbox, cmd_head));
    printf("cmd_tail %ld\n", offsetof(struct tcmu_mailbox, cmd_tail));
}

The outpu is;

version 0
flags 2
cmdr_off 4
cmdr_size 8
cmd_head 12
cmd_tail 64
@emilio
Copy link
Contributor

emilio commented Jul 29, 2017

Thanks a lot for the report :)

@kawamuray
Copy link

FYI this issue seems to have solved with newer clang/llvm version.

I was trying to bindgen for some linux kernel header files like https://github.com/torvalds/linux/blob/master/include/uapi/linux/taskstats.h#L41 and hit the same issue, but on different machine with newer clang/llvm version the padding field is inserted in correct place and it worked well.

The older clang and llvm version is indeed lower than the recommended version at: https://rust-lang.github.io/rust-bindgen/requirements.html#clang

With old clang and llvm:

$ clang --version 
clang version 3.4.2 (tags/RELEASE_34/dot2-final)
$ llvm-config --version 
3.4.2

Generated:

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct taskstats {
(snip)
    pub ac_pad: [__u8; 3usize],
    pub ac_uid: __u32,
(snip)
    pub freepages_delay_total: __u64,
    pub __bindgen_padding_0: u64,
}

With new clang and llvm:

$ clang --version
clang version 7.0.1-8 (tags/RELEASE_701/final)
$ llvm-config --version
7.0.1

Generated:

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct taskstats {
(snip)
    pub ac_pad: [__u8; 3usize],
    pub __bindgen_padding_0: u32,
    pub ac_uid: __u32,
(snip)
    pub freepages_delay_total: __u64,
}

@toshipp
Copy link
Author

toshipp commented May 1, 2020

Thank you for your information.
I checked my current environment, my clang version is 10.0.0.

clang --version
clang version 10.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/sbin

I try to generate rust code from the header with bindgen 0.53.2.
I got following code.

pub const ALIGN_SIZE: u32 = 64;
pub type __u16 = ::std::os::raw::c_ushort;
pub type __u32 = ::std::os::raw::c_uint;
#[repr(C, packed(64))]
#[repr(align(64))]
#[derive(Copy, Clone)]
pub struct tcmu_mailbox {
    pub version: __u16,
    pub flags: __u16,
    pub cmdr_off: __u32,
    pub cmdr_size: __u32,
    pub cmd_head: __u32,
    pub cmd_tail: __u32,
    pub __bindgen_padding_0: [u8; 108usize],
}
<snip test code>

It seems works well, but it can not be compiled.

$ rustc -o foo --test foo.rs
warning: type `__u16` should have an upper camel case name
 --> foo.rs:4:10
  |
4 | pub type __u16 = ::std::os::raw::c_ushort;
  |          ^^^^^ help: convert the identifier to upper camel case: `U16`
  |
  = note: `#[warn(non_camel_case_types)]` on by default

warning: type `__u32` should have an upper camel case name
 --> foo.rs:5:10
  |
5 | pub type __u32 = ::std::os::raw::c_uint;
  |          ^^^^^ help: convert the identifier to upper camel case: `U32`

error[E0587]: type has conflicting packed and align representation hints
  --> foo.rs:9:1
   |
9  | / pub struct tcmu_mailbox {
10 | |     pub version: __u16,
11 | |     pub flags: __u16,
12 | |     pub cmdr_off: __u32,
...  |
16 | |     pub __bindgen_padding_0: [u8; 108usize],
17 | | }
   | |_^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0587`.

@pvdrz
Copy link
Contributor

pvdrz commented Sep 19, 2022

This became an instance of #1538 if I understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants