Skip to content

Add callback to check derives for blocklisted types #2007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/callbacks.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! A public API for more fine-grained customization of bindgen behavior.

pub use crate::ir::analysis::DeriveTrait;
pub use crate::ir::derive::CanDerive as ImplementsTrait;
pub use crate::ir::enum_ty::{EnumVariantCustomBehavior, EnumVariantValue};
pub use crate::ir::int::IntKind;
use std::fmt;
Expand Down Expand Up @@ -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<ImplementsTrait> {
None
}
}
25 changes: 19 additions & 6 deletions src/ir/analysis/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -139,11 +139,24 @@ 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) {
Expand Down
38 changes: 37 additions & 1 deletion src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -380,6 +380,10 @@ pub struct BindgenContext {
/// computed after parsing our IR, and before running any of our analyses.
allowlisted: Option<ItemSet>,

/// Cache for calls to `ParseCallbacks::blocklisted_type_implements_trait`
blocklisted_types_implement_traits:
RefCell<HashMap<DeriveTrait, HashMap<ItemId, CanDerive>>>,

/// The set of `ItemId`s that are allowlisted for code generation _and_ that
/// we should generate accounting for the codegen options.
///
Expand Down Expand Up @@ -560,6 +564,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(),
Expand Down Expand Up @@ -2205,6 +2210,37 @@ 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());
Expand Down
41 changes: 41 additions & 0 deletions tests/expectations/tests/issue-1454.rs
Original file line number Diff line number Diff line change
@@ -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::<local_type>(),
0usize,
concat!("Size of: ", stringify!(local_type))
);
assert_eq!(
::std::mem::align_of::<local_type>(),
1usize,
concat!("Alignment of ", stringify!(local_type))
);
assert_eq!(
unsafe {
&(*(::std::ptr::null::<local_type>())).inner as *const _ as usize
},
0usize,
concat!(
"Offset of field: ",
stringify!(local_type),
"::",
stringify!(inner)
)
);
}
10 changes: 10 additions & 0 deletions tests/headers/issue-1454.h
Original file line number Diff line number Diff line change
@@ -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;
24 changes: 22 additions & 2 deletions tests/parse_callbacks/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use bindgen::callbacks::ParseCallbacks;
use bindgen::callbacks::*;

#[derive(Debug)]
struct EnumVariantRename;
Expand All @@ -8,15 +8,35 @@ impl ParseCallbacks for EnumVariantRename {
&self,
_enum_name: Option<&str>,
original_variant_name: &str,
_variant_value: bindgen::callbacks::EnumVariantValue,
_variant_value: EnumVariantValue,
) -> Option<String> {
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<ImplementsTrait> {
if derive_trait == DeriveTrait::Hash {
Some(ImplementsTrait::No)
} else {
Some(ImplementsTrait::Yes)
}
}
}

pub fn lookup(cb: &str) -> Box<dyn ParseCallbacks> {
match cb {
"enum-variant-rename" => Box::new(EnumVariantRename),
"blocklisted-type-implements-trait" => {
Box::new(BlocklistedTypeImplementsTrait)
}
_ => panic!("Couldn't find name ParseCallbacks: {}", cb),
}
}