From 3ed9434f097b3a05b4ed6eedf80877d5d4b9ace9 Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Fri, 17 May 2024 09:13:49 -0700 Subject: [PATCH] Deduplicate the code that turns transparent structs into typedefs All zero-sized structs now trigger a warning about undefined behavior, and an empty struct definition is emitted regardless of whether the user asked for repr(C) or repr(transparent). --- src/bindgen/ir/structure.rs | 28 ++++++++++++++++++-- src/bindgen/ir/typedef.rs | 17 ++++++++++-- src/bindgen/language_backend/clike.rs | 18 ------------- src/bindgen/language_backend/cython.rs | 18 ------------- src/bindgen/language_backend/mod.rs | 20 +++++++++++++- tests/expectations/transparent.c | 7 ++++- tests/expectations/transparent.compat.c | 7 ++++- tests/expectations/transparent.cpp | 7 ++++- tests/expectations/transparent.pyx | 6 ++++- tests/expectations/transparent_both.c | 7 ++++- tests/expectations/transparent_both.compat.c | 7 ++++- tests/expectations/transparent_tag.c | 7 ++++- tests/expectations/transparent_tag.compat.c | 7 ++++- tests/expectations/transparent_tag.pyx | 6 ++++- tests/rust/transparent.rs | 8 +++++- 15 files changed, 119 insertions(+), 51 deletions(-) diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index 49ae5da85..9b33a15c7 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -11,7 +11,7 @@ use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver; use crate::bindgen::dependencies::Dependencies; use crate::bindgen::ir::{ AnnotationSet, Cfg, Constant, Documentation, Field, GenericArgument, GenericParams, Item, - ItemContainer, Path, Repr, ReprAlign, ReprStyle, Type, + ItemContainer, Path, Repr, ReprAlign, ReprStyle, Type, Typedef, }; use crate::bindgen::library::Library; use crate::bindgen::mangle; @@ -122,11 +122,27 @@ impl Struct { has_tag_field: bool, is_enum_variant_body: bool, alignment: Option, - is_transparent: bool, + mut is_transparent: bool, cfg: Option, annotations: AnnotationSet, documentation: Documentation, ) -> Self { + // WARNING: Zero-sized transparent structs are legal rust [1], but zero-sized types of any + // repr are "best avoided entirely" [2] because they "will be nonsensical or problematic if + // passed through the FFI boundary" [3]. Further, because no well-defined underlying native + // type exists for a ZST, we cannot emit a typedef and must define an empty struct instead. + // + // [1] https://github.com/rust-lang/rust/issues/77841#issuecomment-716575747 + // [2] https://github.com/rust-lang/rust/issues/77841#issuecomment-716796313 + // [3] https://doc.rust-lang.org/nomicon/other-reprs.html + if fields.is_empty() { + warn!( + "Passing zero-sized struct {} across the FFI boundary is undefined behavior", + &path + ); + is_transparent = false; + } + let export_name = path.name().to_owned(); Self { path, @@ -150,6 +166,14 @@ impl Struct { } } + /// Attempts to convert this struct to a typedef (only works for transparent structs). + pub fn as_typedef(&self) -> Option { + match self.fields.first() { + Some(field) if self.is_transparent => Some(Typedef::new_from_struct_field(self, field)), + _ => None, + } + } + pub fn add_monomorphs(&self, library: &Library, out: &mut Monomorphs) { // Generic structs can instantiate monomorphs only once they've been // instantiated. See `instantiate_monomorph` for more details. diff --git a/src/bindgen/ir/typedef.rs b/src/bindgen/ir/typedef.rs index 58095878d..e775a4e80 100644 --- a/src/bindgen/ir/typedef.rs +++ b/src/bindgen/ir/typedef.rs @@ -10,8 +10,8 @@ use crate::bindgen::config::Config; use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver; use crate::bindgen::dependencies::Dependencies; use crate::bindgen::ir::{ - AnnotationSet, Cfg, Documentation, GenericArgument, GenericParams, Item, ItemContainer, Path, - Type, + AnnotationSet, Cfg, Documentation, Field, GenericArgument, GenericParams, Item, ItemContainer, + Path, Struct, Type, }; use crate::bindgen::library::Library; use crate::bindgen::mangle; @@ -70,6 +70,19 @@ impl Typedef { self.aliased.simplify_standard_types(config); } + // Used to convert a transparent Struct to a Typedef. + pub fn new_from_struct_field(item: &Struct, field: &Field) -> Self { + Self { + path: item.path().clone(), + export_name: item.export_name().to_string(), + generic_params: item.generic_params.clone(), + aliased: field.ty.clone(), + cfg: item.cfg().cloned(), + annotations: item.annotations().clone(), + documentation: item.documentation().clone(), + } + } + pub fn transfer_annotations(&mut self, out: &mut HashMap) { if self.annotations.is_empty() { return; diff --git a/src/bindgen/language_backend/clike.rs b/src/bindgen/language_backend/clike.rs index 5da079c3d..bf5a39fb9 100644 --- a/src/bindgen/language_backend/clike.rs +++ b/src/bindgen/language_backend/clike.rs @@ -540,24 +540,6 @@ impl LanguageBackend for CLikeLanguageBackend<'_> { } fn write_struct(&mut self, out: &mut SourceWriter, s: &Struct) { - if s.is_transparent { - let typedef = Typedef { - path: s.path.clone(), - export_name: s.export_name.to_owned(), - generic_params: s.generic_params.clone(), - aliased: s.fields[0].ty.clone(), - cfg: s.cfg.clone(), - annotations: s.annotations.clone(), - documentation: s.documentation.clone(), - }; - self.write_type_def(out, &typedef); - for constant in &s.associated_constants { - out.new_line(); - constant.write(self.config, self, out, Some(s)); - } - return; - } - let condition = s.cfg.to_condition(self.config); condition.write_before(self.config, out); diff --git a/src/bindgen/language_backend/cython.rs b/src/bindgen/language_backend/cython.rs index 5c85b2d02..8497f3cd0 100644 --- a/src/bindgen/language_backend/cython.rs +++ b/src/bindgen/language_backend/cython.rs @@ -159,24 +159,6 @@ impl LanguageBackend for CythonLanguageBackend<'_> { } fn write_struct(&mut self, out: &mut SourceWriter, s: &Struct) { - if s.is_transparent { - let typedef = Typedef { - path: s.path.clone(), - export_name: s.export_name.to_owned(), - generic_params: s.generic_params.clone(), - aliased: s.fields[0].ty.clone(), - cfg: s.cfg.clone(), - annotations: s.annotations.clone(), - documentation: s.documentation.clone(), - }; - self.write_type_def(out, &typedef); - for constant in &s.associated_constants { - out.new_line(); - constant.write(self.config, self, out, Some(s)); - } - return; - } - let condition = s.cfg.to_condition(self.config); condition.write_before(self.config, out); diff --git a/src/bindgen/language_backend/mod.rs b/src/bindgen/language_backend/mod.rs index adb7d8507..065bade20 100644 --- a/src/bindgen/language_backend/mod.rs +++ b/src/bindgen/language_backend/mod.rs @@ -139,6 +139,24 @@ pub trait LanguageBackend: Sized { } } + /// If the struct is transparent, emit a typedef of its NZST field type instead. + fn write_struct_or_typedef( + &mut self, + out: &mut SourceWriter, + s: &Struct, + b: &Bindings, + ) { + if let Some(typedef) = s.as_typedef() { + self.write_type_def(out, &typedef); + for constant in &s.associated_constants { + out.new_line(); + constant.write(&b.config, self, out, Some(s)); + } + } else { + self.write_struct(out, s); + } + } + fn write_items(&mut self, out: &mut SourceWriter, b: &Bindings) { for item in &b.items { if item @@ -155,7 +173,7 @@ pub trait LanguageBackend: Sized { ItemContainer::Constant(..) => unreachable!(), ItemContainer::Static(..) => unreachable!(), ItemContainer::Enum(ref x) => self.write_enum(out, x), - ItemContainer::Struct(ref x) => self.write_struct(out, x), + ItemContainer::Struct(ref x) => self.write_struct_or_typedef(out, x, b), ItemContainer::Union(ref x) => self.write_union(out, x), ItemContainer::OpaqueItem(ref x) => self.write_opaque_item(out, x), ItemContainer::Typedef(ref x) => self.write_type_def(out, x), diff --git a/tests/expectations/transparent.c b/tests/expectations/transparent.c index 4743b506f..3354bc294 100644 --- a/tests/expectations/transparent.c +++ b/tests/expectations/transparent.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 void root(TransparentComplexWrappingStructTuple a, @@ -32,4 +36,5 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent.compat.c b/tests/expectations/transparent.compat.c index c271f8167..1ebe9a037 100644 --- a/tests/expectations/transparent.compat.c +++ b/tests/expectations/transparent.compat.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 #ifdef __cplusplus @@ -36,7 +40,8 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/transparent.cpp b/tests/expectations/transparent.cpp index 22b5f17b9..aa6091ca6 100644 --- a/tests/expectations/transparent.cpp +++ b/tests/expectations/transparent.cpp @@ -26,6 +26,10 @@ using TransparentPrimitiveWithAssociatedConstants = uint32_t; constexpr static const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ZERO = 0; constexpr static const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ONE = 1; +struct TransparentEmptyStructure { + +}; + constexpr static const TransparentPrimitiveWrappingStructure EnumWithAssociatedConstantInImpl_TEN = 10; extern "C" { @@ -37,6 +41,7 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper e, TransparentPrimitiveWrapper f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); } // extern "C" diff --git a/tests/expectations/transparent.pyx b/tests/expectations/transparent.pyx index ac28206f0..839c77427 100644 --- a/tests/expectations/transparent.pyx +++ b/tests/expectations/transparent.pyx @@ -28,6 +28,9 @@ cdef extern from *: const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ZERO # = 0 const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ONE # = 1 + ctypedef struct TransparentEmptyStructure: + pass + const TransparentPrimitiveWrappingStructure EnumWithAssociatedConstantInImpl_TEN # = 10 void root(TransparentComplexWrappingStructTuple a, @@ -37,4 +40,5 @@ cdef extern from *: TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent_both.c b/tests/expectations/transparent_both.c index 6b3c576b0..df9b8a57d 100644 --- a/tests/expectations/transparent_both.c +++ b/tests/expectations/transparent_both.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct TransparentEmptyStructure { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 void root(TransparentComplexWrappingStructTuple a, @@ -32,4 +36,5 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent_both.compat.c b/tests/expectations/transparent_both.compat.c index 8d56046fd..4acd6677d 100644 --- a/tests/expectations/transparent_both.compat.c +++ b/tests/expectations/transparent_both.compat.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct TransparentEmptyStructure { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 #ifdef __cplusplus @@ -36,7 +40,8 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/transparent_tag.c b/tests/expectations/transparent_tag.c index c76d2a368..2f6dea68e 100644 --- a/tests/expectations/transparent_tag.c +++ b/tests/expectations/transparent_tag.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +struct TransparentEmptyStructure { + +}; + #define EnumWithAssociatedConstantInImpl_TEN 10 void root(TransparentComplexWrappingStructTuple a, @@ -32,4 +36,5 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent_tag.compat.c b/tests/expectations/transparent_tag.compat.c index 8329ca8a0..508d6ad67 100644 --- a/tests/expectations/transparent_tag.compat.c +++ b/tests/expectations/transparent_tag.compat.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +struct TransparentEmptyStructure { + +}; + #define EnumWithAssociatedConstantInImpl_TEN 10 #ifdef __cplusplus @@ -36,7 +40,8 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/transparent_tag.pyx b/tests/expectations/transparent_tag.pyx index db0f5d93c..15d66f74c 100644 --- a/tests/expectations/transparent_tag.pyx +++ b/tests/expectations/transparent_tag.pyx @@ -28,6 +28,9 @@ cdef extern from *: const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ZERO # = 0 const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ONE # = 1 + cdef struct TransparentEmptyStructure: + pass + const TransparentPrimitiveWrappingStructure EnumWithAssociatedConstantInImpl_TEN # = 10 void root(TransparentComplexWrappingStructTuple a, @@ -37,4 +40,5 @@ cdef extern from *: TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); diff --git a/tests/rust/transparent.rs b/tests/rust/transparent.rs index 6dc77c1e4..e35200cf1 100644 --- a/tests/rust/transparent.rs +++ b/tests/rust/transparent.rs @@ -41,6 +41,11 @@ impl TransparentPrimitiveWithAssociatedConstants { #[repr(transparent)] struct TransparentPrimitiveWithAssociatedConstants { bits: u32 } +// Transparent zero-sized structs are legal rust, but there's no way to emit a typedef for one, so +// cbindgen should treat it as repr(C) instead and emit an empty struct definition. +#[repr(transparent)] +struct TransparentEmptyStructure; + // Associated constant declared after struct declaration. impl TransparentPrimitiveWithAssociatedConstants { pub const ONE: TransparentPrimitiveWithAssociatedConstants = TransparentPrimitiveWithAssociatedConstants { @@ -63,5 +68,6 @@ pub extern "C" fn root( e: TransparentComplexWrapper, f: TransparentPrimitiveWrapper, g: TransparentPrimitiveWithAssociatedConstants, - h: EnumWithAssociatedConstantInImpl, + h: TransparentEmptyStructure, + i: EnumWithAssociatedConstantInImpl, ) { }