Implement uninit_vec lint

CHANGELOG.md

@@ -3038,6 +3038,7 @@ Released 2018-09-13
 [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc
 [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented
 [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init
+[`uninit_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_vec
 [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
 [`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp
 [`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord

clippy_lints/src/lib.register_all.rs

@@ -278,6 +278,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(types::VEC_BOX),
     LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
     LintId::of(unicode::INVISIBLE_CHARACTERS),
+    LintId::of(uninit_vec::UNINIT_VEC),
     LintId::of(unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
     LintId::of(unit_types::UNIT_ARG),
     LintId::of(unit_types::UNIT_CMP),

clippy_lints/src/lib.register_correctness.rs

@@ -63,6 +63,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
     LintId::of(transmuting_null::TRANSMUTING_NULL),
     LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
     LintId::of(unicode::INVISIBLE_CHARACTERS),
+    LintId::of(uninit_vec::UNINIT_VEC),
     LintId::of(unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
     LintId::of(unit_types::UNIT_CMP),
     LintId::of(unnamed_address::FN_ADDRESS_COMPARISONS),

clippy_lints/src/lib.register_lints.rs

@@ -470,6 +470,7 @@ store.register_lints(&[
     unicode::INVISIBLE_CHARACTERS,
     unicode::NON_ASCII_LITERAL,
     unicode::UNICODE_NOT_NFC,
+    uninit_vec::UNINIT_VEC,
     unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD,
     unit_types::LET_UNIT_VALUE,
     unit_types::UNIT_ARG,

clippy_lints/src/lib.rs

@@ -361,6 +361,7 @@ mod types;
 mod undocumented_unsafe_blocks;
 mod undropped_manually_drops;
 mod unicode;
+mod uninit_vec;
 mod unit_return_expecting_ord;
 mod unit_types;
 mod unnamed_address;
@@ -518,6 +519,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(blocks_in_if_conditions::BlocksInIfConditions));
     store.register_late_pass(|| Box::new(collapsible_match::CollapsibleMatch));
     store.register_late_pass(|| Box::new(unicode::Unicode));
+    store.register_late_pass(|| Box::new(uninit_vec::UninitVec));
     store.register_late_pass(|| Box::new(unit_return_expecting_ord::UnitReturnExpectingOrd));
     store.register_late_pass(|| Box::new(strings::StringAdd));
     store.register_late_pass(|| Box::new(implicit_return::ImplicitReturn));

clippy_lints/src/methods/uninit_assumed_init.rs

@@ -1,9 +1,8 @@
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::{is_expr_path_def_path, match_def_path, paths};
+use clippy_utils::{is_expr_path_def_path, paths, ty::is_uninit_value_valid_for_ty};
 use if_chain::if_chain;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
-use rustc_middle::ty::{self, Ty};
 
 use super::UNINIT_ASSUMED_INIT;
 
@@ -13,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
         if let hir::ExprKind::Call(callee, args) = recv.kind;
         if args.is_empty();
         if is_expr_path_def_path(cx, callee, &paths::MEM_MAYBEUNINIT_UNINIT);
-        if !is_maybe_uninit_ty_valid(cx, cx.typeck_results().expr_ty_adjusted(expr));
+        if !is_uninit_value_valid_for_ty(cx, cx.typeck_results().expr_ty_adjusted(expr));
         then {
             span_lint(
                 cx,
@@ -24,12 +23,3 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
         }
     }
 }
-
-fn is_maybe_uninit_ty_valid(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
-    match ty.kind() {
-        ty::Array(component, _) => is_maybe_uninit_ty_valid(cx, component),
-        ty::Tuple(types) => types.types().all(|ty| is_maybe_uninit_ty_valid(cx, ty)),
-        ty::Adt(adt, _) => match_def_path(cx, adt.did, &paths::MEM_MAYBEUNINIT),
-        _ => false,
-    }
-}

clippy_lints/src/uninit_vec.rs

@@ -0,0 +1,221 @@
+use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
+use clippy_utils::higher::{get_vec_init_kind, VecInitKind};
+use clippy_utils::ty::{is_type_diagnostic_item, is_uninit_value_valid_for_ty};
+use clippy_utils::{is_lint_allowed, path_to_local_id, peel_hir_expr_while, SpanlessEq};
+use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, PathSegment, Stmt, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{sym, Span};
+
+// TODO: add `ReadBuf` (RFC 2930) in "How to fix" once it is available in std
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `set_len()` call that creates `Vec` with uninitialized elements.
+    /// This is commonly caused by calling `set_len()` right after after calling
+    /// `with_capacity()` or `reserve()`.
+    ///
+    /// ### Why is this bad?
+    /// It creates a `Vec` with uninitialized data, which leads to
+    /// undefined behavior with most safe operations.
+    ///
+    /// Notably, uninitialized `Vec<u8>` must not be used with generic `Read`.
+    ///
+    /// ### Known Problems
+    /// This lint only checks directly adjacent statements.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// let mut vec: Vec<u8> = Vec::with_capacity(1000);
+    /// unsafe { vec.set_len(1000); }
+    /// reader.read(&mut vec); // undefined behavior!
+    /// ```
+    ///
+    /// ### How to fix?
+    /// 1. Use an initialized buffer:
+    ///    ```rust,ignore
+    ///    let mut vec: Vec<u8> = vec![0; 1000];
+    ///    reader.read(&mut vec);
+    ///    ```
+    /// 2. Wrap the content in `MaybeUninit`:
+    ///    ```rust,ignore
+    ///    let mut vec: Vec<MaybeUninit<T>> = Vec::with_capacity(1000);
+    ///    vec.set_len(1000);  // `MaybeUninit` can be uninitialized
+    ///    ```
+    /// 3. If you are on nightly, `Vec::spare_capacity_mut()` is available:
+    ///    ```rust,ignore
+    ///    let mut vec: Vec<u8> = Vec::with_capacity(1000);
+    ///    let remaining = vec.spare_capacity_mut();  // `&mut [MaybeUninit<u8>]`
+    ///    // perform initialization with `remaining`
+    ///    vec.set_len(...);  // Safe to call `set_len()` on initialized part
+    ///    ```
+    pub UNINIT_VEC,
+    correctness,
+    "Vec with uninitialized data"
+}
+
+declare_lint_pass!(UninitVec => [UNINIT_VEC]);
+
+// FIXME: update to a visitor-based implementation.
+// Threads: https://github.com/rust-lang/rust-clippy/pull/7682#discussion_r710998368
+impl<'tcx> LateLintPass<'tcx> for UninitVec {
+    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
+        if !in_external_macro(cx.tcx.sess, block.span) {
+            for w in block.stmts.windows(2) {
+                if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = w[1].kind {
+                    handle_uninit_vec_pair(cx, &w[0], expr);
+                }
+            }
+
+            if let (Some(stmt), Some(expr)) = (block.stmts.last(), block.expr) {
+                handle_uninit_vec_pair(cx, stmt, expr);
+            }
+        }
+    }
+}
+
+fn handle_uninit_vec_pair(
+    cx: &LateContext<'tcx>,
+    maybe_init_or_reserve: &'tcx Stmt<'tcx>,
+    maybe_set_len: &'tcx Expr<'tcx>,
+) {
+    if_chain! {
+        if let Some(vec) = extract_init_or_reserve_target(cx, maybe_init_or_reserve);
+        if let Some((set_len_self, call_span)) = extract_set_len_self(cx, maybe_set_len);
+        if vec.location.eq_expr(cx, set_len_self);
+        if let ty::Ref(_, vec_ty, _) = cx.typeck_results().expr_ty_adjusted(set_len_self).kind();
+        if let ty::Adt(_, substs) = vec_ty.kind();
+        // `#[allow(...)]` attribute can be set on enclosing unsafe block of `set_len()`
+        if !is_lint_allowed(cx, UNINIT_VEC, maybe_set_len.hir_id);
+        then {
+            if vec.has_capacity() {
+                // with_capacity / reserve -> set_len
+
+                // Check T of Vec<T>
+                if !is_uninit_value_valid_for_ty(cx, substs.type_at(0)) {
+                    // FIXME: #7698, false positive of the internal lints
+                    #[allow(clippy::collapsible_span_lint_calls)]
+                    span_lint_and_then(
+                        cx,
+                        UNINIT_VEC,
+                        vec![call_span, maybe_init_or_reserve.span],
+                        "calling `set_len()` immediately after reserving a buffer creates uninitialized values",
+                        |diag| {
+                            diag.help("initialize the buffer or wrap the content in `MaybeUninit`");
+                        },
+                    );
+                }
+            } else {
+                // new / default -> set_len
+                span_lint(
+                    cx,
+                    UNINIT_VEC,
+                    vec![call_span, maybe_init_or_reserve.span],
+                    "calling `set_len()` on empty `Vec` creates out-of-bound values",
+                );
+            }
+        }
+    }
+}
+
+/// The target `Vec` that is initialized or reserved
+#[derive(Clone, Copy)]
+struct TargetVec<'tcx> {
+    location: VecLocation<'tcx>,
+    /// `None` if `reserve()`
+    init_kind: Option<VecInitKind>,
+}
+
+impl TargetVec<'_> {
+    pub fn has_capacity(self) -> bool {
+        !matches!(self.init_kind, Some(VecInitKind::New | VecInitKind::Default))
+    }
+}
+
+#[derive(Clone, Copy)]
+enum VecLocation<'tcx> {
+    Local(HirId),
+    Expr(&'tcx Expr<'tcx>),
+}
+
+impl<'tcx> VecLocation<'tcx> {
+    pub fn eq_expr(self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
+        match self {
+            VecLocation::Local(hir_id) => path_to_local_id(expr, hir_id),
+            VecLocation::Expr(self_expr) => SpanlessEq::new(cx).eq_expr(self_expr, expr),
+        }
+    }
+}
+
+/// Finds the target location where the result of `Vec` initialization is stored
+/// or `self` expression for `Vec::reserve()`.
+fn extract_init_or_reserve_target<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) -> Option<TargetVec<'tcx>> {
+    match stmt.kind {
+        StmtKind::Local(local) => {
+            if_chain! {
+                if let Some(init_expr) = local.init;
+                if let PatKind::Binding(_, hir_id, _, None) = local.pat.kind;
+                if let Some(init_kind) = get_vec_init_kind(cx, init_expr);
+                then {
+                    return Some(TargetVec {
+                        location: VecLocation::Local(hir_id),
+                        init_kind: Some(init_kind),
+                    })
+                }
+            }
+        },
+        StmtKind::Expr(expr) | StmtKind::Semi(expr) => match expr.kind {
+            ExprKind::Assign(lhs, rhs, _span) => {
+                if let Some(init_kind) = get_vec_init_kind(cx, rhs) {
+                    return Some(TargetVec {
+                        location: VecLocation::Expr(lhs),
+                        init_kind: Some(init_kind),
+                    });
+                }
+            },
+            ExprKind::MethodCall(path, _, [self_expr, _], _) if is_reserve(cx, path, self_expr) => {
+                return Some(TargetVec {
+                    location: VecLocation::Expr(self_expr),
+                    init_kind: None,
+                });
+            },
+            _ => (),
+        },
+        StmtKind::Item(_) => (),
+    }
+    None
+}
+
+fn is_reserve(cx: &LateContext<'_>, path: &PathSegment<'_>, self_expr: &Expr<'_>) -> bool {
+    is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr).peel_refs(), sym::Vec)
+        && path.ident.name.as_str() == "reserve"
+}
+
+/// Returns self if the expression is `Vec::set_len()`
+fn extract_set_len_self(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<(&'tcx Expr<'tcx>, Span)> {
+    // peel unsafe blocks in `unsafe { vec.set_len() }`
+    let expr = peel_hir_expr_while(expr, |e| {
+        if let ExprKind::Block(block, _) = e.kind {
+            // Extract the first statement/expression
+            match (block.stmts.get(0).map(|stmt| &stmt.kind), block.expr) {
+                (None, Some(expr)) => Some(expr),
+                (Some(StmtKind::Expr(expr) | StmtKind::Semi(expr)), _) => Some(expr),
+                _ => None,
+            }
+        } else {
+            None
+        }
+    });
+    match expr.kind {
+        ExprKind::MethodCall(path, _, [self_expr, _], _) => {
+            let self_type = cx.typeck_results().expr_ty(self_expr).peel_refs();
+            if is_type_diagnostic_item(cx, self_type, sym::Vec) && path.ident.name.as_str() == "set_len" {
+                Some((self_expr, expr.span))
+            } else {
+                None
+            }
+        },
+        _ => None,
+    }
+}

clippy_lints/src/vec_init_then_push.rs

@@ -1,16 +1,14 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher::{get_vec_init_kind, VecInitKind};
 use clippy_utils::source::snippet;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{match_def_path, path_to_local, path_to_local_id, paths};
+use clippy_utils::{path_to_local, path_to_local_id};
 use if_chain::if_chain;
-use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
-use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, QPath, Stmt, StmtKind};
+use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::{symbol::sym, Span};
-use std::convert::TryInto;
+use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -41,11 +39,6 @@ pub struct VecInitThenPush {
     searcher: Option<VecPushSearcher>,
 }
 
-#[derive(Clone, Copy)]
-enum VecInitKind {
-    New,
-    WithCapacity(u64),
-}
 struct VecPushSearcher {
     local_id: HirId,
     init: VecInitKind,
@@ -58,7 +51,8 @@ impl VecPushSearcher {
     fn display_err(&self, cx: &LateContext<'_>) {
         match self.init {
             _ if self.found == 0 => return,
-            VecInitKind::WithCapacity(x) if x > self.found => return,
+            VecInitKind::WithLiteralCapacity(x) if x > self.found => return,
+            VecInitKind::WithExprCapacity(_) => return,
             _ => (),
         };
 
@@ -152,37 +146,3 @@ impl LateLintPass<'_> for VecInitThenPush {
         }
     }
 }
-
-fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<VecInitKind> {
-    if let ExprKind::Call(func, args) = expr.kind {
-        match func.kind {
-            ExprKind::Path(QPath::TypeRelative(ty, name))
-                if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec) =>
-            {
-                if name.ident.name == sym::new {
-                    return Some(VecInitKind::New);
-                } else if name.ident.name.as_str() == "with_capacity" {
-                    return args.get(0).and_then(|arg| {
-                        if_chain! {
-                            if let ExprKind::Lit(lit) = &arg.kind;
-                            if let LitKind::Int(num, _) = lit.node;
-                            then {
-                                Some(VecInitKind::WithCapacity(num.try_into().ok()?))
-                            } else {
-                                None
-                            }
-                        }
-                    });
-                }
-            }
-            ExprKind::Path(QPath::Resolved(_, path))
-                if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD)
-                    && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Vec) =>
-            {
-                return Some(VecInitKind::New);
-            }
-            _ => (),
-        }
-    }
-    None
-}