android system headers -> cyclic type aliases

[Dushistov]

Input C/C++ Header

bitmap.h

//from include/machine/_types.h from sys/_types.h from stdint.h
typedef __builtin_va_list       __va_list;

Bindgen Invocation

$ bindgen --no-layout-tests bitmap.h -- --target=arm-linux-androideabi

Actual Results

/* automatically generated by rust-bindgen */

pub type __va_list = __builtin_va_list;
pub type __builtin_va_list = __va_list;
#[repr(C)]
#[derive(Debug, Copy)]
pub struct __va_list {
    pub __ap: *mut ::std::os::raw::c_void,
}
impl Clone for __va_list {
    fn clone(&self) -> Self { *self }
}

Expected Results

Code that possible to compile with rustc without cyclic type aliases.

[Dushistov]

Side note clang can interpret this code clang --target=android-arm-androideabi -c bitmap.h works without errors.

But indeed it produces ast similar to bindgen generate:

$ clang -cc1  -triple arm-linux-androideabi -ast-dump bitmap.h 
TranslationUnitDecl 0x55d77e8968d0 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x55d77e897088 <<invalid sloc>> <invalid sloc> implicit __NSConstantString 'struct __NSConstantString_tag'
| `-RecordType 0x55d77e896e90 'struct __NSConstantString_tag'
|   `-Record 0x55d77e896e08 '__NSConstantString_tag'
|-TypedefDecl 0x55d77e897220 <<invalid sloc>> <invalid sloc> implicit referenced __builtin_va_list 'struct __va_list'
| `-RecordType 0x55d77e897160 'struct __va_list'
|   `-Record 0x55d77e8970d8 '__va_list'
`-TypedefDecl 0x55d77e8972a0 <bitmap.h:1:1, col:27> col:27 __va_list '__builtin_va_list':'struct __va_list'
  `-TypedefType 0x55d77e897270 '__builtin_va_list' sugar
    |-Typedef 0x55d77e897220 '__builtin_va_list'
    `-RecordType 0x55d77e897160 'struct __va_list'
      `-Record 0x55d77e8970d8 '__va_list'

So may be it has some heuristics to solve such cases.

[fitzgen]

Wow... I wonder if we should treat all __builtin_* types as opaque by default?

[fitzgen]

For posterity / anyone wondering, yes the --target=arm-linux-androideabi is necessary to reproduce the bug. When targeting x86_64-unknown-linux-gnu it does the right thing for me.

[Dushistov]

@fitzgen

I wonder if we should treat all _builtin* types as opaque by default?

But how this helps?

$ bindgen --opaque-type __builtin_va_list --no-layout-tests bitmap.h -- --target=arm-linux-androideabi
/* automatically generated by rust-bindgen */

pub type __va_list = __builtin_va_list;
pub type __builtin_va_list = u32;
#[repr(C)]
#[derive(Debug, Copy)]
pub struct __va_list {
    pub __ap: *mut ::std::os::raw::c_void,
}
impl Clone for __va_list {
    fn clone(&self) -> Self { *self }
}

As you can see __va_list defined two times, so opaque not helps,

or you mean to treat all compiler's internal identifiers as opaque by default:

ISO 9899:2011
7.1.3 Reserved identifiers
All identifiers that begin with an underscore and either an uppercase letter or another underscore are > always reserved for any use.

so __va_list is internal and may be treated as opaque?

yes the --target=arm-linux-androideabi is necessary to reproduce the bug

Also aarch64-linux-android may be used to reproduce bug,

By the way aarch64-apple-ios target cause crash:

$ RUST_BACKTRACE=1 bindgen --no-layout-tests bitmap.h -- --target=aarch64-apple-ios
thread 'main' panicked at 'TranslationUnit::parse failed', /checkout/src/libcore/option.rs:819:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:497
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:92
   9: core::option::expect_failed
             at /checkout/src/libcore/option.rs:819
  10: bindgen::ir::context::BindgenContext::new
  11: bindgen::Bindings::generate
  12: bindgen::Builder::generate
  13: std::panicking::try::do_call
  14: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  15: bindgen::main
  16: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  17: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:458
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:59
  18: __libc_start_main
  19: _start

[Dushistov]

@fitzgen

May be problem is NOT in this code:

//from include/machine/_types.h from sys/_types.h from stdint.h
typedef __builtin_va_list       __va_list;

but in the fact that bindgen uses not clang headers when see

#include <stdint.h>
#include <stdarg.h>

?

My problem in code from stdint.h that comes with Android NDK,
which part of gcc. But there is also stdint.h that part of clang installation.

Why bindgen not use for headers, that tightly coupled with compiler clang's one headers?

[Dushistov]

I've tried bindgen 0.32.3, and looks like #953 completly brake bindgen for android,
at now bindgen generates not valid rustc code.

Now:

pub type __gnuc_va_list = __builtin_va_list;
pub type va_list = __gnuc_va_list;
pub type __va_list = __builtin_va_list;
pub type __builtin_va_list = __va_list;
pub struct __va_list {

So multiply definitions of __va_list.

Before:

pub type __gnuc_va_list = __builtin_va_list;
pub type va_list = __gnuc_va_list;
pub type __va_list = __builtin_va_list;
pub type __builtin_va_list = [__va_list_tag; 1usize];
pub struct __va_list_tag {

[emilio]

Could you try to get a preprocessed input header with Builder::dump_preprocessed_input? That way I can investigate it. Thanks!

[Dushistov]

@emilio

I've attached file. But why you need it?

There is reduced test case in the first message:

$ cat test.h
typedef __builtin_va_list       __va_list;

you need to run bindgen with --target=arm-linux-androideabi to see problem.

__bindgen.i.gz

[emilio]

Ok so here's a more generic test-case which works independently of the platform:

struct foo { };

typedef struct foo bar;

typedef bar foo;

[emilio]

I guess we need to expand the typedef struct foo {} foo; checks to work across other typedefs.

[emilio]

#1239