From 721044bafcd6e44447565de14db1f9921e84e8a0 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Fri, 3 Sep 2021 10:35:56 +0100 Subject: [PATCH 1/3] Add --blocklist-file option Update Item to hold a `clang::SourceLocation` and use this to allow blocklisting based on filename. The existing code has a special case that always maps integer types to corresponding Rust integer types, even if the C types are blocklisted. To match this special case behaviour, also treat these C types as being eligible for derived Copy/Clone/Debug traits. Fixes #2096 --- CONTRIBUTING.md | 6 ++- book/src/blocklisting.md | 14 ++++- src/clang.rs | 6 +++ src/ir/context.rs | 37 ++++++++++--- src/ir/item.rs | 26 ++++++++- src/lib.rs | 14 +++++ src/options.rs | 14 +++++ tests/expectations/tests/blocklist-file.rs | 63 ++++++++++++++++++++++ tests/headers/blocklist-file.hpp | 10 ++++ tests/headers/blocklisted/fake-stdint.h | 7 +++ tests/headers/blocklisted/file.hpp | 19 +++++++ 11 files changed, 202 insertions(+), 14 deletions(-) create mode 100644 tests/expectations/tests/blocklist-file.rs create mode 100644 tests/headers/blocklist-file.hpp create mode 100644 tests/headers/blocklisted/fake-stdint.h create mode 100644 tests/headers/blocklisted/file.hpp diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8a9f8688f6..e6b6a505d0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -247,8 +247,8 @@ See [./csmith-fuzzing/README.md](./csmith-fuzzing/README.md) for details. The `tests/quickchecking` crate generates property tests for `bindgen`. From the crate's directory you can run the tests with `cargo run`. For details -on additional configuration including how to preserve / inspect the generated -property tests, see +on additional configuration including how to preserve / inspect the generated +property tests, see [./tests/quickchecking/README.md](./tests/quickchecking/README.md). ## Code Overview @@ -298,6 +298,8 @@ looking at. Here is a summary of the IR types and their relationships: * Its type's `ItemId` * Optionally, a mangled name * Optionally, a value + * An optional `clang::SourceLocation` that holds the first source code + location where the `Item` was encountered. The IR forms a graph of interconnected and inter-referencing types and functions. The `ir::traversal` module provides IR graph traversal diff --git a/book/src/blocklisting.md b/book/src/blocklisting.md index 5c082ddaba..6eb5786328 100644 --- a/book/src/blocklisting.md +++ b/book/src/blocklisting.md @@ -2,8 +2,7 @@ If you need to provide your own custom translation of some type (for example, because you need to wrap one of its fields in an `UnsafeCell`), you can -explicitly blocklist - generation of its definition. Uses of the blocklisted type +explicitly blocklist generation of its definition. Uses of the blocklisted type will still appear in other types' definitions. (If you don't want the type to appear in the bindings at all, [make it opaque](./opaque.md) instead of @@ -13,14 +12,25 @@ Blocklisted types are pessimistically assumed not to be able to `derive` any traits, which can transitively affect other types' ability to `derive` traits or not. +The `blocklist-file` option also allows the blocklisting of all items from a +particular path regex, for example to block all types defined in system headers +that are transitively included. + ### Library +* [`bindgen::Builder::blocklist_file`](https://docs.rs/bindgen/latest/bindgen/struct.Builder.html#method.blocklist_file) +* [`bindgen::Builder::blocklist_function`](https://docs.rs/bindgen/latest/bindgen/struct.Builder.html#method.blocklist_function) +* [`bindgen::Builder::blocklist_item`](https://docs.rs/bindgen/latest/bindgen/struct.Builder.html#method.blocklist_item) * [`bindgen::Builder::blocklist_type`](https://docs.rs/bindgen/latest/bindgen/struct.Builder.html#method.blocklist_type) ### Command Line +* `--blocklist-file ` +* `--blocklist-function ` +* `--blocklist-item ` * `--blocklist-type ` + ### Annotations ```cpp diff --git a/src/clang.rs b/src/clang.rs index 36ccd266f8..074d459ba7 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -1401,6 +1401,12 @@ impl fmt::Display for SourceLocation { } } +impl fmt::Debug for SourceLocation { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self) + } +} + /// A comment in the source text. /// /// Comments are sort of parsed by Clang, and have a tree structure. diff --git a/src/ir/context.rs b/src/ir/context.rs index 874fb5df2b..03a96bb13a 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -1423,7 +1423,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" fn build_root_module(id: ItemId) -> Item { let module = Module::new(Some("root".into()), ModuleKind::Normal); - Item::new(id, None, None, id, ItemKind::Module(module)) + Item::new(id, None, None, id, ItemKind::Module(module), None) } /// Get the root module. @@ -1733,6 +1733,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" None, self.current_module.into(), ItemKind::Type(sub_ty), + Some(child.location()), ); // Bypass all the validations in add_item explicitly. @@ -1797,6 +1798,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" None, self.current_module.into(), ItemKind::Type(ty), + Some(location.location()), ); // Bypass all the validations in add_item explicitly. @@ -1930,6 +1932,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" ) -> TypeId { let spelling = ty.spelling(); let layout = ty.fallible_layout(self).ok(); + let location = ty.declaration().location(); let type_kind = TypeKind::ResolvedTypeRef(wrapped_id); let ty = Type::new(Some(spelling), layout, type_kind, is_const); let item = Item::new( @@ -1938,6 +1941,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" None, parent_id.unwrap_or_else(|| self.current_module.into()), ItemKind::Type(ty), + Some(location), ); self.add_builtin_item(item); with_id.as_type_id_unchecked() @@ -1998,6 +2002,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" let spelling = ty.spelling(); let is_const = ty.is_const(); let layout = ty.fallible_layout(self).ok(); + let location = ty.declaration().location(); let ty = Type::new(Some(spelling), layout, type_kind, is_const); let id = self.next_item_id(); let item = Item::new( @@ -2006,6 +2011,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" None, self.root_module.into(), ItemKind::Type(ty), + Some(location), ); self.add_builtin_item(item); Some(id.as_type_id_unchecked()) @@ -2194,6 +2200,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" None, self.current_module.into(), ItemKind::Module(module), + Some(cursor.location()), ); let module_id = module.id().as_module_id_unchecked(); @@ -2241,11 +2248,6 @@ If you encounter an error missing from this list, please file an issue or a PR!" 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() @@ -2255,8 +2257,27 @@ If you encounter an error missing from this list, please file an issue or a PR!" .or_insert_with(|| { item.expect_type() .name() - .and_then(|name| { - cb.blocklisted_type_implements_trait(name, derive_trait) + .and_then(|name| match self.options.parse_callbacks { + Some(ref cb) => cb.blocklisted_type_implements_trait( + name, + derive_trait, + ), + // Sized integer types from get mapped to Rust primitive + // types regardless of whether they are blocklisted, so ensure that + // standard traits are considered derivable for them too. + None => match name { + "int8_t" | "uint8_t" | "int16_t" | "uint16_t" | + "int32_t" | "uint32_t" | "int64_t" | + "uint64_t" | "uintptr_t" | "intptr_t" | + "ptrdiff_t" => Some(CanDerive::Yes), + "size_t" if self.options.size_t_is_usize => { + Some(CanDerive::Yes) + } + "ssize_t" if self.options.size_t_is_usize => { + Some(CanDerive::Yes) + } + _ => Some(CanDerive::No), + }, }) .unwrap_or(CanDerive::No) }) diff --git a/src/ir/item.rs b/src/ir/item.rs index 730271efea..969929f4e5 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -417,6 +417,8 @@ pub struct Item { parent_id: ItemId, /// The item kind. kind: ItemKind, + /// The source location of the item. + location: Option, } impl AsRef for Item { @@ -433,6 +435,7 @@ impl Item { annotations: Option, parent_id: ItemId, kind: ItemKind, + location: Option, ) -> Self { debug_assert!(id != parent_id || kind.is_module()); Item { @@ -445,6 +448,7 @@ impl Item { comment, annotations: annotations.unwrap_or_default(), kind, + location, } } @@ -454,10 +458,15 @@ impl Item { ty: &clang::Type, ctx: &mut BindgenContext, ) -> TypeId { + let location = ty.declaration().location(); let ty = Opaque::from_clang_ty(ty, ctx); let kind = ItemKind::Type(ty); let parent = ctx.root_module().into(); - ctx.add_item(Item::new(with_id, None, None, parent, kind), None, None); + ctx.add_item( + Item::new(with_id, None, None, parent, kind, Some(location)), + None, + None, + ); with_id.as_type_id_unchecked() } @@ -632,6 +641,15 @@ impl Item { return true; } + if let Some(location) = &self.location { + let (file, _, _, _) = location.location(); + if let Some(filename) = file.name() { + if ctx.options().blocklisted_files.matches(&filename) { + return true; + } + } + } + let path = self.path_for_allowlisting(ctx); let name = path[1..].join("::"); ctx.options().blocklisted_items.matches(&name) || @@ -1297,7 +1315,7 @@ impl ClangItemParser for Item { let id = ctx.next_item_id(); let module = ctx.root_module().into(); ctx.add_item( - Item::new(id, None, None, module, ItemKind::Type(ty)), + Item::new(id, None, None, module, ItemKind::Type(ty), None), None, None, ); @@ -1335,6 +1353,7 @@ impl ClangItemParser for Item { annotations, relevant_parent_id, ItemKind::$what(item), + Some(cursor.location()), ), declaration, Some(cursor), @@ -1516,6 +1535,7 @@ impl ClangItemParser for Item { None, parent_id.unwrap_or_else(|| current_module.into()), ItemKind::Type(Type::new(None, None, kind, is_const)), + Some(location.location()), ), None, None, @@ -1647,6 +1667,7 @@ impl ClangItemParser for Item { annotations, relevant_parent_id, ItemKind::Type(item), + Some(location.location()), ), declaration, Some(location), @@ -1875,6 +1896,7 @@ impl ClangItemParser for Item { None, parent, ItemKind::Type(Type::named(name)), + Some(location.location()), ); ctx.add_type_param(item, definition); Some(id.as_type_id_unchecked()) diff --git a/src/lib.rs b/src/lib.rs index 1129efe767..9adbb1079c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -308,6 +308,7 @@ impl Builder { (&self.options.blocklisted_types, "--blocklist-type"), (&self.options.blocklisted_functions, "--blocklist-function"), (&self.options.blocklisted_items, "--blocklist-item"), + (&self.options.blocklisted_files, "--blocklist-file"), (&self.options.opaque_types, "--opaque-type"), (&self.options.allowlisted_functions, "--allowlist-function"), (&self.options.allowlisted_types, "--allowlist-type"), @@ -821,6 +822,13 @@ impl Builder { self } + /// Hide any contents of the given file from the generated bindings, + /// regardless of whether it's a type, function, module etc. + pub fn blocklist_file>(mut self, arg: T) -> Builder { + self.options.blocklisted_files.insert(arg); + self + } + /// Treat the given type as opaque in the generated bindings. Regular /// expressions are supported. /// @@ -1669,6 +1677,10 @@ struct BindgenOptions { /// blocklisted and should not appear in the generated code. blocklisted_items: RegexSet, + /// The set of files whose contents should be blocklisted and should not + /// appear in the generated code. + blocklisted_files: RegexSet, + /// The set of types that should be treated as opaque structures in the /// generated code. opaque_types: RegexSet, @@ -1982,6 +1994,7 @@ impl BindgenOptions { &mut self.blocklisted_types, &mut self.blocklisted_functions, &mut self.blocklisted_items, + &mut self.blocklisted_files, &mut self.opaque_types, &mut self.bitfield_enums, &mut self.constified_enums, @@ -2029,6 +2042,7 @@ impl Default for BindgenOptions { blocklisted_types: Default::default(), blocklisted_functions: Default::default(), blocklisted_items: Default::default(), + blocklisted_files: Default::default(), opaque_types: Default::default(), rustfmt_path: Default::default(), depfile: Default::default(), diff --git a/src/options.rs b/src/options.rs index 9aac5dae83..bc7431c52d 100644 --- a/src/options.rs +++ b/src/options.rs @@ -164,6 +164,14 @@ where .takes_value(true) .multiple(true) .number_of_values(1), + Arg::with_name("blocklist-file") + .alias("blacklist-file") + .long("blocklist-file") + .help("Mark all contents of as hidden.") + .value_name("path") + .takes_value(true) + .multiple(true) + .number_of_values(1), Arg::with_name("no-layout-tests") .long("no-layout-tests") .help("Avoid generating layout tests for any type."), @@ -630,6 +638,12 @@ where } } + if let Some(hidden_files) = matches.values_of("blocklist-file") { + for file in hidden_files { + builder = builder.blocklist_file(file); + } + } + if matches.is_present("builtins") { builder = builder.emit_builtins(); } diff --git a/tests/expectations/tests/blocklist-file.rs b/tests/expectations/tests/blocklist-file.rs new file mode 100644 index 0000000000..6c95b066b2 --- /dev/null +++ b/tests/expectations/tests/blocklist-file.rs @@ -0,0 +1,63 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct SizedIntegers { + pub x: u8, + pub y: u16, + pub z: u32, +} +#[test] +fn bindgen_test_layout_SizedIntegers() { + assert_eq!( + ::std::mem::size_of::(), + 8usize, + concat!("Size of: ", stringify!(SizedIntegers)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(SizedIntegers)) + ); + assert_eq!( + unsafe { + &(*(::std::ptr::null::())).x as *const _ as usize + }, + 0usize, + concat!( + "Offset of field: ", + stringify!(SizedIntegers), + "::", + stringify!(x) + ) + ); + assert_eq!( + unsafe { + &(*(::std::ptr::null::())).y as *const _ as usize + }, + 2usize, + concat!( + "Offset of field: ", + stringify!(SizedIntegers), + "::", + stringify!(y) + ) + ); + assert_eq!( + unsafe { + &(*(::std::ptr::null::())).z as *const _ as usize + }, + 4usize, + concat!( + "Offset of field: ", + stringify!(SizedIntegers), + "::", + stringify!(z) + ) + ); +} diff --git a/tests/headers/blocklist-file.hpp b/tests/headers/blocklist-file.hpp new file mode 100644 index 0000000000..30b661814d --- /dev/null +++ b/tests/headers/blocklist-file.hpp @@ -0,0 +1,10 @@ +// bindgen-flags: --blocklist-file ".*/blocklisted/file.*" -- -Itests/headers + +#include "blocklisted/file.hpp" +#include "blocklisted/fake-stdint.h" + +struct SizedIntegers { + uint8_t x; + uint16_t y; + uint32_t z; +}; diff --git a/tests/headers/blocklisted/fake-stdint.h b/tests/headers/blocklisted/fake-stdint.h new file mode 100644 index 0000000000..a8cfe01056 --- /dev/null +++ b/tests/headers/blocklisted/fake-stdint.h @@ -0,0 +1,7 @@ +// -like types get special treatment even when blocklisted. +typedef signed char int8_t; +typedef unsigned char uint8_t; +typedef signed short int16_t; +typedef unsigned short uint16_t; +typedef signed int int32_t; +typedef unsigned int uint32_t; diff --git a/tests/headers/blocklisted/file.hpp b/tests/headers/blocklisted/file.hpp new file mode 100644 index 0000000000..9dbc83ed64 --- /dev/null +++ b/tests/headers/blocklisted/file.hpp @@ -0,0 +1,19 @@ +void SomeFunction(); +extern int someVar; +#define SOME_DEFUN 123 + +class someClass { + void somePrivateMethod(); +public: + void somePublicMethod(int foo); +}; + +extern "C" void ExternFunction(); + +namespace foo { + void NamespacedFunction(); +} + +namespace bar { + void NamespacedFunction(); +} From ccd4b8226cdc99837642df492aa6f5a050254667 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Fri, 29 Oct 2021 07:25:10 +0100 Subject: [PATCH 2/3] fixup: skip blocklist check if no files specified --- src/ir/item.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ir/item.rs b/src/ir/item.rs index 969929f4e5..8692575fe9 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -641,11 +641,13 @@ impl Item { return true; } - if let Some(location) = &self.location { - let (file, _, _, _) = location.location(); - if let Some(filename) = file.name() { - if ctx.options().blocklisted_files.matches(&filename) { - return true; + if !ctx.options().blocklisted_files.is_empty() { + if let Some(location) = &self.location { + let (file, _, _, _) = location.location(); + if let Some(filename) = file.name() { + if ctx.options().blocklisted_files.matches(&filename) { + return true; + } } } } From 46a7d0bd4dcb4ed1f085149a49bc27189849833e Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Fri, 29 Oct 2021 07:46:56 +0100 Subject: [PATCH 3/3] fixup: add fwd decls to test case - struct with normal fwd decl and blocklisted definition ignored - struct with blocklisted fwd decl and normal definition emitted --- tests/expectations/tests/blocklist-file.rs | 31 ++++++++++++++++++++++ tests/headers/blocklist-file.hpp | 8 ++++++ tests/headers/blocklisted/file.hpp | 7 +++++ 3 files changed, 46 insertions(+) diff --git a/tests/expectations/tests/blocklist-file.rs b/tests/expectations/tests/blocklist-file.rs index 6c95b066b2..fe00b5ab4a 100644 --- a/tests/expectations/tests/blocklist-file.rs +++ b/tests/expectations/tests/blocklist-file.rs @@ -61,3 +61,34 @@ fn bindgen_test_layout_SizedIntegers() { ) ); } +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct StructWithBlocklistedFwdDecl { + pub b: u8, +} +#[test] +fn bindgen_test_layout_StructWithBlocklistedFwdDecl() { + assert_eq!( + ::std::mem::size_of::(), + 1usize, + concat!("Size of: ", stringify!(StructWithBlocklistedFwdDecl)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(StructWithBlocklistedFwdDecl)) + ); + assert_eq!( + unsafe { + &(*(::std::ptr::null::())).b + as *const _ as usize + }, + 0usize, + concat!( + "Offset of field: ", + stringify!(StructWithBlocklistedFwdDecl), + "::", + stringify!(b) + ) + ); +} diff --git a/tests/headers/blocklist-file.hpp b/tests/headers/blocklist-file.hpp index 30b661814d..ad8bcd665c 100644 --- a/tests/headers/blocklist-file.hpp +++ b/tests/headers/blocklist-file.hpp @@ -1,5 +1,8 @@ // bindgen-flags: --blocklist-file ".*/blocklisted/file.*" -- -Itests/headers +// Forward declaration of struct that's defined in a blocklisted file. +struct StructWithBlocklistedDefinition; + #include "blocklisted/file.hpp" #include "blocklisted/fake-stdint.h" @@ -8,3 +11,8 @@ struct SizedIntegers { uint16_t y; uint32_t z; }; + +// Actual definition of struct that has a forward declaration in a blocklisted file. +struct StructWithBlocklistedFwdDecl { + uint8_t b; +}; diff --git a/tests/headers/blocklisted/file.hpp b/tests/headers/blocklisted/file.hpp index 9dbc83ed64..4bcb589e1d 100644 --- a/tests/headers/blocklisted/file.hpp +++ b/tests/headers/blocklisted/file.hpp @@ -17,3 +17,10 @@ namespace foo { namespace bar { void NamespacedFunction(); } + + +struct StructWithBlocklistedFwdDecl; + +struct StructWithBlocklistedDefinition { + StructWithBlocklistedFwdDecl* other; +};