Skip to content

Commit

Permalink
Make the unsafe_sizeof_count_copies lint find copy_{from,to} method c…
Browse files Browse the repository at this point in the history 
…alls
  • Loading branch information
@nico-abram
nico-abram committed Nov 29, 2020
1 parent a06f548 commit ea8b52b
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 32 deletions.
66 changes: 48 additions & 18 deletions clippy_lints/src/unsafe_sizeof_count_copies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -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! {
Expand All @@ -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: &'static 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: &'static 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
);
}
};
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/unsafe_sizeof_count_copies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])) };

Expand Down
60 changes: 46 additions & 14 deletions tests/ui/unsafe_sizeof_count_copies.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit ea8b52b

Please sign in to comment.