Skip to content

Commit

Permalink
Make the improper_ctypes lint more robust
Browse files Browse the repository at this point in the history
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 rust-lang#20098, fix rust-lang#19834
  • Loading branch information
tomjakubowski committed Dec 29, 2014
1 parent 19f73b4 commit 5ec3ee9
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 10 deletions.
27 changes: 18 additions & 9 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
Expand All @@ -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");
}
}
_ => ()
Expand Down Expand Up @@ -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);
}
Expand All @@ -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)
}
}
_ => (),
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/libstd/rt/unwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
23 changes: 23 additions & 0 deletions src/test/compile-fail/lint-ctypes-xcrate-struct.rs
Original file line number Diff line number Diff line change
@@ -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 <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(improper_ctypes)]

#[repr(C)]
pub struct Foo {
_x: String
}

extern {
pub fn bad(f: Foo); //~ ERROR found type without foreign-function-safe
}

fn main() {
}
25 changes: 24 additions & 1 deletion src/test/compile-fail/lint-ctypes.rs
Original file line number Diff line number Diff line change
@@ -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.
//
Expand All @@ -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() {
}

0 comments on commit 5ec3ee9

Please sign in to comment.