From 43d58cd5ba170975a8fe0024ba792a46603de82d Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Wed, 10 Mar 2021 19:16:47 +0100 Subject: [PATCH] Add callback to check derives for blocklisted types Fixes #1454 #2003 --- src/callbacks.rs | 19 ++++++++++++ src/ir/analysis/derive.rs | 23 +++++++++++---- src/ir/context.rs | 30 ++++++++++++++++++- tests/expectations/tests/issue-1454.rs | 41 ++++++++++++++++++++++++++ tests/headers/issue-1454.h | 10 +++++++ tests/parse_callbacks/mod.rs | 22 ++++++++++++-- 6 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 tests/expectations/tests/issue-1454.rs create mode 100644 tests/headers/issue-1454.h diff --git a/src/callbacks.rs b/src/callbacks.rs index 1cd7f37b07..a8ca1f9143 100644 --- a/src/callbacks.rs +++ b/src/callbacks.rs @@ -2,6 +2,8 @@ pub use crate::ir::enum_ty::{EnumVariantCustomBehavior, EnumVariantValue}; pub use crate::ir::int::IntKind; +pub use crate::ir::analysis::DeriveTrait; +pub use crate::ir::derive::CanDerive as ImplementsTrait; use std::fmt; use std::panic::UnwindSafe; @@ -76,4 +78,21 @@ pub trait ParseCallbacks: fmt::Debug + UnwindSafe { /// This will be called on every file inclusion, with the full path of the included file. fn include_file(&self, _filename: &str) {} + + /// This will be called to determine whether a particular blocklisted type + /// implements a trait or not. This will be used to implement traits on + /// other types containing the blocklisted type. + /// + /// * `None`: use the default behavior + /// * `Some(ImplementsTrait::Yes)`: `_name` implements `_derive_trait` + /// * `Some(ImplementsTrait::Manually)`: any type including `_name` can't + /// derive `_derive_trait` but can implemented it manually + /// * `Some(ImplementsTrait::No)`: `_name` doesn't implement `_derive_trait` + fn blocklisted_type_implements_trait( + &self, + _name: &str, + _derive_trait: DeriveTrait + ) -> Option { + None + } } diff --git a/src/ir/analysis/derive.rs b/src/ir/analysis/derive.rs index fa4ce795da..6ac6684576 100644 --- a/src/ir/analysis/derive.rs +++ b/src/ir/analysis/derive.rs @@ -16,7 +16,7 @@ use crate::ir::ty::{Type, TypeKind}; use crate::{Entry, HashMap, HashSet}; /// Which trait to consider when doing the `CannotDerive` analysis. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum DeriveTrait { /// The `Copy` trait. Copy, @@ -139,11 +139,22 @@ impl<'ctx> CannotDerive<'ctx> { fn constrain_type(&mut self, item: &Item, ty: &Type) -> CanDerive { if !self.ctx.allowlisted_items().contains(&item.id()) { - trace!( - " cannot derive {} for blocklisted type", - self.derive_trait - ); - return CanDerive::No; + let can_derive = self.ctx.blocklisted_type_implements_trait(item, self.derive_trait); + match can_derive { + CanDerive::Yes => trace!( + " blocklisted type explicitly implements {}", + self.derive_trait + ), + CanDerive::Manually => trace!( + " blocklisted type requires manual implementation of {}", + self.derive_trait + ), + CanDerive::No => trace!( + " cannot derive {} for blocklisted type", + self.derive_trait + ), + } + return can_derive; } if self.derive_trait.not_by_name(self.ctx, &item) { diff --git a/src/ir/context.rs b/src/ir/context.rs index a0d4de1e22..05b51cc345 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -28,7 +28,7 @@ use cexpr; use clang_sys; use proc_macro2::{Ident, Span}; use std::borrow::Cow; -use std::cell::Cell; +use std::cell::{Cell, RefCell}; use std::collections::HashMap as StdHashMap; use std::iter::IntoIterator; use std::mem; @@ -380,6 +380,9 @@ pub struct BindgenContext { /// computed after parsing our IR, and before running any of our analyses. allowlisted: Option, + /// Cache for calls to `ParseCallbacks::blocklisted_type_implements_trait` + blocklisted_types_implement_traits: RefCell>>, + /// The set of `ItemId`s that are allowlisted for code generation _and_ that /// we should generate accounting for the codegen options. /// @@ -560,6 +563,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" options, generated_bindgen_complex: Cell::new(false), allowlisted: None, + blocklisted_types_implement_traits: Default::default(), codegen_items: None, used_template_parameters: None, need_bitfield_allocation: Default::default(), @@ -2205,6 +2209,30 @@ If you encounter an error missing from this list, please file an issue or a PR!" self.allowlisted.as_ref().unwrap() } + /// Check whether a particular blocklisted type implements a trait or not. + /// Results may be cached. + pub fn blocklisted_type_implements_trait(&self, item: &Item, derive_trait: DeriveTrait) -> CanDerive { + assert!(self.in_codegen_phase()); + assert!(self.current_module == self.root_module); + + let cb = match self.options.parse_callbacks { + Some(ref cb) => cb, + None => return CanDerive::No, + }; + + *self.blocklisted_types_implement_traits + .borrow_mut() + .entry(derive_trait) + .or_default() + .entry(item.id()) + .or_insert_with(|| + item.expect_type() + .name() + .and_then(|name| cb.blocklisted_type_implements_trait(name, derive_trait)) + .unwrap_or(CanDerive::No) + ) + } + /// Get a reference to the set of items we should generate. pub fn codegen_items(&self) -> &ItemSet { assert!(self.in_codegen_phase()); diff --git a/tests/expectations/tests/issue-1454.rs b/tests/expectations/tests/issue-1454.rs new file mode 100644 index 0000000000..e88e46978b --- /dev/null +++ b/tests/expectations/tests/issue-1454.rs @@ -0,0 +1,41 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug)] +pub struct extern_type; + +#[repr(C)] +#[derive(Debug)] +pub struct local_type { + pub inner: extern_type, +} +#[test] +fn bindgen_test_layout_local_type() { + assert_eq!( + ::std::mem::size_of::(), + 0usize, + concat!("Size of: ", stringify!(local_type)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(local_type)) + ); + assert_eq!( + unsafe { + &(*(::std::ptr::null::())).inner as *const _ as usize + }, + 0usize, + concat!( + "Offset of field: ", + stringify!(local_type), + "::", + stringify!(inner) + ) + ); +} diff --git a/tests/headers/issue-1454.h b/tests/headers/issue-1454.h new file mode 100644 index 0000000000..96645dac9d --- /dev/null +++ b/tests/headers/issue-1454.h @@ -0,0 +1,10 @@ +// bindgen-flags: --no-recursive-allowlist --allowlist-type "local_type" --with-derive-hash --no-derive-copy --no-derive-default --raw-line "#[repr(C)] #[derive(Debug)] pub struct extern_type;" +// bindgen-parse-callbacks: blocklisted-type-implements-trait + +struct extern_type {}; + +typedef struct +{ + struct extern_type inner; +} +local_type; diff --git a/tests/parse_callbacks/mod.rs b/tests/parse_callbacks/mod.rs index 01993cfc77..d80ceb5537 100644 --- a/tests/parse_callbacks/mod.rs +++ b/tests/parse_callbacks/mod.rs @@ -1,4 +1,4 @@ -use bindgen::callbacks::ParseCallbacks; +use bindgen::callbacks::*; #[derive(Debug)] struct EnumVariantRename; @@ -8,15 +8,33 @@ impl ParseCallbacks for EnumVariantRename { &self, _enum_name: Option<&str>, original_variant_name: &str, - _variant_value: bindgen::callbacks::EnumVariantValue, + _variant_value: EnumVariantValue, ) -> Option { Some(format!("RENAMED_{}", original_variant_name)) } } +#[derive(Debug)] +struct BlocklistedTypeImplementsTrait; + +impl ParseCallbacks for BlocklistedTypeImplementsTrait { + fn blocklisted_type_implements_trait( + &self, + _name: &str, + derive_trait: DeriveTrait + ) -> Option { + if derive_trait == DeriveTrait::Hash { + Some(ImplementsTrait::No) + } else { + Some(ImplementsTrait::Yes) + } + } +} + pub fn lookup(cb: &str) -> Box { match cb { "enum-variant-rename" => Box::new(EnumVariantRename), + "blocklisted-type-implements-trait" => Box::new(BlocklistedTypeImplementsTrait), _ => panic!("Couldn't find name ParseCallbacks: {}", cb), } }