yggdrasil/cbindgen
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

{struct,union}: do not generate unsafe layouts

Browse Source 
If the user has not specified a corresponding annotation for types that
carry #[repr(packed)] or #[repr(align(...))] we must not generate the
layout because it can cause interoperability issues (and possible memory
unsafety) between C and Rust code.

This unfortunately required some less-than-pretty plumbing changes so
that the load phase of the parser has access to the complete Config
object (luckily this does make the overall code look a bit cleaner
IMHO). The individual IR loaders only get their corresponding structs,
to avoid introducing confusing semantics.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This commit is contained in:
Aleksa Sarai
2019-12-15 02:28:15 +11:00
committed by Emilio Cobos Álvarez
parent b95df2bd2b
commit 725378d419
5 changed files with 101 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/bindgen/builder.rs
+3 -11
View File
@@ -295,7 +295,7 @@ impl Builder {
}
for x in &self.srcs {
result.extend_with(&parser::parse_src(x, &self.config.macro_expansion)?);
result.extend_with(&parser::parse_src(x, &self.config)?);
}
if let Some((lib_dir, binding_lib_name)) = self.lib.clone() {
@@ -319,17 +319,9 @@ impl Builder {
)?
};
result.extend_with(&parser::parse_lib(
cargo,
&self.config.macro_expansion,
&self.config.parse,
)?);
result.extend_with(&parser::parse_lib(cargo, &self.config)?);
} else if let Some(cargo) = self.lib_cargo.clone() {
result.extend_with(&parser::parse_lib(
cargo,
&self.config.macro_expansion,
&self.config.parse,
)?);
result.extend_with(&parser::parse_lib(cargo, &self.config)?);
}
Library::new(
src/bindgen/config.rs
+11
View File
@@ -14,6 +14,7 @@ use toml;
use bindgen::ir::annotation::AnnotationSet;
use bindgen::ir::path::Path;
use bindgen::ir::repr::ReprAlign;
pub use bindgen::rename::RenameRule;
pub const VERSION: &str = env!("CARGO_PKG_VERSION");
@@ -258,6 +259,16 @@ pub struct LayoutConfig {
pub aligned_n: Option<String>,
}
impl LayoutConfig {
pub(crate) fn ensure_safe_to_represent(&self, align: &ReprAlign) -> Result<(), String> {
match (align, &self.packed, &self.aligned_n) {
(ReprAlign::Packed, None, _) => Err("Cannot safely represent #[repr(packed)] type without configured 'packed' annotation.".to_string()),
(ReprAlign::Align(_), _, None) => Err("Cannot safely represent #[repr(aligned(...))] type without configured 'aligned_n' annotation.".to_string()),
_ => Ok(()),
}
}
}
/// Settings to apply to generated functions.
#[derive(Debug, Clone, Deserialize)]
#[serde(rename_all = "snake_case")]
src/bindgen/ir/structure.rs
+11 -2
View File
@@ -6,7 +6,7 @@ use std::io::Write;
use syn;
use bindgen::config::{Config, Language};
use bindgen::config::{Config, Language, LayoutConfig};
use bindgen::declarationtyperesolver::DeclarationTypeResolver;
use bindgen::dependencies::Dependencies;
use bindgen::ir::{
@@ -51,7 +51,11 @@ impl Struct {
self.associated_constants.push(c);
}
pub fn load(item: &syn::ItemStruct, mod_cfg: Option<&Cfg>) -> Result<Self, String> {
pub fn load(
layout_config: &LayoutConfig,
item: &syn::ItemStruct,
mod_cfg: Option<&Cfg>,
) -> Result<Self, String> {
let repr = Repr::load(&item.attrs)?;
let is_transparent = match repr.style {
ReprStyle::C => false,
@@ -61,6 +65,11 @@ impl Struct {
}
};
// Ensure we can safely represent the struct given the configuration.
if let Some(align) = repr.align {
layout_config.ensure_safe_to_represent(&align)?;
}
let (fields, tuple_struct) = match item.fields {
syn::Fields::Unit => (Vec::new(), false),
syn::Fields::Named(ref fields) => {
src/bindgen/ir/union.rs
+11 -2
View File
@@ -6,7 +6,7 @@ use std::io::Write;
use syn;
use bindgen::config::{Config, Language};
use bindgen::config::{Config, Language, LayoutConfig};
use bindgen::declarationtyperesolver::DeclarationTypeResolver;
use bindgen::dependencies::Dependencies;
use bindgen::ir::SynFieldHelpers;
@@ -35,12 +35,21 @@ pub struct Union {
}
impl Union {
pub fn load(item: &syn::ItemUnion, mod_cfg: Option<&Cfg>) -> Result<Union, String> {
pub fn load(
layout_config: &LayoutConfig,
item: &syn::ItemUnion,
mod_cfg: Option<&Cfg>,
) -> Result<Union, String> {
let repr = Repr::load(&item.attrs)?;
if repr.style != ReprStyle::C {
return Err("Union is not marked #[repr(C)].".to_owned());
}
// Ensure we can safely represent the union given the configuration.
if let Some(align) = repr.align {
layout_config.ensure_safe_to_represent(&align)?;
}
let (fields, tuple_union) = {
let out = item
.fields
src/bindgen/parser.rs
+65 -65
View File
@@ -11,7 +11,7 @@ use syn;
use bindgen::bitflags;
use bindgen::cargo::{Cargo, PackageRef};
use bindgen::config::{MacroExpansionConfig, ParseConfig};
use bindgen::config::{Config, ParseConfig};
use bindgen::error::Error;
use bindgen::ir::{
AnnotationSet, Cfg, Constant, Documentation, Enum, Function, GenericParams, ItemMap,
@@ -31,20 +31,17 @@ const STD_CRATES: &[&str] = &[
type ParseResult = Result<Parse, Error>;
/// Parses a single rust source file, not following `mod` or `extern crate`.
pub fn parse_src(
src_file: &FilePath,
macro_expansion_config: &MacroExpansionConfig,
) -> ParseResult {
pub fn parse_src(src_file: &FilePath, config: &Config) -> ParseResult {
let mod_name = src_file.file_stem().unwrap().to_str().unwrap();
let parse_config = ParseConfig {
let mut config = config.clone();
config.parse = ParseConfig {
parse_deps: true,
..ParseConfig::default()
};
let mut context = Parser {
binding_crate_name: mod_name.to_owned(),
macro_expansion_config,
parse_config: &parse_config,
config: &config,
lib: None,
parsed_crates: HashSet::new(),
cache_src: HashMap::new(),
@@ -67,15 +64,10 @@ pub fn parse_src(
/// Inside a crate, `mod` and `extern crate` declarations are followed
/// and parsed. To find an external crate, the parser uses the `cargo metadata`
/// command to find the location of dependencies.
pub(crate) fn parse_lib(
lib: Cargo,
macro_expansion_config: &MacroExpansionConfig,
parse_config: &ParseConfig,
) -> ParseResult {
pub(crate) fn parse_lib(lib: Cargo, config: &Config) -> ParseResult {
let mut context = Parser {
binding_crate_name: lib.binding_crate_name().to_owned(),
macro_expansion_config,
parse_config,
config,
lib: Some(lib),
parsed_crates: HashSet::new(),
cache_src: HashMap::new(),
@@ -93,8 +85,7 @@ pub(crate) fn parse_lib(
struct Parser<'a> {
binding_crate_name: String,
lib: Option<Cargo>,
macro_expansion_config: &'a MacroExpansionConfig,
parse_config: &'a ParseConfig,
config: &'a Config,
parsed_crates: HashSet<String>,
cache_src: HashMap<FilePathBuf, Vec<syn::Item>>,
@@ -111,24 +102,24 @@ impl<'a> Parser<'a> {
return false;
}
if !self.parse_config.parse_deps {
if !self.config.parse.parse_deps {
return false;
}
// Skip any whitelist or blacklist for expand
if self.parse_config.expand.crates.contains(&pkg_name) {
if self.config.parse.expand.crates.contains(&pkg_name) {
return true;
}
// If we have a whitelist, check it
if let Some(ref include) = self.parse_config.include {
if let Some(ref include) = self.config.parse.include {
if !include.contains(&pkg_name) {
return false;
}
}
// Check the blacklist
!STD_CRATES.contains(&pkg_name.as_ref()) && !self.parse_config.exclude.contains(&pkg_name)
!STD_CRATES.contains(&pkg_name.as_ref()) && !self.config.parse.exclude.contains(&pkg_name)
}
fn parse_crate(&mut self, pkg: &PackageRef) -> Result<(), Error> {
@@ -136,7 +127,7 @@ impl<'a> Parser<'a> {
self.parsed_crates.insert(pkg.name.clone());
// Check if we should use cargo expand for this crate
if self.parse_config.expand.crates.contains(&pkg.name) {
if self.config.parse.expand.crates.contains(&pkg.name) {
self.parse_expand_crate(pkg)?;
} else {
// Parse the crate before the dependencies otherwise the same-named idents we
@@ -193,9 +184,9 @@ impl<'a> Parser<'a> {
.unwrap()
.expand_crate(
pkg,
self.parse_config.expand.all_features,
self.parse_config.expand.default_features,
&self.parse_config.expand.features,
self.config.parse.expand.all_features,
self.config.parse.expand.default_features,
&self.config.parse.expand.features,
)
.map_err(|x| Error::CargoExpand(pkg.name.clone(), x))?;
let i = syn::parse_file(&s).map_err(|x| Error::ParseSyntaxError {
@@ -214,8 +205,7 @@ impl<'a> Parser<'a> {
fn process_expanded_mod(&mut self, pkg: &PackageRef, items: &[syn::Item]) -> Result<(), Error> {
self.out.load_syn_crate_mod(
&self.macro_expansion_config,
&self.parse_config,
&self.config,
&self.binding_crate_name,
&pkg.name,
Cfg::join(&self.cfg_stack).as_ref(),
@@ -287,8 +277,7 @@ impl<'a> Parser<'a> {
items: &[syn::Item],
) -> Result<(), Error> {
self.out.load_syn_crate_mod(
&self.macro_expansion_config,
&self.parse_config,
&self.config,
&self.binding_crate_name,
&pkg.name,
Cfg::join(&self.cfg_stack).as_ref(),
@@ -425,8 +414,7 @@ impl Parse {
pub fn load_syn_crate_mod(
&mut self,
macro_expansion_config: &MacroExpansionConfig,
parse_config: &ParseConfig,
config: &Config,
binding_crate_name: &str,
crate_name: &str,
mod_cfg: Option<&Cfg>,
@@ -441,7 +429,7 @@ impl Parse {
match item {
syn::Item::ForeignMod(ref item) => {
self.load_syn_foreign_mod(
parse_config,
config,
binding_crate_name,
crate_name,
mod_cfg,
@@ -449,31 +437,19 @@ impl Parse {
);
}
syn::Item::Fn(ref item) => {
self.load_syn_fn(parse_config, binding_crate_name, crate_name, mod_cfg, item);
self.load_syn_fn(config, binding_crate_name, crate_name, mod_cfg, item);
}
syn::Item::Const(ref item) => {
self.load_syn_const(
parse_config,
binding_crate_name,
crate_name,
mod_cfg,
item,
);
self.load_syn_const(config, binding_crate_name, crate_name, mod_cfg, item);
}
syn::Item::Static(ref item) => {
self.load_syn_static(
parse_config,
binding_crate_name,
crate_name,
mod_cfg,
item,
);
self.load_syn_static(config, binding_crate_name, crate_name, mod_cfg, item);
}
syn::Item::Struct(ref item) => {
self.load_syn_struct(crate_name, mod_cfg, item);
self.load_syn_struct(config, crate_name, mod_cfg, item);
}
syn::Item::Union(ref item) => {
self.load_syn_union(crate_name, mod_cfg, item);
self.load_syn_union(config, crate_name, mod_cfg, item);
}
syn::Item::Enum(ref item) => {
self.load_syn_enum(crate_name, mod_cfg, item);
@@ -491,7 +467,7 @@ impl Parse {
}
}
syn::Item::Macro(ref item) => {
self.load_builtin_macro(macro_expansion_config, crate_name, mod_cfg, item)
self.load_builtin_macro(config, crate_name, mod_cfg, item)
}
_ => {}
}
@@ -523,7 +499,7 @@ impl Parse {
/// Enters a `extern "C" { }` declaration and loads function declarations.
fn load_syn_foreign_mod(
&mut self,
parse_config: &ParseConfig,
config: &Config,
binding_crate_name: &str,
crate_name: &str,
mod_cfg: Option<&Cfg>,
@@ -536,7 +512,10 @@ impl Parse {
for foreign_item in &item.items {
if let syn::ForeignItem::Fn(ref function) = *foreign_item {
if !parse_config.should_generate_top_level_item(crate_name, binding_crate_name) {
if !config
.parse
.should_generate_top_level_item(crate_name, binding_crate_name)
{
info!(
"Skip {}::{} - (fn's outside of the binding crate are not used).",
crate_name, &function.sig.ident
@@ -564,13 +543,16 @@ impl Parse {
/// Loads a `fn` declaration
fn load_syn_fn(
&mut self,
parse_config: &ParseConfig,
config: &Config,
binding_crate_name: &str,
crate_name: &str,
mod_cfg: Option<&Cfg>,
item: &syn::ItemFn,
) {
if !parse_config.should_generate_top_level_item(crate_name, binding_crate_name) {
if !config
.parse
.should_generate_top_level_item(crate_name, binding_crate_name)
{
info!(
"Skip {}::{} - (fn's outside of the binding crate are not used).",
crate_name, &item.sig.ident
@@ -682,13 +664,16 @@ impl Parse {
/// Loads a `const` declaration
fn load_syn_const(
&mut self,
parse_config: &ParseConfig,
config: &Config,
binding_crate_name: &str,
crate_name: &str,
mod_cfg: Option<&Cfg>,
item: &syn::ItemConst,
) {
if !parse_config.should_generate_top_level_item(crate_name, binding_crate_name) {
if !config
.parse
.should_generate_top_level_item(crate_name, binding_crate_name)
{
info!(
"Skip {}::{} - (const's outside of the binding crate are not used).",
crate_name, &item.ident
@@ -721,13 +706,16 @@ impl Parse {
/// Loads a `static` declaration
fn load_syn_static(
&mut self,
parse_config: &ParseConfig,
config: &Config,
binding_crate_name: &str,
crate_name: &str,
mod_cfg: Option<&Cfg>,
item: &syn::ItemStatic,
) {
if !parse_config.should_generate_top_level_item(crate_name, binding_crate_name) {
if !config
.parse
.should_generate_top_level_item(crate_name, binding_crate_name)
{
info!(
"Skip {}::{} - (static's outside of the binding crate are not used).",
crate_name, &item.ident
@@ -761,8 +749,14 @@ impl Parse {
}
/// Loads a `struct` declaration
fn load_syn_struct(&mut self, crate_name: &str, mod_cfg: Option<&Cfg>, item: &syn::ItemStruct) {
match Struct::load(item, mod_cfg) {
fn load_syn_struct(
&mut self,
config: &Config,
crate_name: &str,
mod_cfg: Option<&Cfg>,
item: &syn::ItemStruct,
) {
match Struct::load(&config.layout, item, mod_cfg) {
Ok(st) => {
info!("Take {}::{}.", crate_name, &item.ident);
self.structs.try_insert(st);
@@ -778,8 +772,14 @@ impl Parse {
}
/// Loads a `union` declaration
fn load_syn_union(&mut self, crate_name: &str, mod_cfg: Option<&Cfg>, item: &syn::ItemUnion) {
match Union::load(item, mod_cfg) {
fn load_syn_union(
&mut self,
config: &Config,
crate_name: &str,
mod_cfg: Option<&Cfg>,
item: &syn::ItemUnion,
) {
match Union::load(&config.layout, item, mod_cfg) {
Ok(st) => {
info!("Take {}::{}.", crate_name, &item.ident);
@@ -840,7 +840,7 @@ impl Parse {
fn load_builtin_macro(
&mut self,
macro_expansion_config: &MacroExpansionConfig,
config: &Config,
crate_name: &str,
mod_cfg: Option<&Cfg>,
item: &syn::ItemMacro,
@@ -850,7 +850,7 @@ impl Parse {
None => return,
};
if name != "bitflags" || !macro_expansion_config.bitflags {
if name != "bitflags" || !config.macro_expansion.bitflags {
return;
}
@@ -863,7 +863,7 @@ impl Parse {
};
let (struct_, impl_) = bitflags.expand();
self.load_syn_struct(crate_name, mod_cfg, &struct_);
self.load_syn_struct(config, crate_name, mod_cfg, &struct_);
// We know that the expansion will only reference `struct_`, so it's
// fine to just do it here instead of deferring it like we do with the
// other calls to this function.