From 6390196160739701ad8525977dc60ae7f68b0e26 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. --- src/codegen/mod.rs | 9 ++++++++- 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 +-- tests/expectations/tests/jsval_layout_opaque.rs | 3 +-- tests/expectations/tests/jsval_layout_opaque_1_0.rs | 3 +-- tests/expectations/tests/layout_array.rs | 3 +-- tests/expectations/tests/msvc-no-usr.rs | 3 +-- .../{size_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 +- 13 files changed, 32 insertions(+), 31 deletions(-) rename tests/expectations/tests/{size_t_is_usize.rs => no_size_t_is_usize.rs} (90%) rename tests/headers/{size_t_is_usize.h => no_size_t_is_usize.h} (75%) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 6f8a451a2a..421517b4e1 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -820,9 +820,16 @@ 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!(layout.size == ctx.target_pointer_size() && layout.align == ctx.target_pointer_size(), + "Target platform requires --no-size_t-is-usize"); + } return; } diff --git a/src/lib.rs b/src/lib.rs index 1ac053b7f8..acedf43dd5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -523,8 +523,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 { @@ -2065,7 +2065,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 cbdc945ebb..3a036bd95a 100644 --- a/src/options.rs +++ b/src/options.rs @@ -429,7 +429,12 @@ where ), Arg::with_name("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::with_name("no-rustfmt-bindings") .long("no-rustfmt-bindings") .help("Do not format the generated bindings with rustfmt."), @@ -874,8 +879,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 22136ddfb7..ed520e7273 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); @@ -85,16 +84,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 b2ae0b2901..661a127238 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 4f8a89305b..b972474569 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 5b8427913c..4d89dd874b 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 260282dbba..2f5a8704b2 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 3ba1b6d0ba..5b5f9b5de9 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 { @@ -298,7 +297,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 ea5a90b871..d46f0438e2 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 90% rename from tests/expectations/tests/size_t_is_usize.rs rename to tests/expectations/tests/no_size_t_is_usize.rs index 0d9ab2caba..6de96305cf 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;