Skip to content

Commit

Permalink
Don't derive Copy in FFI structs/unions that contain pointers
Browse files Browse the repository at this point in the history
Clone is still derived to as explicitly cloning the structs is less of a
problem, but implicitly copying them can easily cause double frees.

Fixes #1048
  • Loading branch information
sdroege committed Feb 4, 2021
1 parent cbdc2e4 commit 0dc1494
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 2 deletions.
88 changes: 86 additions & 2 deletions src/analysis/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,94 @@ pub trait DerivesCopy {
fn derives_copy(&self, lib: &Library) -> bool;
}

impl<T: IsIncomplete> DerivesCopy for T {
impl DerivesCopy for Fundamental {
fn derives_copy(&self, lib: &Library) -> bool {
// Copy is derived for all complete types.
!self.is_incomplete(lib)
&& !matches!(
self,
Fundamental::Pointer
| Fundamental::VarArgs
| Fundamental::Utf8
| Fundamental::Filename
| Fundamental::IntPtr
| Fundamental::UIntPtr
| Fundamental::OsString
| Fundamental::Unsupported
)
}
}

impl DerivesCopy for Alias {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib) && !self.is_ptr()
}
}

impl DerivesCopy for Field {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib) && !self.is_ptr()
}
}

impl<'a> DerivesCopy for &'a [Field] {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib) && !self.iter().any(|field| field.is_ptr())
}
}

impl DerivesCopy for Class {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib) && self.fields.as_slice().derives_copy(lib)
}
}

impl DerivesCopy for Record {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib) && self.fields.as_slice().derives_copy(lib)
}
}

impl DerivesCopy for Union {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib) && self.fields.as_slice().derives_copy(lib)
}
}

impl DerivesCopy for Function {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib)
}
}

impl DerivesCopy for TypeId {
fn derives_copy(&self, lib: &Library) -> bool {
!self.is_incomplete(lib) && lib.type_(*self).derives_copy(lib)
}
}

impl DerivesCopy for Type {
fn derives_copy(&self, lib: &Library) -> bool {
if self.is_incomplete(lib) {
return false;
}

match *self {
Type::Fundamental(ref fundamental) => fundamental.derives_copy(lib),
Type::Alias(ref alias) => alias.derives_copy(lib),
Type::FixedArray(tid, ..) => tid.derives_copy(lib),
Type::Class(ref klass) => klass.derives_copy(lib),
Type::Record(ref record) => record.derives_copy(lib),
Type::Union(ref union) => union.derives_copy(lib),
Type::Function(ref function) => function.derives_copy(lib),
Type::Enumeration(..) | Type::Bitfield(..) | Type::Interface(..) => true,
Type::Custom(..)
| Type::Array(..)
| Type::CArray(..)
| Type::PtrArray(..)
| Type::HashTable(..)
| Type::List(..)
| Type::SList(..) => false,
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/codegen/sys/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub struct Fields {
pub external: bool,
/// Reason for truncating the representation, if any.
pub truncated: Option<String>,
derives_clone: bool,
derives_copy: bool,
/// "struct" or "union"
pub kind: &'static str,
Expand All @@ -38,6 +39,8 @@ impl Fields {
let mut traits = Vec::new();
if self.derives_copy {
traits.push("Copy");
}
if self.derives_clone {
traits.push("Clone");
}
traits
Expand All @@ -58,11 +61,13 @@ impl FieldInfo {
pub fn from_record(env: &Env, record: &Record) -> Fields {
let (fields, truncated) = analyze_fields(env, false, &record.fields);
let derives_copy = truncated.is_none() && record.derives_copy(&env.library);
let derives_clone = truncated.is_none() && !record.is_incomplete(&env.library);
Fields {
name: record.c_type.clone(),
external: record.is_external(&env.library),
truncated,
derives_copy,
derives_clone,
kind: "struct",
cfg_condition: get_gobject_cfg_condition(env, &record.name),
fields,
Expand All @@ -72,11 +77,13 @@ pub fn from_record(env: &Env, record: &Record) -> Fields {
pub fn from_class(env: &Env, klass: &Class) -> Fields {
let (fields, truncated) = analyze_fields(env, false, &klass.fields);
let derives_copy = truncated.is_none() && klass.derives_copy(&env.library);
let derives_clone = truncated.is_none() && !klass.is_incomplete(&env.library);
Fields {
name: klass.c_type.clone(),
external: klass.is_external(&env.library),
truncated,
derives_copy,
derives_clone,
kind: "struct",
cfg_condition: get_gobject_cfg_condition(env, &klass.name),
fields,
Expand All @@ -86,11 +93,13 @@ pub fn from_class(env: &Env, klass: &Class) -> Fields {
pub fn from_union(env: &Env, union: &Union) -> Fields {
let (fields, truncated) = analyze_fields(env, true, &union.fields);
let derives_copy = truncated.is_none() && union.derives_copy(&env.library);
let derives_clone = truncated.is_none() && !union.is_incomplete(&env.library);
Fields {
name: union.c_type.as_ref().unwrap().clone(),
external: union.is_external(&env.library),
truncated,
derives_copy,
derives_clone,
kind: "union",
cfg_condition: None,
fields,
Expand Down

0 comments on commit 0dc1494

Please sign in to comment.