Make the unsafe_sizeof_count_copies lint find copy_{from,to} method calls

nico-abram · nico-abram · commit 0f954babef41 · 2020-12-03T20:54:28.000-03:00
diff --git a/clippy_lints/src/unsafe_sizeof_count_copies.rs b/clippy_lints/src/unsafe_sizeof_count_copies.rs
@@ -6,7 +6,7 @@ use if_chain::if_chain;
 use rustc_hir::BinOpKind;
 use rustc_hir::{Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{Ty as TyM, TyS};
+use rustc_middle::ty::{self, Ty, TyS, TypeAndMut};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 declare_clippy_lint! {
@@ -40,7 +40,7 @@ declare_clippy_lint! {
 
 declare_lint_pass!(UnsafeSizeofCountCopies => [UNSAFE_SIZEOF_COUNT_COPIES]);
 
-fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<TyM<'tcx>> {
+fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Ty<'tcx>> {
     match &expr.kind {
         ExprKind::Call(ref count_func, _func_args) => {
             if_chain! {
@@ -62,35 +62,65 @@ fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<TyM<'t
     }
 }
 
+fn get_pointee_ty_and_count_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<(Ty<'tcx>, &'tcx Expr<'tcx>)> {
+    if_chain! {
+        // Find calls to ptr::copy and copy_nonoverlapping
+        if let ExprKind::Call(ref func, ref args) = expr.kind;
+        if let [_src, _dest, count] = &**args;
+        if let ExprKind::Path(ref func_qpath) = func.kind;
+        if let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id();
+        if match_def_path(cx, def_id, &paths::COPY_NONOVERLAPPING)
+            || match_def_path(cx, def_id, &paths::COPY);
+
+        // Get the pointee type
+        if let Some(pointee_ty) = cx.typeck_results().node_substs(func.hir_id).types().next();
+        then {
+            return Some((pointee_ty, count));
+        }
+    };
+    if_chain! {
+        // Find calls to copy_{from,to}{,_nonoverlapping}
+        if let ExprKind::MethodCall(ref method_path, _, ref args, _) = expr.kind;
+        if let [ptr_self, _, count] = &**args;
+        let method_ident = method_path.ident.as_str();
+        if method_ident== "copy_to" || method_ident == "copy_from"
+            || method_ident == "copy_to_nonoverlapping" || method_ident == "copy_from_nonoverlapping";
+
+        // Get the pointee type
+        if let ty::RawPtr(TypeAndMut { ty: pointee_ty, mutbl:_mutability }) =
+            cx.typeck_results().expr_ty(ptr_self).kind();
+        then {
+            return Some((pointee_ty, count));
+        }
+    };
+    None
+}
+
 impl<'tcx> LateLintPass<'tcx> for UnsafeSizeofCountCopies {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if_chain! {
-            // Find calls to ptr::copy and copy_nonoverlapping
-            if let ExprKind::Call(ref func, ref func_args) = expr.kind;
-            if let ExprKind::Path(ref func_qpath) = func.kind;
-            if let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id();
-            if match_def_path(cx, def_id, &paths::COPY_NONOVERLAPPING)
-                || match_def_path(cx, def_id, &paths::COPY);
+        const HELP_MSG: &str = "use a count of elements instead of a count of bytes \
+            for the count parameter, it already gets multiplied by the size of the pointed to type";
+
+        const LINT_MSG: &str = "unsafe memory copying using a byte count \
+            (Multiplied by size_of::<T>) instead of a count of T";
 
-            // Get the pointee type
-            let _substs = cx.typeck_results().node_substs(func.hir_id);
-            if let Some(pointee_ty) = cx.typeck_results().node_substs(func.hir_id).types().next();
+        if_chain! {
+            // Find calls to unsafe copy functions and get
+            // the pointee type and count parameter expression
+            if let Some((pointee_ty, count_expr)) = get_pointee_ty_and_count_expr(cx, expr);
 
             // Find a size_of call in the count parameter expression and
             // check that it's the same type
-            if let [_src, _dest, count] = &**func_args;
-            if let Some(ty_used_for_size_of) = get_size_of_ty(cx, count);
+            if let Some(ty_used_for_size_of) = get_size_of_ty(cx, count_expr);
             if TyS::same_type(pointee_ty, ty_used_for_size_of);
             then {
                 span_lint_and_help(
                     cx,
                     UNSAFE_SIZEOF_COUNT_COPIES,
                     expr.span,
-                    "unsafe memory copying using a byte count (Multiplied by size_of::<T>) \
-                    instead of a count of T",
+                    LINT_MSG,
                     None,
-                    "use a count of elements instead of a count of bytes for the count parameter, \
-                    it already gets multiplied by the size of the pointed to type"
+                    HELP_MSG
                 );
             }
         };
diff --git a/tests/ui/unsafe_sizeof_count_copies.rs b/tests/ui/unsafe_sizeof_count_copies.rs
@@ -14,6 +14,11 @@ fn main() {
     unsafe { copy_nonoverlapping::<u8>(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
 
+    unsafe { x.as_ptr().copy_to(y.as_mut_ptr(), size_of::<u8>()) };
+    unsafe { x.as_ptr().copy_to_nonoverlapping(y.as_mut_ptr(), size_of::<u8>()) };
+    unsafe { y.as_mut_ptr().copy_from(x.as_ptr(), size_of::<u8>()) };
+    unsafe { y.as_mut_ptr().copy_from_nonoverlapping(x.as_ptr(), size_of::<u8>()) };
+
     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
 
diff --git a/tests/ui/unsafe_sizeof_count_copies.stderr b/tests/ui/unsafe_sizeof_count_copies.stderr
@@ -18,114 +18,146 @@ LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
   --> $DIR/unsafe_sizeof_count_copies.rs:17:14
    |
+LL |     unsafe { x.as_ptr().copy_to(y.as_mut_ptr(), size_of::<u8>()) };
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
+
+error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
+  --> $DIR/unsafe_sizeof_count_copies.rs:18:14
+   |
+LL |     unsafe { x.as_ptr().copy_to_nonoverlapping(y.as_mut_ptr(), size_of::<u8>()) };
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
+
+error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
+  --> $DIR/unsafe_sizeof_count_copies.rs:19:14
+   |
+LL |     unsafe { y.as_mut_ptr().copy_from(x.as_ptr(), size_of::<u8>()) };
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
+
+error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
+  --> $DIR/unsafe_sizeof_count_copies.rs:20:14
+   |
+LL |     unsafe { y.as_mut_ptr().copy_from_nonoverlapping(x.as_ptr(), size_of::<u8>()) };
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
+
+error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
+  --> $DIR/unsafe_sizeof_count_copies.rs:22:14
+   |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:18:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:23:14
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:21:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:26:14
    |
 LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:22:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:27:14
    |
 LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0]) * SIZE) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:24:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:29:14
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:25:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:30:14
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0]) * SIZE) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:28:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:33:14
    |
 LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * HALF_SIZE * 2) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:29:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:34:14
    |
 LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), HALF_SIZE * size_of_val(&x[0]) * 2) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:31:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:36:14
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE * HALF_SIZE) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:32:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:37:14
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0]) * HALF_SIZE * 2) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:35:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:40:14
    |
 LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * DOUBLE_SIZE / 2) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:36:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:41:14
    |
 LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / 2 * size_of_val(&x[0])) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:38:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:43:14
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>() / 2) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
 error: unsafe memory copying using a byte count (Multiplied by size_of::<T>) instead of a count of T
-  --> $DIR/unsafe_sizeof_count_copies.rs:39:14
+  --> $DIR/unsafe_sizeof_count_copies.rs:44:14
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0]) * DOUBLE_SIZE / 2) };
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes for the count parameter, it already gets multiplied by the size of the pointed to type
 
-error: aborting due to 16 previous errors
+error: aborting due to 20 previous errors
 
