Auto-generated test failure using <link.h> on Debian 9.

[vext01]

Input C/C++ Header

Sorry, I know you asked for no #includes, but I'm at a loss as to what the predicate script should do when I'm just importing types from the C library -- If you can't reproduce from the info below, then could you help me with the predicate script plz? :)

#define _GNU_SOURCE
#include <link.h>

Bindgen Invocation

    let bindings = bindgen::Builder::default()
                           .header(WRAPPER_HEADER)
                           .generate()
                           .expect("bindgen failed");

Actual Results

When included, like this:

#![allow(non_upper_case_globals)]
#![allow(non_camel_case_types)]
#![allow(non_snake_case)]
#![allow(dead_code)]
include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

$ RUST_BACKTRACE=1 cargo test bindgen_test_layout_La_x86_64_retval
---- bindgen_test_layout_La_x86_64_retval stdout ----
thread 'bindgen_test_layout_La_x86_64_retval' panicked at 'assertion failed: `(left == right)`
  left: `224`,
 right: `240`: Size of: La_x86_64_retval', /home/vext01/research/bg/target/debug/build/bg-29903e2596eaef66/out/bindings.rs:3:221231
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:221
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:475
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   6: std::panicking::begin_panic_fmt
             at libstd/panicking.rs:345
   7: bg::bindgen_test_layout_La_x86_64_retval
             at ./target/debug/build/bg-29903e2596eaef66/out/bindings.rs:3
   8: bg::__test::TESTS::{{closure}}
             at ./target/debug/build/bg-29903e2596eaef66/out/bindings.rs:3
   9: core::ops::function::FnOnce::call_once
             at /checkout/src/libcore/ops/function.rs:223
  10: <F as alloc::boxed::FnBox<A>>::call_box
             at libtest/lib.rs:1459
             at /checkout/src/libcore/ops/function.rs:223
             at /checkout/src/liballoc/boxed.rs:642
  11: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102

This is the only failing test.

Expected Results

Test passes.

This is on Debian 9 x86_64, using today's Rust nightly.

FWIW, OpenBSD's <link.h> does work on an OpenBSD system.

Any ideas?

[emilio]

Is there any chance of knowing the preprocessed input? Otherwise it's going to be hard to figure out.

See: https://github.com/rust-lang-nursery/rust-bindgen/blob/master/CONTRIBUTING.md#isolating-your-test-case

If you feel like trying creduce to reduce it it'd be awesome as well!

[emilio]

Sorry, I know you asked for no #includes, but I'm at a loss as to what the predicate script should do when I'm just importing types from the C library -- If you can't reproduce from the info below, then could you help me with the predicate script plz? :)

Oh, the predicate script is only for reducing it. The .i file would still be really helpful to repro it locally.

[vext01]

Gzipped .i file attached. Hopefully that helps.

__bindgen.i.gz

[vext01]

Any joy?

Thanks

[emilio]

Sorry, I haven't had the chance to look at this in detail yet.

[vext01]

Hi emilio,

How does one go about debugging this test failure? I see the test in bindings.rs, but it's large and all on one line. Is there a way to get this in a more human-readable format?

We have a PR blocked on this issue, so I'm keen to push on.

Thanks

[emilio]

If you have rustfmt installed you should get the bindings properly formatted.

[vext01]

I've made a little progress.

thread 'p_ffi::bindgen_test_layout_La_x86_64_retval' panicked at 'assertion failed: `(left == right)`
  left: `224`,
 right: `240`: Size of: La_x86_64_retval'

Rust expects the La_x86_64_retval struct to be of size 240, but C expects it to be of size 224.

The Rust definition (and the types it depends on) look like this:

pub type La_x86_64_xmm = [f32; 4usize]; // 4 * 4        16
pub type La_x86_64_ymm = [f32; 8usize]; // 4 * 8        32
pub type La_x86_64_zmm = [f64; 8usize]; // 8 * 8        64

#[repr(C)]
#[derive(Copy, Clone)]
pub union La_x86_64_vector {
    pub ymm: [La_x86_64_ymm; 2usize],  // 32 * 2        64
    pub zmm: [La_x86_64_zmm; 1usize],  // 64 * 1        64
    pub xmm: [La_x86_64_xmm; 4usize],  // 16 * 4        64
    _bindgen_union_align: [u8; 64usize],// 1 * 64       64
}
// ^ 64 bytes

#[repr(C)]
#[derive(Copy, Clone)]
pub struct La_x86_64_retval {
    pub lrv_rax: u64,               // 8
    pub lrv_rdx: u64,               // 8
    pub lrv_xmm0: La_x86_64_xmm,    // 16
    pub lrv_xmm1: La_x86_64_xmm,    // 16
    pub lrv_st0: f64,               // 8
    pub lrv_st1: f64,               // 8
    pub lrv_vector0: La_x86_64_vector, // 64
    pub lrv_vector1: La_x86_64_vector, // 64
    pub lrv_bnd0: __int128_t,        // 16
    pub lrv_bnd1: __int128_t,        // 16
}
// ^ 224 bytes

I've annotated the sizes of things using comments, and determined that this lot is of size 224.

The C definition looks like this:

typedef union
{
# if __GNUC_PREREQ (4,0)
  La_x86_64_ymm ymm[2];
  La_x86_64_zmm zmm[1];
# endif
  La_x86_64_xmm xmm[4];
} La_x86_64_vector __attribute__ ((__aligned__ (16)));

/* Registers for entry into PLT on x86-64.  */
# if __GNUC_PREREQ (4,0)
typedef float La_x86_64_xmm __attribute__ ((__vector_size__ (16)));
typedef float La_x86_64_ymm
    __attribute__ ((__vector_size__ (32), __aligned__ (16)));
typedef double La_x86_64_zmm
    __attribute__ ((__vector_size__ (64), __aligned__ (16)));
# else
typedef float La_x86_64_xmm __attribute__ ((__mode__ (__V4SF__)));
# endif


typedef struct La_x86_64_retval
{
  uint64_t lrv_rax;
  uint64_t lrv_rdx;
  La_x86_64_xmm lrv_xmm0;
  La_x86_64_xmm lrv_xmm1;
  long double lrv_st0;
  long double lrv_st1;
  La_x86_64_vector lrv_vector0;
  La_x86_64_vector lrv_vector1;
#ifndef __ILP32__
  __int128_t lrv_bnd0;
  __int128_t lrv_bnd1;
#endif
} La_x86_64_retval;

I think bindgen is not dealing with the conditionally compiled fields properly. The size of these structs depends on macros:

$ cat a.c
#include <stdio.h>
#include <stdlib.h>
#include <link.h>

int
main(int argc, char **argv)
{
    printf("size=%lu\n", sizeof(struct La_x86_64_retval));
    return (EXIT_SUCCESS);
}
$ cc -D__ILP32__ a.c && ./a.out
size=208
$ cc a.c && ./a.out
size=240

(oddly enough, not 224 in either case, probably more macros)

Does any of this help?

[vext01]

I think bindgen is not dealing with the conditionally compiled fields properly.

Actually, that statement can't be right.

Any ideas?

[vext01]

Here's the real problem:

long double lrv_st0;

becomes:

pub lrv_st0: f64,

Which I think is incorrect.

On my system, a long double is 16 bytes long, whereas an f64 is 8, and this messes up the size of the struct.

Related: #550

[emilio]

Ahá

[emilio]

We use f64 because rust didn't have f128 at the time and u128 and such were unstable...

[emilio]

We could generate u128 conditionally there now, I'd think...

[vext01]

Hey Emilio, Sounds good, but why conditionally? Cheers

…

On Tue, 18 Sep 2018, 19:22 Emilio Cobos Álvarez, ***@***.***> wrote: We could generate u128 conditionally there now, I'd think... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1370 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAk7G9DTxKWeLMS46YKJS9BnW2nsEbltks5ucTntgaJpZM4WGCB7> .

[emilio]

Because we want to support old rust versions, so it'd need to be behind a rust_target check.

[emilio]

I took a bit of spare time to implement the first step to properly do that: #1391. That fixes integer generation. Then I need to teach the rest of the codegen to conditionally support that, but shouldn't be terrible.

[emilio]

I ended up doing the whole thing :)

[emilio]

Note that there's an unexpected gotcha here: rust-lang/rust#54341.

[vext01]

Hi emilio,

I ended up doing the whole thing :)

Wow thanks. I'll give it a go. Looks like this branch:
https://github.com/emilio/rust-bindgen/commits/u128

Note that there's an unexpected gotcha here

Whilst I don't think this will affect my test case, I think it should be fixed. Having things randomly crash because of incorrect alignment aint good. It aint Rust :)

Ideally, Rust (or the libc crate) would have a long_double type which aligns properly. How likely is it that we could convince them?

[vext01]

FWIW, the test still fails on that branch.

[emilio]

Have you enabled rust_target(Rust_Target::Stable_1_26)? Otherwise behavior should be unchanged.

[emilio]

Ideally, Rust (or the libc crate) would have a long_double type which aligns properly. How likely is it that we could convince them?

I think I did file an issue on this long time ago... But I can't find it now. It's worth a shot probably :)

[vext01]

Have you enabled rust_target(Rust_Target::Stable_1_26)?

Um, no. I thought nightly would be enough. Let me give it a go.

[vext01]

Right, so this makes a difference:

---- p_ffi::bindgen_test_layout_La_x86_64_retval stdout ----
thread 'p_ffi::bindgen_test_layout_La_x86_64_retval' panicked at 'assertion failed: `(left == right)`
  left: `8`,
 right: `16`: Alignment of La_x86_64_retval', /home/vext01/research/phdrs_pv/target/debug/build/phdrs-bbf2d34987b05535/out/bindings.rs:7273:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- p_ffi::bindgen_test_layout_La_x86_64_vector stdout ----
thread 'p_ffi::bindgen_test_layout_La_x86_64_vector' panicked at 'assertion failed: `(left == right)`
  left: `8`,
 right: `16`: Alignment of La_x86_64_vector', /home/vext01/research/phdrs_pv/target/debug/build/phdrs-bbf2d34987b05535/out/bindings.rs:7077:5

[emilio]

I pushed a commit that should fix those. It's a bit of a work-around, but it only improves correctness so I think we should land it.

[emilio]

And rustc already warns when using u128 as a raw type via FFI, so I think that should close this.

warning: extern block uses type u128 which is not FFI-safe: 128-bit integers don't currently have a known stable ABI

[vext01]

Hi emilio,

So should my tests pass now? Using the latest version of your u128 branch, I still have one test failure:

---- p_ffi::bindgen_test_layout_La_x86_64_vector stdout ----
thread 'p_ffi::bindgen_test_layout_La_x86_64_vector' panicked at 'assertion failed: `(left == right)`
  left: `8`,
 right: `16`: Alignment of La_x86_64_vector', /home/vext01/research/phdrs_pv/target/debug/build/phdrs-766b5f3742f78b0d/out/bindings.rs:7077:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

[emilio]

Oh, interesting, that is not a struct but a union with alignment attributes. Mind filing a new issue for that one? I'm not on my laptop now but I think it should be pretty easy to create a reduced test-case.

[vext01]

Sure, I'll raise another issue.

[vext01]

Hi emilio,

Quick question. For how long will I need the .rust_target(bindgen::RustTarget::Stable_1_26) hack in build.rs?

Thanks

[emilio]

It's not really a hack, it's there so bindgen will generate repr(align) attributes. We could deprecate support for older rust versions I guess, and make one version the default from now on, but it's not clear what version should that be.

[vext01]

Maybe this is not possible, but wouldn't you just want something which works with whatever version of rust the user is currently running?

[emilio]

Not really, because the version you run bindgen for may or may not be the version you compile the bindings for. And even if it is, bindgen output should be deterministic at least for a given architecture, or, let's say, as deterministic as the C code you're building.

Otherwise imagine you develop a library using bindgen of a new compiler, and bindgen generates unions for you.

Then you have an user that builds that library with an old rustc compiler, and the code you wrote using the unions from bindgen no longer builds. That's not great.