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

Warning in bindgen: found non-foreign-function-safe member in struct marked #[repr(C)] #634

Closed
siscia opened this issue Apr 15, 2017 · 12 comments

Comments

@siscia
Copy link

siscia commented Apr 15, 2017

Input C/C++ Header

#include "src/CDeps/Redis/include/redismodule.h"
#include "src/CDeps/Redis/include/export_redismodule.h"

Bindgen Invokation

    #[derive(Debug)]
    struct SqliteTypeChooser;

    impl ParseCallbacks for SqliteTypeChooser {
        fn int_macro(&self, _name: &str, value: i64) -> Option<IntKind> {
            if value >= i32::min_value() as i64 &&
               value <= i32::max_value() as i64 {
                Some(IntKind::I32)
            } else {
                None
            }
        }
    }


    let bindings = bindgen::Builder::default()
        .no_unstable_rust()
        .parse_callbacks(Box::new(SqliteTypeChooser))
        .header("wrapper.h")
        .generate()
        .expect("Unable to generate bindings");

    let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
    bindings.write_to_file(out_path.join("bindings.rs"))
        .expect("Couldn't write bindings!");
}

Actual Results

and

extern "C" {
    pub fn fgetpos(__stream: *mut FILE, __pos: *mut fpos_t)
     -> ::std::os::raw::c_int;
}
extern "C" {
    pub fn fsetpos(__stream: *mut FILE, __pos: *const fpos_t)
     -> ::std::os::raw::c_int;
}
warning: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type, #[warn(improper_ctypes)] on by default
    --> /home/simo/rediSQL-rst/target/debug/build/rediSQL-rst-196441589fefb3b1/out/bindings.rs:5575:48
     |
5575 |     pub fn fgetpos(__stream: *mut FILE, __pos: *mut fpos_t)
     |                                                ^^^^^^^^^^^

warning: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type, #[warn(improper_ctypes)] on by default
    --> /home/simo/rediSQL-rst/target/debug/build/rediSQL-rst-196441589fefb3b1/out/bindings.rs:5579:48
     |
5579 |     pub fn fsetpos(__stream: *mut FILE, __pos: *const fpos_t)
     |                                                ^^^^^^^^^^^^^

-->

Expected Results

I wasn't expecting the warning generated.

RUST_LOG=bindgen Output

It actually didn't change anything, but I it may be me running it wrong.

The project is here: https://github.com/siscia/rediSQL-rs and the C dependencies are here: https://github.com/siscia/rediSQL-rs/tree/master/src/CDeps/Redis

For anything please just let me know :)

@emilio
Copy link
Contributor

emilio commented Apr 16, 2017

Could you post the definition of fpos_t? May it be an empty struct or something like that?

@siscia
Copy link
Author

siscia commented Apr 16, 2017

Hi,

Actually all the code I get is the one that you see. I have no idea of the definition of fpos_t however I assume it the one documented here: http://www.cplusplus.com/reference/cstdio/fpos_t/

Stuff from the stdio...

Sorry for this vague information but I really don't know how to help futher...

@emilio
Copy link
Contributor

emilio commented Apr 16, 2017

So I can't reproduce it on my system.

The warning points to a source file (in this case /home/simo/rediSQL-rst/target/debug/build/rediSQL-rst-196441589fefb3b1/out/bindings.rs). That file should contain the rust definition for fpos_t, could you paste that bit, or upload the file?

Thanks again! :)

@siscia
Copy link
Author

siscia commented Apr 16, 2017

Hi emilio,

what I got is here:

#[repr(C)]
#[derive(Debug, Copy)]
pub struct _G_fpos_t {
    pub __pos: __off_t,
    pub __state: __mbstate_t,
}
#[test]
fn bindgen_test_layout__G_fpos_t() {
    assert_eq!(::std::mem::size_of::<_G_fpos_t>() , 16usize , concat ! (
               "Size of: " , stringify ! ( _G_fpos_t ) ));
    assert_eq! (::std::mem::align_of::<_G_fpos_t>() , 8usize , concat ! (
                "Alignment of " , stringify ! ( _G_fpos_t ) ));
    assert_eq! (unsafe {
                & ( * ( 0 as * const _G_fpos_t ) ) . __pos as * const _ as
                usize } , 0usize , concat ! (
                "Alignment of field: " , stringify ! ( _G_fpos_t ) , "::" ,
                stringify ! ( __pos ) ));
    assert_eq! (unsafe {
                & ( * ( 0 as * const _G_fpos_t ) ) . __state as * const _ as
                usize } , 8usize , concat ! (
                "Alignment of field: " , stringify ! ( _G_fpos_t ) , "::" ,
                stringify ! ( __state ) ));
}
impl Clone for _G_fpos_t {
    fn clone(&self) -> Self { *self }
}
#[repr(C)]
#[derive(Debug, Copy)]
pub struct _G_fpos64_t {
    pub __pos: __off64_t,
    pub __state: __mbstate_t,
}

...

pub type fpos_t = _G_fpos_t;

I looked inside the source file of Redis and there is nothing like fpos_t however it includes stdio.h

However let me guess that a lot of other projects include stdio.h

Did you try to fork my repo and reproduce the issues or simply recreate the same environment give the indication that I provide to you?

@emilio
Copy link
Contributor

emilio commented Apr 17, 2017

Did you try to fork my repo and reproduce the issues or simply recreate the same environment give the indication that I provide to you?

I forked and built, but couldn't repro. This is inside system headers so that's somewhat expected I guess.

Could you post the definitions of __off64_t and __mbstate_t?

@dimbleby
Copy link
Contributor

duplicate #442?

@emilio
Copy link
Contributor

emilio commented Apr 17, 2017

Maybe if the rustc version the reporter is compiling with doesn't contain rust-lang/rust#39462?

But even with that, I don't see off-hand a reason those types should have PhantomData. Oh, perhaps __mbstate_t is/contains a union? That'd explain it I guess.

@siscia
Copy link
Author

siscia commented Apr 17, 2017

Hi,

here are the definitions:

pub type __off_t = ::std::os::raw::c_long;
pub type __off64_t = ::std::os::raw::c_long;

...

#[repr(C)]
#[derive(Debug, Copy)]
pub struct __mbstate_t {
    pub __count: ::std::os::raw::c_int,
    pub __value: __mbstate_t__bindgen_ty_1,
}
#[repr(C)]
#[derive(Debug, Copy)]
pub struct __mbstate_t__bindgen_ty_1 {
    pub __wch: __BindgenUnionField<::std::os::raw::c_uint>,
    pub __wchb: __BindgenUnionField<[::std::os::raw::c_char; 4usize]>,
    pub bindgen_union_field: u32,
}

Is this enough? I see primitive so I would rule out that the structs are empty.

Also, let me post the whole file in this gist

@siscia
Copy link
Author

siscia commented Apr 17, 2017

Also, I am running/compiling in ubuntu 16.04 with:

$ cargo -V
cargo-0.17.0-nightly (f9e5481 2017-03-03)
$ rustc -V
rustc 1.16.0 (30cf806ef 2017-03-10)

@emilio
Copy link
Contributor

emilio commented Apr 18, 2017

Hmm, interesting. So it seems definitely like a dupe of #442. Given the date you should have the fix in tree though, could you check just in case that updating rustc still warns?

@dimbleby
Copy link
Contributor

I don't believe that the compiler fix is in stable (1.16.0) rust yet.

@emilio
Copy link
Contributor

emilio commented Apr 22, 2017

That's right, closing this as a dupe of #442. Thanks both :)

@emilio emilio closed this as completed Apr 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants