Skip to content

Commit

Permalink
Added a unsafe_ffi_drop_implementations lint.
Browse files Browse the repository at this point in the history
This detects cases where a struct or enum are annotated with `#[repr(C)]`,
and *do not* have `#[unsafe_no_drop_flag]`, whereby it warns the user that
the type may not have the expected size or layout.

Also includes tests to ensure the lint is triggered by FFI+Drop structs
and enums, *not* triggered by FFI+Drop+unsafe_no_drop_flag structs
and enums, and *not* triggered by FFI+!Drop structs and enums.

It also contains a tangential change to libstd/sys/windows/backtrace.rs.
Specifically, the `Cleanup` type had `#[repr(C)]` and Drop, but was never
passed to any FFI function.
  • Loading branch information
DanielKeep committed Feb 10, 2015
1 parent de8bc44 commit 26fcbd7
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 1 deletion.
49 changes: 49 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2121,3 +2121,52 @@ impl LintPass for UnstableFeatures {
}
}
}

declare_lint! {
UNSAFE_FFI_DROP_IMPLEMENTATIONS,
Warn,
"ffi types that may have unexpected layout due to a Drop implementation"
}

/// Forbids FFI types implementing `Drop` without `#[unsafe_no_drop_flag]`.
#[derive(Copy)]
pub struct UnsafeFfiDropImplementations;

impl LintPass for UnsafeFfiDropImplementations {
fn get_lints(&self) -> LintArray {
lint_array!(UNSAFE_FFI_DROP_IMPLEMENTATIONS)
}

fn check_item(&mut self, cx: &Context, item: &ast::Item) {
// We only care about structs and enums.
match item.node {
ast::ItemEnum(..) | ast::ItemStruct(..) => (),
_ => return
}
let &ast::Item { ident, id, span, .. } = item;
let tcx = &cx.tcx;

// Is the type annotated with #[repr(C)]?
let s_ty = tcx.node_types.borrow()[id];
let def_id = match s_ty.sty {
ty::ty_enum(def_id, _) => def_id,
ty::ty_struct(def_id, _) => def_id,
_ => panic!("expected an enum or struct type")
};
let reprs = ty::lookup_repr_hints(tcx, def_id);
if !reprs.contains(&attr::ReprExtern) {
return;
}

// It is. Does it have #[unsafe_no_drop_flag]?
if !ty::ty_dtor(tcx, def_id).has_drop_flag() {
return;
}

// It doesn't. Oh *dear*.
cx.span_lint(UNSAFE_FFI_DROP_IMPLEMENTATIONS, span,
&format!("{} is marked for use with FFI, but has a `Drop` implementation; \
this may cause the type to have an unexpected size and layout",
ident));
}
}
1 change: 1 addition & 0 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ impl LintStore {
Stability,
UnconditionalRecursion,
PrivateNoMangleFns,
UnsafeFfiDropImplementations,
);

add_builtin_with_new!(sess,
Expand Down
1 change: 0 additions & 1 deletion src/libstd/sys/windows/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ mod arch {
}
}

#[repr(C)]
struct Cleanup {
handle: libc::HANDLE,
SymCleanup: SymCleanupFn,
Expand Down
37 changes: 37 additions & 0 deletions src/test/compile-fail/issue-18308.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![deny(unsafe_ffi_drop_implementations)]
#![allow(dead_code)]

extern {
fn f(x: *const FfiUnsafeStruct, y: *const FfiUnsafeEnum);
}

#[repr(C)]
struct FfiUnsafeStruct { //~ ERROR: unexpected size and layout
i: i32,
}

impl Drop for FfiUnsafeStruct {
fn drop(&mut self) {}
}

#[repr(C)]
enum FfiUnsafeEnum { //~ ERROR: unexpected size and layout
Kaboom = 0,
Splang = 1,
}

impl Drop for FfiUnsafeEnum {
fn drop(&mut self) {}
}

fn main() {}
51 changes: 51 additions & 0 deletions src/test/run-pass/issue-18308.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![deny(unsafe_ffi_drop_implementations)]
#![allow(dead_code)]

extern {
fn f(x: *const FfiSafeStruct, y: *const FfiSafeEnum);
}

#[repr(C)]
#[unsafe_no_drop_flag]
struct FfiSafeStruct {
i: i32,
}

impl Drop for FfiSafeStruct {
fn drop(&mut self) {}
}

#[repr(C)]
#[unsafe_no_drop_flag]
enum FfiSafeEnum {
Kaboom = 0,
Splang = 1,
}

impl Drop for FfiSafeEnum {
fn drop(&mut self) {}
}

// These two should not be affected as they have no Drop impl.
#[repr(C)]
struct FfiSafeStructNoDrop {
i: i32,
}

#[repr(C)]
enum FfiSafeEnumNoDrop {
Peace = 0,
WhaleSong = 1,
}

fn main() {}

0 comments on commit 26fcbd7

Please sign in to comment.