Initial impl of raw_assign_to_drop

Fixes #4294

Author: lukaslueg

CHANGELOG.md

@@ -5930,6 +5930,7 @@ Released 2018-09-13
 [`range_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one
 [`range_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_step_by_zero
 [`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len
+[`raw_assign_to_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#raw_assign_to_drop
 [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer
 [`rc_clone_in_vec_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_clone_in_vec_init
 [`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex

clippy_lints/src/declared_lints.rs

@@ -599,6 +599,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::operators::NEEDLESS_BITWISE_BOOL_INFO,
     crate::operators::OP_REF_INFO,
     crate::operators::PTR_EQ_INFO,
+    crate::operators::RAW_ASSIGN_TO_DROP_INFO,
     crate::operators::REDUNDANT_COMPARISONS_INFO,
     crate::operators::SELF_ASSIGNMENT_INFO,
     crate::operators::VERBOSE_BIT_MASK_INFO,

clippy_lints/src/operators/mod.rs

@@ -18,6 +18,7 @@ mod needless_bitwise_bool;
 mod numeric_arithmetic;
 mod op_ref;
 mod ptr_eq;
+mod raw_assign_to_drop;
 mod self_assignment;
 mod verbose_bit_mask;
 
@@ -837,6 +838,45 @@ declare_clippy_lint! {
     "explicit self-assignment"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for assignments via raw pointers that involve types with destructors.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Assignments of the form `*ptr = new_value;` assume that `*ptr` contains an initialized
+    /// value, and unconditionally execute the `std::ops::Drop`-implementation if such
+    /// implementation is defined on the type of `*ptr`. If the value is in fact
+    /// uninitialized or otherwise invalid, the execution of `std::ops::Drop::drop(&mut self)`
+    /// is always Undefined Behavior.
+    ///
+    /// Use `std::ptr::write()` to overwrite a value without executing the destructor.
+    ///
+    /// Use `std::ptr::drop_in_place()` to conditionally execute the destructor if you are
+    /// sure that the place contains an initialized value.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// // Direct assignment always executes `String`'s destructor on `oldvalue`
+    /// *oldvalue = "New Value".to_owned();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// if oldvalue_is_initialized {
+    ///     // Having established that `oldvalue` points to a valid value, selectively
+    ///     // execute the destructor to prevent a memory-leak
+    ///     oldvalue.drop_in_place();
+    /// }
+    /// // Overwrite the old value without running the destructor unconditionally
+    /// oldvalue.write("New Value".to_owned());
+    /// ```
+    #[clippy::version = "1.85.0"]
+    pub RAW_ASSIGN_TO_DROP,
+    suspicious,
+    "assignment via raw pointer that involves destructors"
+}
+
 pub struct Operators {
     arithmetic_context: numeric_arithmetic::Context,
     verbose_bit_mask_threshold: u64,
@@ -879,6 +919,7 @@ impl_lint_pass!(Operators => [
     NEEDLESS_BITWISE_BOOL,
     PTR_EQ,
     SELF_ASSIGNMENT,
+    RAW_ASSIGN_TO_DROP,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Operators {
@@ -925,6 +966,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
             ExprKind::Assign(lhs, rhs, _) => {
                 assign_op_pattern::check(cx, e, lhs, rhs);
                 self_assignment::check(cx, e, lhs, rhs);
+                raw_assign_to_drop::check(cx, lhs);
             },
             ExprKind::Unary(op, arg) => {
                 if op == UnOp::Neg {

clippy_lints/src/operators/raw_assign_to_drop.rs

@@ -0,0 +1,32 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use rustc_hir::{Expr, ExprKind, UnOp};
+use rustc_lint::LateContext;
+
+use super::RAW_ASSIGN_TO_DROP;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, lhs: &'tcx Expr<'_>) {
+    if let ExprKind::Unary(UnOp::Deref, expr) = lhs.kind
+        && let ty = cx.typeck_results().expr_ty(expr)
+        && ty.is_unsafe_ptr()
+        && let Some(deref_ty) = ty.builtin_deref(true)
+        && deref_ty.needs_drop(cx.tcx, cx.typing_env())
+    {
+        span_lint_and_then(
+            cx,
+            RAW_ASSIGN_TO_DROP,
+            expr.span,
+            "assignment via raw pointer always executes destructor",
+            |diag| {
+                diag.note(format!(
+                    "the destructor defined by `{deref_ty}` is executed during assignment of the new value"
+                ));
+                diag.span_label(
+                    expr.span,
+                    "this place may be uninitialized, causing Undefined Behavior when the destructor executes",
+                );
+                diag.help("use `std::ptr::write()` to overwrite a (possibly uninitialized) place");
+                diag.help("use `std::ptr::drop_in_place()` to drop the previous value if such value exists");
+            },
+        );
+    }
+}

tests/ui/raw_assign_to_drop.rs

@@ -0,0 +1,23 @@
+#![warn(clippy::raw_assign_to_drop)]
+
+fn main() {
+    unsafe fn foo(r: *mut String, i: *mut i32) {
+        *r = "foo".to_owned();
+
+        // no lint on {integer}
+        *i = 47;
+
+        (*r, *r) = ("foo".to_owned(), "bar".to_owned());
+
+        (*r, *i) = ("foo".to_owned(), 47);
+
+        let mut x: String = Default::default();
+        *(&mut x as *mut _) = "Foo".to_owned();
+
+        // no lint on `u8`
+        *x.as_mut_ptr() = b'a';
+
+        let mut v: Vec<String> = vec![];
+        *v.as_mut_ptr() = Default::default();
+    }
+}

tests/ui/raw_assign_to_drop.stderr

@@ -0,0 +1,64 @@
+error: assignment via raw pointer always executes destructor
+  --> tests/ui/raw_assign_to_drop.rs:5:10
+   |
+LL |         *r = "foo".to_owned();
+   |          ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |
+   = note: the destructor defined by `std::string::String` is executed during assignment of the new value
+   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
+   = note: `-D clippy::raw-assign-to-drop` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::raw_assign_to_drop)]`
+
+error: assignment via raw pointer always executes destructor
+  --> tests/ui/raw_assign_to_drop.rs:10:11
+   |
+LL |         (*r, *r) = ("foo".to_owned(), "bar".to_owned());
+   |           ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |
+   = note: the destructor defined by `std::string::String` is executed during assignment of the new value
+   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
+
+error: assignment via raw pointer always executes destructor
+  --> tests/ui/raw_assign_to_drop.rs:10:15
+   |
+LL |         (*r, *r) = ("foo".to_owned(), "bar".to_owned());
+   |               ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |
+   = note: the destructor defined by `std::string::String` is executed during assignment of the new value
+   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
+
+error: assignment via raw pointer always executes destructor
+  --> tests/ui/raw_assign_to_drop.rs:12:11
+   |
+LL |         (*r, *i) = ("foo".to_owned(), 47);
+   |           ^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |
+   = note: the destructor defined by `std::string::String` is executed during assignment of the new value
+   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
+
+error: assignment via raw pointer always executes destructor
+  --> tests/ui/raw_assign_to_drop.rs:15:10
+   |
+LL |         *(&mut x as *mut _) = "Foo".to_owned();
+   |          ^^^^^^^^^^^^^^^^^^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |
+   = note: the destructor defined by `std::string::String` is executed during assignment of the new value
+   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
+
+error: assignment via raw pointer always executes destructor
+  --> tests/ui/raw_assign_to_drop.rs:21:10
+   |
+LL |         *v.as_mut_ptr() = Default::default();
+   |          ^^^^^^^^^^^^^^ this place may be uninitialized, causing Undefined Behavior when the destructor executes
+   |
+   = note: the destructor defined by `std::string::String` is executed during assignment of the new value
+   = help: use `std::ptr::write()` to overwrite a (possibly uninitialized) place
+   = help: use `std::ptr::drop_in_place()` to drop the previous value if such value exists
+
+error: aborting due to 6 previous errors
+