From 26fcbd76d8443c8836827eec8e548db7c2c984b8 Mon Sep 17 00:00:00 2001 From: Daniel Keep Date: Wed, 11 Feb 2015 00:50:09 +1100 Subject: [PATCH] Added a `unsafe_ffi_drop_implementations` lint. 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. --- src/librustc/lint/builtin.rs | 49 ++++++++++++++++++++++++++ src/librustc/lint/context.rs | 1 + src/libstd/sys/windows/backtrace.rs | 1 - src/test/compile-fail/issue-18308.rs | 37 ++++++++++++++++++++ src/test/run-pass/issue-18308.rs | 51 ++++++++++++++++++++++++++++ 5 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/issue-18308.rs create mode 100644 src/test/run-pass/issue-18308.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 638ecf4572d5d..d13e84d05eeb7 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -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)); + } +} diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 18628f6903f5d..6e6f0e1b14051 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -214,6 +214,7 @@ impl LintStore { Stability, UnconditionalRecursion, PrivateNoMangleFns, + UnsafeFfiDropImplementations, ); add_builtin_with_new!(sess, diff --git a/src/libstd/sys/windows/backtrace.rs b/src/libstd/sys/windows/backtrace.rs index 92e309da34bef..8da4f8b48318f 100644 --- a/src/libstd/sys/windows/backtrace.rs +++ b/src/libstd/sys/windows/backtrace.rs @@ -284,7 +284,6 @@ mod arch { } } -#[repr(C)] struct Cleanup { handle: libc::HANDLE, SymCleanup: SymCleanupFn, diff --git a/src/test/compile-fail/issue-18308.rs b/src/test/compile-fail/issue-18308.rs new file mode 100644 index 0000000000000..70655d7d33d38 --- /dev/null +++ b/src/test/compile-fail/issue-18308.rs @@ -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 or the MIT license +// , 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() {} diff --git a/src/test/run-pass/issue-18308.rs b/src/test/run-pass/issue-18308.rs new file mode 100644 index 0000000000000..c9bbec7547c5f --- /dev/null +++ b/src/test/run-pass/issue-18308.rs @@ -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 or the MIT license +// , 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() {}