From cc78b6fdb6e829e5fb8fa1639f2182cb49333569 Mon Sep 17 00:00:00 2001 From: Geoffrey Thomas Date: Thu, 3 Jun 2021 10:47:26 -0400 Subject: [PATCH] Map size_t to usize by default and check compatibility (fixes #1901, #1903) This addresses the underlying issue identified in #1671, that size_t (integer that can hold any object size) isn't guaranteed to match usize, which is defined more like uintptr_t (integer that can hold any pointer). However, on almost all platforms, this is true, and in fact Rust already uses usize extensively in contexts where size_t would be more appropriate, such as slice indexing. So, it's better for ergonomics when interfacing with C code to map the C size_t type to usize. (See also discussion in rust-lang/rust#65473 about how usize really should be defined as size_t, not uintptr_t.) The previous fix for #1671 removed the special case for size_t and defaulted to binding it as a normal typedef. This change effectively reverts that and goes back to mapping size_t to usize (and ssize_t to isize), but also ensures that if size_t is emitted, the typedef'd type of size_t in fact is compatible with usize (defined by checking that the size and alignment match the target pointer width). For (hypothetical) platforms where this is not true, or for compatibility with the default behavior of bindgen between 0.53 and this commit, onwards, you can disable this mapping with --no-size_t-is-usize. --- asd.fish | 31 +++++++++++++++++++ src/codegen/mod.rs | 27 +++++++++++++++- src/lib.rs | 6 ++-- src/options.rs | 11 +++++-- tests/expectations/tests/blocks-signature.rs | 12 ++----- tests/expectations/tests/blocks.rs | 1 - tests/expectations/tests/issue-1498.rs | 3 +- .../expectations/tests/jsval_layout_opaque.rs | 3 +- .../tests/jsval_layout_opaque_1_0.rs | 3 +- tests/expectations/tests/layout_array.rs | 3 +- tests/expectations/tests/msvc-no-usr.rs | 3 +- ...ze_t_is_usize.rs => no_size_t_is_usize.rs} | 6 ++-- tests/expectations/tests/nsBaseHashtable.rs | 1 - ...size_t_is_usize.h => no_size_t_is_usize.h} | 2 +- 14 files changed, 81 insertions(+), 31 deletions(-) create mode 100644 asd.fish rename tests/expectations/tests/{size_t_is_usize.rs => no_size_t_is_usize.rs} (91%) rename tests/headers/{size_t_is_usize.h => no_size_t_is_usize.h} (75%) diff --git a/asd.fish b/asd.fish new file mode 100644 index 0000000000..b6c9a270e7 --- /dev/null +++ b/asd.fish @@ -0,0 +1,31 @@ +for name in (rg -lF addr_of! tests/expectations | sd '.*/(.*).rs' '$1') + set path (fd --glob "$name.*" tests/headers) + if test -n "$path" + + set flags (rg -F "// bindgen-flags" $path) + if test -n "$flags" + set minor (rg ".*\-\-rust\-target[ =]1.(\d+).*" $path -r '$1') + if test -n "$minor" + if test $minor -gt 47 + echo $path needs to change the version from 1.$minor to 1.47 + sd -s "1.$minor" "1.47" $path + else + echo $path uses version 1.$minor and that is fine + end + else + echo $path does not have the `--rust-target` flag + sd "// bindgen-flags: (.*)" '// bindgen-flags: --rust-target 1.47 $1' $path + end + else + echo $path does not have the flags at all + set contents (echo -e "// bindgen-flags: --rust-target 1.47\n"; cat $path) + rm $path + touch $path + for line in $contents + echo $line >> $path + end + end + else + echo $name headers not found + end +end diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 843d5111a1..8eb7b0134c 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -836,9 +836,34 @@ impl CodeGenerator for Type { } // If this is a known named type, disallow generating anything - // for it too. + // for it too. If size_t -> usize conversions are enabled, we + // need to check that these conversions are permissible, but + // nothing needs to be generated, still. let spelling = self.name().expect("Unnamed alias?"); if utils::type_from_named(ctx, spelling).is_some() { + if let "size_t" | "ssize_t" = spelling { + let layout = inner_item + .kind() + .expect_type() + .layout(ctx) + .expect("No layout?"); + assert_eq!( + layout.size, + ctx.target_pointer_size(), + "Target platform requires `--no-size_t-is-usize`. The size of `{}` ({}) does not match the target pointer size ({})", + spelling, + layout.size, + ctx.target_pointer_size(), + ); + assert_eq!( + layout.align, + ctx.target_pointer_size(), + "Target platform requires `--no-size_t-is-usize`. The alignment of `{}` ({}) does not match the target pointer size ({})", + spelling, + layout.align, + ctx.target_pointer_size(), + ); + } return; } diff --git a/src/lib.rs b/src/lib.rs index 3c89368f11..9a5887df8e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -564,8 +564,8 @@ impl Builder { output_vector.push("--no-record-matches".into()); } - if self.options.size_t_is_usize { - output_vector.push("--size_t-is-usize".into()); + if !self.options.size_t_is_usize { + output_vector.push("--no-size_t-is-usize".into()); } if !self.options.rustfmt_bindings { @@ -2253,7 +2253,7 @@ impl Default for BindgenOptions { time_phases: false, record_matches: true, rustfmt_bindings: true, - size_t_is_usize: false, + size_t_is_usize: true, rustfmt_configuration_file: None, no_partialeq_types: Default::default(), no_copy_types: Default::default(), diff --git a/src/options.rs b/src/options.rs index f707ab9bdf..29edb78b1c 100644 --- a/src/options.rs +++ b/src/options.rs @@ -465,7 +465,12 @@ where ), Arg::new("size_t-is-usize") .long("size_t-is-usize") - .help("Translate size_t to usize."), + .help("Ignored - this is enabled by default.") + .hidden(true), + Arg::with_name("no-size_t-is-usize") + .long("no-size_t-is-usize") + .help("Do not bind size_t as usize (useful on platforms \ + where those types are incompatible)."), Arg::new("no-rustfmt-bindings") .long("no-rustfmt-bindings") .help("Do not format the generated bindings with rustfmt."), @@ -975,8 +980,8 @@ where builder = builder.record_matches(false); } - if matches.is_present("size_t-is-usize") { - builder = builder.size_t_is_usize(true); + if matches.is_present("no-size_t-is-usize") { + builder = builder.size_t_is_usize(false); } let no_rustfmt_bindings = matches.is_present("no-rustfmt-bindings"); diff --git a/tests/expectations/tests/blocks-signature.rs b/tests/expectations/tests/blocks-signature.rs index 779d0ecbcc..ff7114a7d4 100644 --- a/tests/expectations/tests/blocks-signature.rs +++ b/tests/expectations/tests/blocks-signature.rs @@ -7,7 +7,6 @@ #![cfg(target_os = "macos")] extern crate block; -pub type size_t = ::std::os::raw::c_ulonglong; extern "C" { #[link_name = "\u{1}_Z8atexit_bU13block_pointerFvvE"] pub fn atexit_b(arg1: _bindgen_ty_id_33); @@ -82,16 +81,11 @@ impl Default for contains_block_pointers { } pub type _bindgen_ty_id_33 = *const ::block::Block<(), ()>; pub type _bindgen_ty_id_40 = *const ::block::Block< - ( - dispatch_data_t, - size_t, - *const ::std::os::raw::c_void, - size_t, - ), + (dispatch_data_t, usize, *const ::std::os::raw::c_void, usize), bool, >; -pub type _bindgen_ty_id_50 = *const ::block::Block<(size_t,), ()>; -pub type _bindgen_ty_id_56 = *const ::block::Block<(size_t,), ()>; +pub type _bindgen_ty_id_50 = *const ::block::Block<(usize,), ()>; +pub type _bindgen_ty_id_56 = *const ::block::Block<(usize,), ()>; pub type contains_block_pointers__bindgen_ty_id_61 = *const ::block::Block<(::std::os::raw::c_int,), ()>; pub type _bindgen_ty_id_68 = diff --git a/tests/expectations/tests/blocks.rs b/tests/expectations/tests/blocks.rs index dbafaf94c2..6f242989b6 100644 --- a/tests/expectations/tests/blocks.rs +++ b/tests/expectations/tests/blocks.rs @@ -6,7 +6,6 @@ )] #![cfg(target_os = "macos")] -pub type size_t = ::std::os::raw::c_ulonglong; extern "C" { #[link_name = "\u{1}_Z8atexit_bU13block_pointerFvvE"] pub fn atexit_b(arg1: *mut ::std::os::raw::c_void); diff --git a/tests/expectations/tests/issue-1498.rs b/tests/expectations/tests/issue-1498.rs index 08ba9ef42d..eb5e280457 100644 --- a/tests/expectations/tests/issue-1498.rs +++ b/tests/expectations/tests/issue-1498.rs @@ -5,7 +5,6 @@ non_upper_case_globals )] -pub type size_t = u64; #[repr(C, packed)] #[derive(Copy, Clone)] pub struct rte_memseg { @@ -13,7 +12,7 @@ pub struct rte_memseg { pub phys_addr: u64, pub __bindgen_anon_1: rte_memseg__bindgen_ty_1, ///< Length of the segment. - pub len: size_t, + pub len: usize, ///< The pagesize of underlying memory pub hugepage_sz: u64, ///< NUMA socket ID. diff --git a/tests/expectations/tests/jsval_layout_opaque.rs b/tests/expectations/tests/jsval_layout_opaque.rs index 2873f6a2ff..a812e90560 100644 --- a/tests/expectations/tests/jsval_layout_opaque.rs +++ b/tests/expectations/tests/jsval_layout_opaque.rs @@ -94,7 +94,6 @@ where pub const JSVAL_TAG_SHIFT: u32 = 47; pub const JSVAL_PAYLOAD_MASK: u64 = 140737488355327; pub const JSVAL_TAG_MASK: i64 = -140737488355328; -pub type size_t = ::std::os::raw::c_ulonglong; #[repr(u8)] #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum JSValueType { @@ -186,7 +185,7 @@ pub union jsval_layout { pub s: jsval_layout__bindgen_ty_2, pub asDouble: f64, pub asPtr: *mut ::std::os::raw::c_void, - pub asWord: size_t, + pub asWord: usize, pub asUIntPtr: usize, } #[repr(C)] diff --git a/tests/expectations/tests/jsval_layout_opaque_1_0.rs b/tests/expectations/tests/jsval_layout_opaque_1_0.rs index 33594c0e05..b439499355 100644 --- a/tests/expectations/tests/jsval_layout_opaque_1_0.rs +++ b/tests/expectations/tests/jsval_layout_opaque_1_0.rs @@ -137,7 +137,6 @@ impl ::std::cmp::Eq for __BindgenUnionField {} pub const JSVAL_TAG_SHIFT: u32 = 47; pub const JSVAL_PAYLOAD_MASK: u64 = 140737488355327; pub const JSVAL_TAG_MASK: i64 = -140737488355328; -pub type size_t = ::std::os::raw::c_ulonglong; #[repr(u8)] #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum JSValueType { @@ -229,7 +228,7 @@ pub struct jsval_layout { pub s: __BindgenUnionField, pub asDouble: __BindgenUnionField, pub asPtr: __BindgenUnionField<*mut ::std::os::raw::c_void>, - pub asWord: __BindgenUnionField, + pub asWord: __BindgenUnionField, pub asUIntPtr: __BindgenUnionField, pub bindgen_union_field: u64, } diff --git a/tests/expectations/tests/layout_array.rs b/tests/expectations/tests/layout_array.rs index f3bbf51e0d..daf0ad06fd 100644 --- a/tests/expectations/tests/layout_array.rs +++ b/tests/expectations/tests/layout_array.rs @@ -9,7 +9,6 @@ pub const RTE_CACHE_LINE_SIZE: u32 = 64; pub const RTE_MEMPOOL_OPS_NAMESIZE: u32 = 32; pub const RTE_MEMPOOL_MAX_OPS_IDX: u32 = 16; pub const RTE_HEAP_NUM_FREELISTS: u32 = 13; -pub type size_t = ::std::os::raw::c_longlong; #[repr(C)] #[derive(Debug, Copy, Clone)] pub struct rte_mempool { @@ -279,7 +278,7 @@ pub struct malloc_heap { pub lock: rte_spinlock_t, pub free_head: [malloc_heap__bindgen_ty_1; 13usize], pub alloc_count: ::std::os::raw::c_uint, - pub total_size: size_t, + pub total_size: usize, } #[repr(C)] #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] diff --git a/tests/expectations/tests/msvc-no-usr.rs b/tests/expectations/tests/msvc-no-usr.rs index 285670ed68..64db43a6b6 100644 --- a/tests/expectations/tests/msvc-no-usr.rs +++ b/tests/expectations/tests/msvc-no-usr.rs @@ -5,11 +5,10 @@ non_upper_case_globals )] -pub type size_t = ::std::os::raw::c_ulonglong; #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct A { - pub foo: size_t, + pub foo: usize, } #[test] fn bindgen_test_layout_A() { diff --git a/tests/expectations/tests/size_t_is_usize.rs b/tests/expectations/tests/no_size_t_is_usize.rs similarity index 91% rename from tests/expectations/tests/size_t_is_usize.rs rename to tests/expectations/tests/no_size_t_is_usize.rs index 2397e0a56c..f399a66724 100644 --- a/tests/expectations/tests/size_t_is_usize.rs +++ b/tests/expectations/tests/no_size_t_is_usize.rs @@ -5,11 +5,13 @@ non_upper_case_globals )] +pub type size_t = ::std::os::raw::c_ulong; +pub type ssize_t = ::std::os::raw::c_long; #[repr(C)] #[derive(Debug, Copy, Clone)] pub struct A { - pub len: usize, - pub offset: isize, + pub len: size_t, + pub offset: ssize_t, pub next: *mut A, } #[test] diff --git a/tests/expectations/tests/nsBaseHashtable.rs b/tests/expectations/tests/nsBaseHashtable.rs index d7607b91ab..26179b763e 100644 --- a/tests/expectations/tests/nsBaseHashtable.rs +++ b/tests/expectations/tests/nsBaseHashtable.rs @@ -5,7 +5,6 @@ non_upper_case_globals )] -pub type size_t = ::std::os::raw::c_ulonglong; #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct nsBaseHashtableET { diff --git a/tests/headers/size_t_is_usize.h b/tests/headers/no_size_t_is_usize.h similarity index 75% rename from tests/headers/size_t_is_usize.h rename to tests/headers/no_size_t_is_usize.h index 564b486784..d4370baf9f 100644 --- a/tests/headers/size_t_is_usize.h +++ b/tests/headers/no_size_t_is_usize.h @@ -1,4 +1,4 @@ -// bindgen-flags: --size_t-is-usize +// bindgen-flags: --no-size_t-is-usize typedef unsigned long size_t; typedef long ssize_t;