Skip to content

Incorrect field offset when using #[repr(C, packed(8))] #128373

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

Closed
kurtmcmillan opened this issue Jul 29, 2024 · 4 comments
Closed

Incorrect field offset when using #[repr(C, packed(8))] #128373

kurtmcmillan opened this issue Jul 29, 2024 · 4 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR.

Comments

@kurtmcmillan
Copy link

I'm using bindgen to create bindings for a library. It appears that rustc is not packing the structs in the same way as clang/gcc.

struct foo
{
    uint64_t a;
    uint32_t b;
    uint32_t c;
    uint32_t d;
    uint64_t e;
} __attribute__((packed, aligned(8)));

bindgen generates the following:

#[repr(C, packed(8))]
#[derive(Debug, Copy, Clone)]
pub struct foo {
    pub a: u64,
    pub b: u32,
    pub c: u32,
    pub d: u32,
    pub e: u64,
}

I expected to see this happen:

The bindgen tests pass. The e field is at offset 20.

Instead, this happened:

The bindgen tests fail. The e field is at offset 24.

fn bindgen_test_layout_foo() {
    const UNINIT: ::std::mem::MaybeUninit<foo> = ::std::mem::MaybeUninit::uninit();
    let ptr = UNINIT.as_ptr();
    // ... snip ...
    assert_eq!(
        unsafe { ::std::ptr::addr_of!((*ptr).e) as usize - ptr as usize },
        20usize,
        concat!("Offset of field: ", stringify!(foo), "::", stringify!(e))
    );
}
cargo test
... snip ...
---- bindings::bindgen_test_layout_foo stdout ----
thread 'bindings::bindgen_test_layout_foo' panicked at /home/kumcmill/bindgen/target/debug/build/bindgen-bbd2a2691349bec7/out/bindings.rs:246:5:
assertion `left == right` failed: Offset of field: foo::e
  left: 24
 right: 20

Reproduced with a smaller example:

struct bar
{
    uint32_t f;
    uint64_t g;
} __attribute__((packed, aligned(8)));

bindgen generates the following:

#[repr(C, packed(8))]
#[derive(Debug, Copy, Clone)]
pub struct bar {
    pub f: u32,
    pub g: u64,
}

I expected to see this happen:

The bindgen tests pass. The g field is at offset 4.

Instead, this happened:

The bindgen tests fail. The g field is at offset 8.

#[test]
fn bindgen_test_layout_bar() {
    const UNINIT: ::std::mem::MaybeUninit<bar> = ::std::mem::MaybeUninit::uninit();
    let ptr = UNINIT.as_ptr();
    // ... snip ...
    assert_eq!(
        unsafe { ::std::ptr::addr_of!((*ptr).e) as usize - ptr as usize },
        4usize,
        concat!("Offset of field: ", stringify!(bar), "::", stringify!(e))
    );
}
cargo test
... snip ...
---- bindings::bindgen_test_layout_bar stdout ----
thread 'bindings::bindgen_test_layout_bar' panicked at /home/kumcmill/bindgen/target/debug/build/bindgen-bbd2a2691349bec7/out/bindings.rs:277:5:
assertion `left == right` failed: Offset of field: bar::e
  left: 8
 right: 4

With gcc and clang:

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

struct foo
{
    uint64_t a;
    uint32_t b;
    uint32_t c;
    uint32_t d;
    uint64_t e;
} __attribute__((packed, aligned(8)));

struct bar
{
    uint32_t f;
    uint64_t g;
} __attribute__((packed, aligned(8)));

int main() {
    printf("offset of foo.e: %lu\n", offsetof(struct foo, e));
    printf("offset of bar.g: %lu\n", offsetof(struct bar, g));
}

clang:

$ clang --version
clang version 10.0.0-4ubuntu1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

$ clang main.c -o main && ./main
offset of foo.e: 20
offset of bar.g: 4

gcc:

$ gcc --version
gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc main.c -o main && ./main
offset of foo.e: 20
offset of bar.g: 4

Meta

rustc --version --verbose:

$ rustc --version --verbose
rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: x86_64-unknown-linux-gnu
release: 1.79.0
@kurtmcmillan kurtmcmillan added the C-bug Category: This is a bug. label Jul 29, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 29, 2024
@wwylele
Copy link
Contributor

wwylele commented Jul 29, 2024

It seems that gcc's (packed, aligned(N)) and rust's packed(N) have different meaning

gcc's (packed, aligned(N)) on types
https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html

The aligned attribute specifies a minimum alignment (in bytes) for variables of the specified type.

In other words, it only affects the starting address of the struct, not the layout of the members

rust's packed(N)
https://doc.rust-lang.org/reference/type-layout.html

For packed, if the specified alignment is greater than the type's alignment without the packed modifier, then the alignment and layout is unaffected. The alignments of each field, for the purpose of positioning fields, is the smaller of the specified alignment and the alignment of the field's type. Inter-field padding is guaranteed to be the minimum required in order to satisfy each field's (possibly altered) alignment (although note that, on its own, packed does not provide any guarantee about field ordering). An important consequence of these rules is that a type with #[repr(packed(1))] (or #[repr(packed)]) will have no inter-field padding.

In other words, rust's specifier adjusts the layout of the members to satisfy the alignment specified by packed

@ChrisDenton
Copy link
Member

This seems like a duplicate of #59154.

The rust equivalent would be #[repr(C, packed, aligned(8))] but that isn't legal. So you'd need to wrap a packed inner struct with an align(8) outer struct,

@workingjubilee
Copy link
Member

Closing as duplicate. If these are auto-generated tests from bindgen, then please report the test bug(?) to rust-bindgen, as bindgen should either

  • not test for this
  • emit an error explaining the situation
  • generate correct bindings using some malefic hack

@workingjubilee workingjubilee closed this as not planned Won't fix, can't repro, duplicate, stale Jul 30, 2024
@workingjubilee
Copy link
Member

Probably the last one.

@saethlin saethlin added C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR.
Projects
None yet
Development

No branches or pull requests

6 participants