From 5ec3ee944bfa8195dcbe1f08d36e88a87a620fe6 Mon Sep 17 00:00:00 2001 From: Tom Jakubowski Date: Sat, 20 Dec 2014 20:49:20 -0800 Subject: [PATCH] Make the improper_ctypes lint more robust It now checks extern fns in addition to foreign fns and checks `DefStruct` defs as well. There is room for improvement: for example, I believe that pointers should be safe in extern fn signatures regardless of the pointee's representation. The `FIXME` on `rust_begin_unwind_fmt` is because I don't think it should be, or needs to be, C ABI (cc @eddyb) and it takes a problematic `fmt::Arguments` structure by value. Fix #20098, fix #19834 --- src/librustc/lint/builtin.rs | 27 ++++++++++++------- src/libstd/rt/unwind.rs | 2 ++ .../compile-fail/lint-ctypes-xcrate-struct.rs | 23 ++++++++++++++++ src/test/compile-fail/lint-ctypes.rs | 25 ++++++++++++++++- 4 files changed, 67 insertions(+), 10 deletions(-) create mode 100644 src/test/compile-fail/lint-ctypes-xcrate-struct.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 3edcd0b515641..186cb2ce001e6 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -405,15 +405,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match self.cx.tcx.def_map.borrow()[path_id].clone() { def::DefPrimTy(ast::TyInt(ast::TyI)) => { self.cx.span_lint(IMPROPER_CTYPES, sp, - "found rust type `int` in foreign module, while \ - libc::c_int or libc::c_long should be used"); + "found rust type `int` in foreign module or \ + extern fn, while libc::c_int or libc::c_long \ + should be used"); } def::DefPrimTy(ast::TyUint(ast::TyU)) => { self.cx.span_lint(IMPROPER_CTYPES, sp, - "found rust type `uint` in foreign module, while \ - libc::c_uint or libc::c_ulong should be used"); + "found rust type `uint` in foreign module or \ + extern fn, while libc::c_uint or libc::c_ulong \ + should be used"); } - def::DefTy(..) => { + def::DefTy(..) | def::DefStruct(..) => { let tty = match self.cx.tcx.ast_ty_to_ty_cache.borrow().get(&ty_id) { Some(&ty::atttce_resolved(t)) => t, _ => panic!("ast_ty_to_ty_cache was incomplete after typeck!") @@ -422,8 +424,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if !ty::is_ffi_safe(self.cx.tcx, tty) { self.cx.span_lint(IMPROPER_CTYPES, sp, "found type without foreign-function-safe - representation annotation in foreign module, consider \ - adding a #[repr(...)] attribute to the type"); + representation annotation in foreign module or \ + extern fn, consider adding a #[repr(...)] \ + attribute to the type"); } } _ => () @@ -455,7 +458,7 @@ impl LintPass for ImproperCTypes { vis.visit_ty(ty); } - fn check_foreign_fn(cx: &Context, decl: &ast::FnDecl) { + fn check_fn(cx: &Context, decl: &ast::FnDecl) { for input in decl.inputs.iter() { check_ty(cx, &*input.ty); } @@ -468,11 +471,17 @@ impl LintPass for ImproperCTypes { ast::ItemForeignMod(ref nmod) if nmod.abi != abi::RustIntrinsic => { for ni in nmod.items.iter() { match ni.node { - ast::ForeignItemFn(ref decl, _) => check_foreign_fn(cx, &**decl), + ast::ForeignItemFn(ref decl, _) => check_fn(cx, &**decl), ast::ForeignItemStatic(ref t, _) => check_ty(cx, &**t) } } } + ast::ItemFn(ref decl, _, abi, _, _) => { + match abi { + abi::Rust | abi::RustIntrinsic | abi::RustCall => (), + _ => check_fn(cx, &**decl) + } + } _ => (), } } diff --git a/src/libstd/rt/unwind.rs b/src/libstd/rt/unwind.rs index 261a8335173d0..e2453c1a6c639 100644 --- a/src/libstd/rt/unwind.rs +++ b/src/libstd/rt/unwind.rs @@ -482,6 +482,7 @@ pub mod eabi { #[cfg(not(test))] /// Entry point of panic from the libcore crate. #[lang = "panic_fmt"] +#[allow(improper_ctypes)] // FIXME(tomjakubowski) pub extern fn rust_begin_unwind(msg: fmt::Arguments, file: &'static str, line: uint) -> ! { begin_unwind_fmt(msg, &(file, line)) @@ -492,6 +493,7 @@ pub extern fn rust_begin_unwind(msg: fmt::Arguments, #[cfg(not(test))] /// Entry point of panic from the libcore crate. #[lang = "panic_fmt"] +#[allow(improper_ctypes)] // FIXME(tomjakubowski) pub extern fn rust_begin_unwind(msg: &fmt::Arguments, file: &'static str, line: uint) -> ! { begin_unwind_fmt(msg, &(file, line)) diff --git a/src/test/compile-fail/lint-ctypes-xcrate-struct.rs b/src/test/compile-fail/lint-ctypes-xcrate-struct.rs new file mode 100644 index 0000000000000..8b02da8cca2fa --- /dev/null +++ b/src/test/compile-fail/lint-ctypes-xcrate-struct.rs @@ -0,0 +1,23 @@ +// Copyright 2014 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(improper_ctypes)] + +#[repr(C)] +pub struct Foo { + _x: String +} + +extern { + pub fn bad(f: Foo); //~ ERROR found type without foreign-function-safe +} + +fn main() { +} diff --git a/src/test/compile-fail/lint-ctypes.rs b/src/test/compile-fail/lint-ctypes.rs index 1755a9a2481b4..2e16559cbaf1e 100644 --- a/src/test/compile-fail/lint-ctypes.rs +++ b/src/test/compile-fail/lint-ctypes.rs @@ -1,4 +1,4 @@ -// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -12,15 +12,38 @@ extern crate libc; +#[deriving(Copy)] +pub struct Bad { + _x: u32 +} + +#[deriving(Copy)] +#[repr(C)] +pub struct Good { + _x: u32 +} + extern { pub fn bare_type1(size: int); //~ ERROR: found rust type pub fn bare_type2(size: uint); //~ ERROR: found rust type pub fn ptr_type1(size: *const int); //~ ERROR: found rust type pub fn ptr_type2(size: *const uint); //~ ERROR: found rust type + pub fn non_c_repr_type(b: Bad); //~ ERROR: found type without foreign-function-safe pub fn good1(size: *const libc::c_int); pub fn good2(size: *const libc::c_uint); + pub fn good3(g: Good); } +pub extern fn ex_bare_type1(_: int) {} //~ ERROR: found rust type +pub extern fn ex_bare_type2(_: uint) {} //~ ERROR: found rust type +pub extern fn ex_ptr_type1(_: *const int) {} //~ ERROR: found rust type +pub extern fn ex_ptr_type2(_: *const uint) {} //~ ERROR: found rust type +pub extern fn ex_non_c_repr_type(_: Bad) {} //~ ERROR: found type without foreign-function-safe + +pub extern fn ex_good1(_: *const libc::c_int) {} +pub extern fn ex_good2(_: *const libc::c_uint) {} +pub extern fn ex_good3(_: Good) {} + fn main() { }