add configuration for lint [mem_unsafe_functions];

finish implement lint [`mem_unsafe_functions`];
add simple description for unimplemented lints;

Signed-off-by: J-ZhengLi <[email protected]>

Author: J-ZhengLi

clippy_lints/src/guidelines/falliable_memory_allocation.rs

@@ -1,8 +1 @@
-use rustc_lint::{LateContext, LintContext};
 
-use super::FALLIABLE_MEMORY_ALLOCATION;
-
-// TODO: Adjust the parameters as necessary
-pub(super) fn check(cx: &LateContext<'_>) {
-    todo!();
-}

clippy_lints/src/guidelines/mem_unsafe_functions.rs

@@ -1,10 +1,45 @@
-use clippy_utils::diagnostics::span_lint;
+use clippy_utils::diagnostics::span_lint_and_help;
+use if_chain::if_chain;
+use rustc_hir::def::Res;
+use rustc_hir::def_id::DefIdSet;
+use rustc_hir::{Expr, ExprKind, Item, ItemKind, QPath};
 use rustc_lint::LateContext;
-use rustc_span::Span;
 
 use super::MEM_UNSAFE_FUNCTIONS;
 
-// TODO: Adjust the parameters as necessary
-pub(super) fn check(cx: &LateContext<'_>, span: Span) {
-    span_lint(cx, MEM_UNSAFE_FUNCTIONS, span, "it's working!");
+/// Check extern function definitions.
+///
+/// The main purpose of this function is to load `def_ids` of declared external functions.
+pub(super) fn check_foreign_item(item: &Item<'_>, blacklist: &[String], blacklist_ids: &mut DefIdSet) {
+    if let ItemKind::ForeignMod { items, .. } = item.kind {
+        for f_item in items {
+            if blacklist.contains(&f_item.ident.as_str().to_string()) {
+                let f_did = f_item.id.hir_id().owner.def_id.to_def_id();
+                blacklist_ids.insert(f_did);
+            }
+        }
+    }
+}
+
+/// Check function call expression
+///
+/// Will lint if the name of called function was blacklisted by the configuration.
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, blacklist_ids: &DefIdSet) {
+    if_chain! {
+        if let ExprKind::Call(fn_expr, _) = &expr.kind;
+        if let ExprKind::Path(qpath) = &fn_expr.kind;
+        if let QPath::Resolved(_, path) = qpath;
+        if let Res::Def(_, did) = path.res;
+        if blacklist_ids.contains(&did);
+        then {
+            span_lint_and_help(
+                cx,
+                MEM_UNSAFE_FUNCTIONS,
+                fn_expr.span,
+                "use of potentially dangerous memory manipulation function",
+                None,
+                "consider using its safe version",
+            );
+        }
+    }
 }

clippy_lints/src/guidelines/mod.rs

@@ -3,16 +3,18 @@ mod mem_unsafe_functions;
 mod passing_string_to_c_functions;
 mod untrusted_lib_loading;
 
+use clippy_utils::def_path_def_ids;
 use rustc_hir as hir;
+use rustc_hir::def_id::{DefId, DefIdSet};
 use rustc_hir::intravisit;
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::def_id::LocalDefId;
 use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Check for direct usage of external functions that modify memory
+    /// Checks for direct usage of external functions that modify memory
     /// without concerning about memory safety, such as `memcpy`, `strcpy`, `strcat` etc.
     ///
     /// ### Why is this bad?
@@ -49,7 +51,7 @@ declare_clippy_lint! {
     #[clippy::version = "1.70.0"]
     pub UNTRUSTED_LIB_LOADING,
     nursery,
-    "default lint description"
+    "attempt to load dynamic library from untrusted source"
 }
 
 declare_clippy_lint! {
@@ -68,7 +70,7 @@ declare_clippy_lint! {
     #[clippy::version = "1.70.0"]
     pub PASSING_STRING_TO_C_FUNCTIONS,
     nursery,
-    "default lint description"
+    "passing string or str to extern C function"
 }
 
 declare_clippy_lint! {
@@ -87,10 +89,25 @@ declare_clippy_lint! {
     #[clippy::version = "1.70.0"]
     pub FALLIABLE_MEMORY_ALLOCATION,
     nursery,
-    "default lint description"
+    "memory allocation without checking arguments and result"
 }
 
-declare_lint_pass!(GuidelineLints => [
+#[derive(Clone, Default)]
+pub struct GuidelineLints {
+    mem_uns_fns: Vec<String>,
+    mem_uns_fns_ty_ids: DefIdSet,
+}
+
+impl GuidelineLints {
+    pub fn new(mem_uns_fns: Vec<String>) -> Self {
+        Self {
+            mem_uns_fns,
+            mem_uns_fns_ty_ids: DefIdSet::new(),
+        }
+    }
+}
+
+impl_lint_pass!(GuidelineLints => [
     MEM_UNSAFE_FUNCTIONS,
     UNTRUSTED_LIB_LOADING,
     PASSING_STRING_TO_C_FUNCTIONS,
@@ -100,15 +117,43 @@ declare_lint_pass!(GuidelineLints => [
 impl<'tcx> LateLintPass<'tcx> for GuidelineLints {
     fn check_fn(
         &mut self,
-        cx: &LateContext<'tcx>,
+        _cx: &LateContext<'tcx>,
         _kind: intravisit::FnKind<'tcx>,
         _decl: &'tcx hir::FnDecl<'_>,
         _body: &'tcx hir::Body<'_>,
-        span: Span,
+        _span: Span,
         _def_id: LocalDefId,
     ) {
-        mem_unsafe_functions::check(cx, span);
     }
 
-    fn check_item(&mut self, _cx: &LateContext<'tcx>, _item: &'tcx hir::Item<'_>) {}
+    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
+        // Resolve function names to def_ids from configuration
+        for uns_fns in &self.mem_uns_fns {
+            // Path like function names such as `libc::foo` or `aa::bb::cc::bar`,
+            // this only works with dependencies.
+            if uns_fns.contains("::") {
+                let path: Vec<&str> = uns_fns.split("::").collect();
+                for did in def_path_def_ids(cx, path.as_slice()) {
+                    self.mem_uns_fns_ty_ids.insert(did);
+                }
+            }
+            // Plain function names, then we should take its libc variant into account
+            else if let Some(did) = libc_fn_def_id(cx, uns_fns) {
+                self.mem_uns_fns_ty_ids.insert(did);
+            }
+        }
+    }
+
+    fn check_item(&mut self, _cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
+        mem_unsafe_functions::check_foreign_item(item, &self.mem_uns_fns, &mut self.mem_uns_fns_ty_ids);
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+        mem_unsafe_functions::check(cx, expr, &self.mem_uns_fns_ty_ids);
+    }
+}
+
+fn libc_fn_def_id(cx: &LateContext<'_>, fn_name: &str) -> Option<DefId> {
+    let path = &["libc", fn_name];
+    def_path_def_ids(cx, path).next()
 }

clippy_lints/src/guidelines/passing_string_to_c_functions.rs

@@ -1,8 +1 @@
-use rustc_lint::{LateContext, LintContext};
 
-use super::PASSING_STRING_TO_C_FUNCTIONS;
-
-// TODO: Adjust the parameters as necessary
-pub(super) fn check(cx: &LateContext<'_>) {
-    todo!();
-}

clippy_lints/src/guidelines/untrusted_lib_loading.rs

@@ -1,8 +1 @@
-use rustc_lint::{LateContext, LintContext};
 
-use super::UNTRUSTED_LIB_LOADING;
-
-// TODO: Adjust the parameters as necessary
-pub(super) fn check(cx: &LateContext<'_>) {
-    todo!();
-}

clippy_lints/src/lib.rs

@@ -960,7 +960,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule));
     store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation));
     store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments));
-    store.register_late_pass(|_| Box::new(guidelines::GuidelineLints));
+    let mem_unsafe_functions = conf.mem_unsafe_functions.clone();
+    store.register_late_pass(move |_| Box::new(guidelines::GuidelineLints::new(mem_unsafe_functions.clone())));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/utils/conf.rs

@@ -32,6 +32,21 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[
 ];
 const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"];
 
+#[rustfmt::skip]
+const DEFAULT_MEM_UNSAFE_FUNCTIONS: &[&str] = &[
+    "memcpy", "bcopy", "wmemcpy", "memmove", "wmemmove",
+    "strcpy", "wcscpy", "strncpy", "wcsncpy",
+    "strcat", "wcscat", "strncat", "wcsncat",
+    "sprintf", "swprintf", "vsprintf", "vswprintf", "snprintf", "wsnprintf",
+    // formatted input functions
+    "scanf", "wscanf", "vscanf", "vwscanf", "fscanf", "fwscanf", "vfscanf", "vfwscanf",
+    "sscanf", "swscanf", "vsscanf", "vswscanf",
+    // standard input
+    "gets",
+    // memory initialization
+    "memset",
+];
+
 /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint.
 #[derive(Clone, Debug, Deserialize)]
 pub struct Rename {
@@ -463,6 +478,11 @@ define_Conf! {
     ///
     /// The maximum byte size a `Future` can have, before it triggers the `clippy::large_futures` lint
     (future_size_threshold: u64 = 16 * 1024),
+    /// Lint: MEM_UNSAFE_FUNCTIONS.
+    ///
+    /// The list of function names to lint about.
+    /// Providing empty list has the same effect as disabling this lint.
+    (mem_unsafe_functions: Vec<String> = super::DEFAULT_MEM_UNSAFE_FUNCTIONS.iter().map(ToString::to_string).collect()),
 }
 
 /// Search for the configuration file.

tests/ui-toml/mem_unsafe_functions/clippy.toml

@@ -0,0 +1 @@
+mem-unsafe-functions = [ "not_safe", "dont_use", "del_mem", "libc::memcpy", "memcpy" ]

tests/ui-toml/mem_unsafe_functions/mem_unsafe_functions.rs

@@ -0,0 +1,34 @@
+#![warn(clippy::mem_unsafe_functions)]
+#![allow(unused)]
+
+mod libc {
+    pub fn safe() {}
+    pub fn not_safe() {}
+    pub fn memcpy() {}
+}
+
+extern "C" {
+    fn safe();
+    fn safe_1();
+    fn not_safe();
+    fn dont_use();
+    fn del_mem();
+    fn memcpy();
+}
+
+fn main() {
+    unsafe {
+        // Don't trigger
+        safe();
+        safe_1();
+        libc::safe();
+        libc::not_safe();
+        libc::memcpy(); // because it's user defined
+
+        // should trigger
+        not_safe();
+        dont_use();
+        del_mem();
+        memcpy();
+    }
+}

tests/ui-toml/mem_unsafe_functions/mem_unsafe_functions.stderr

@@ -0,0 +1,35 @@
+error: use of potentially dangerous memory manipulation function
+  --> $DIR/mem_unsafe_functions.rs:29:9
+   |
+LL |         not_safe();
+   |         ^^^^^^^^
+   |
+   = help: consider using its safe version
+   = note: `-D clippy::mem-unsafe-functions` implied by `-D warnings`
+
+error: use of potentially dangerous memory manipulation function
+  --> $DIR/mem_unsafe_functions.rs:30:9
+   |
+LL |         dont_use();
+   |         ^^^^^^^^
+   |
+   = help: consider using its safe version
+
+error: use of potentially dangerous memory manipulation function
+  --> $DIR/mem_unsafe_functions.rs:31:9
+   |
+LL |         del_mem();
+   |         ^^^^^^^
+   |
+   = help: consider using its safe version
+
+error: use of potentially dangerous memory manipulation function
+  --> $DIR/mem_unsafe_functions.rs:32:9
+   |
+LL |         memcpy();
+   |         ^^^^^^
+   |
+   = help: consider using its safe version
+
+error: aborting due to 4 previous errors
+

tests/ui/mem_unsafe_functions.rs

@@ -1,7 +1,9 @@
 #![feature(c_size_t)]
 #![warn(clippy::mem_unsafe_functions)]
 
+use core::ffi::c_size_t as size_t;
 use core::ffi::{c_char, c_int, c_size_t, c_void};
+use std::ptr::{null, null_mut};
 
 // mock libc crate
 mod libc {
@@ -18,10 +20,42 @@ mod libc {
 
 extern "C" {
     fn strcpy(dst: *mut c_char, src: *const c_char) -> *mut c_char;
-    fn strncpy(dst: *mut c_char, src: *const c_char, n: c_size_t) -> *mut c_char;
-    fn memcpy(dest: *mut c_void, src: *const c_void, n: c_size_t) -> *mut c_void;
-    fn memmove(dest: *mut c_void, src: *const c_void, n: c_size_t) -> *mut c_void;
-    fn memset(dest: *mut c_void, c: c_int, n: c_size_t) -> *mut c_void;
+    fn strncpy(dst: *mut c_char, src: *const c_char, n: size_t) -> *mut c_char;
+    fn memcpy(dest: *mut c_void, src: *const c_void, n: size_t) -> *mut c_void;
+    fn memmove(dest: *mut c_void, src: *const c_void, n: size_t) -> *mut c_void;
+    fn memset(dest: *mut c_void, c: c_int, n: size_t) -> *mut c_void;
 }
 
-fn main() {}
+unsafe fn with_void_ptrs() {
+    let src: *const c_void = null();
+    let dst: *mut c_void = null_mut();
+    let n: size_t = 1;
+
+    // Should lint these
+    let _ = memcpy(dst, src, n);
+    let _ = memmove(dst, src, n);
+    let _ = memset(dst, 1, n);
+    let _ = libc::memcpy(dst, src, n);
+    let _ = libc::memmove(dst, src, n);
+    use libc::memset;
+    let _ = memset(dst, 1, n);
+}
+
+unsafe fn with_char_ptrs() {
+    let src: *const c_char = null();
+    let dst: *mut c_char = null_mut();
+
+    // Should lint these
+    let _ = strcpy(dst, src);
+    let _ = strncpy(dst, src, 1);
+    let _ = libc::strcpy(dst, src);
+    use libc::strncpy;
+    let _ = strncpy(dst, src, 1);
+}
+
+fn main() {
+    unsafe {
+        with_char_ptrs();
+        with_void_ptrs();
+    }
+}