From 0e3dcd13765c5fed9795227ca75e15b8849ff5a4 Mon Sep 17 00:00:00 2001 From: mcarton Date: Tue, 7 Jun 2016 17:29:22 +0200 Subject: [PATCH] Improve `NOT_UNSAFE_PTR_ARG_DEREF` with functions --- README.md | 2 +- clippy_lints/src/functions.rs | 51 ++++++++++++++++++++------------- tests/compile-fail/functions.rs | 8 ++++++ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index b1f8e7aab..3159a3e89 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 157 lints included in this crate: +There are 158 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index c02f02f06..a1918aed6 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -88,7 +88,7 @@ impl LateLintPass for Functions { self.check_arg_number(cx, decl, span); } - self.check_raw_ptr(cx, unsafety, decl, block, span, nodeid); + self.check_raw_ptr(cx, unsafety, decl, block, nodeid); } fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) { @@ -96,7 +96,7 @@ impl LateLintPass for Functions { self.check_arg_number(cx, &sig.decl, item.span); if let Some(ref block) = *block { - self.check_raw_ptr(cx, sig.unsafety, &sig.decl, block, item.span, item.id); + self.check_raw_ptr(cx, sig.unsafety, &sig.decl, block, item.id); } } } @@ -113,7 +113,7 @@ impl Functions { } } - fn check_raw_ptr(&self, cx: &LateContext, unsafety: hir::Unsafety, decl: &hir::FnDecl, block: &hir::Block, span: Span, nodeid: ast::NodeId) { + fn check_raw_ptr(&self, cx: &LateContext, unsafety: hir::Unsafety, decl: &hir::FnDecl, block: &hir::Block, nodeid: ast::NodeId) { if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(nodeid) { let raw_ptrs = decl.inputs.iter().filter_map(|arg| raw_ptr_arg(cx, arg)).collect::>(); @@ -144,32 +144,43 @@ struct DerefVisitor<'a, 'tcx: 'a> { impl<'a, 'tcx, 'v> hir::intravisit::Visitor<'v> for DerefVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'v hir::Expr) { - let ptr = match expr.node { - hir::ExprUnary(hir::UnDeref, ref ptr) => Some(ptr), + match expr.node { + hir::ExprCall(ref f, ref args) => { + let ty = self.cx.tcx.expr_ty(f); + + if type_is_unsafe_function(ty) { + for arg in args { + self.check_arg(arg); + } + } + } hir::ExprMethodCall(_, _, ref args) => { let method_call = ty::MethodCall::expr(expr.id); let base_type = self.cx.tcx.tables.borrow().method_map[&method_call].ty; if type_is_unsafe_function(base_type) { - Some(&args[0]) - } else { - None - } - } - _ => None, - }; - - if let Some(ptr) = ptr { - if let Some(def) = self.cx.tcx.def_map.borrow().get(&ptr.id) { - if self.ptrs.contains(&def.def_id()) { - span_lint(self.cx, - NOT_UNSAFE_PTR_ARG_DEREF, - ptr.span, - "this public function dereferences a raw pointer but is not marked `unsafe`"); + for arg in args { + self.check_arg(arg); + } } } + hir::ExprUnary(hir::UnDeref, ref ptr) => self.check_arg(ptr), + _ => (), } hir::intravisit::walk_expr(self, expr); } } + +impl<'a, 'tcx: 'a> DerefVisitor<'a, 'tcx> { + fn check_arg(&self, ptr: &hir::Expr) { + if let Some(def) = self.cx.tcx.def_map.borrow().get(&ptr.id) { + if self.ptrs.contains(&def.def_id()) { + span_lint(self.cx, + NOT_UNSAFE_PTR_ARG_DEREF, + ptr.span, + "this public function dereferences a raw pointer but is not marked `unsafe`"); + } + } + } +} diff --git a/tests/compile-fail/functions.rs b/tests/compile-fail/functions.rs index 311da1ed2..f7ee41d28 100644 --- a/tests/compile-fail/functions.rs +++ b/tests/compile-fail/functions.rs @@ -36,6 +36,10 @@ impl Foo for Bar { fn ptr(p: *const u8) { println!("{}", unsafe { *p }); //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + println!("{:?}", unsafe { p.as_ref() }); + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + unsafe { std::ptr::read(p) }; + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` } } @@ -50,6 +54,8 @@ pub fn public(p: *const u8) { //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` println!("{:?}", unsafe { p.as_ref() }); //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + unsafe { std::ptr::read(p) }; + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` } impl Bar { @@ -62,6 +68,8 @@ impl Bar { //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` println!("{:?}", unsafe { p.as_ref() }); //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + unsafe { std::ptr::read(p) }; + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` } pub fn public_ok(self, p: *const u8) {