nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-23 15:23:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Implement uninit_vec lint

Browse Source
This commit is contained in:
Yechan Bae 2021-09-17 02:55:26 -04:00
parent 22144c02c2
commit 452181c69d
11 changed files with 278 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -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

1
clippy_lints/src/lib.register_all.rs
View File

@ -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),

1
clippy_lints/src/lib.register_correctness.rs
View File

@ -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),

1
clippy_lints/src/lib.register_lints.rs
View File

@ -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,

2
clippy_lints/src/lib.rs
View File

@ -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));

14
clippy_lints/src/methods/uninit_assumed_init.rs
View File

@ -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, is_uninit_ty_valid, paths};
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_ty_valid(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,
}
}

174
clippy_lints/src/uninit_vec.rs Normal file
View File

@ -0,0 +1,174 @@
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::{is_uninit_ty_valid, match_def_path, path_to_local_id, paths, peel_hir_expr_while, SpanlessEq};
use rustc_hir::def::Res;
use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;
declare_clippy_lint! {
/// ### What it does
/// Checks for the creation of uninitialized `Vec<T>` by calling `set_len()`
/// immediately after `with_capacity()` or `reserve()`.
///
/// ### Why is this bad?
/// It creates `Vec<T>` that contains uninitialized data, which leads to an
/// undefined behavior with most safe operations.
/// Notably, using uninitialized `Vec<u8>` with generic `Read` is unsound.
///
/// ### Example
/// ```rust,ignore
/// let mut vec: Vec<u8> = Vec::with_capacity(1000);
/// unsafe { vec.set_len(1000); }
/// reader.read(&mut vec); // undefined behavior!
/// ```
/// Use an initialized buffer:
/// ```rust,ignore
/// let mut vec: Vec<u8> = vec![0; 1000];
/// reader.read(&mut vec);
/// ```
/// Or, wrap the content in `MaybeUninit`:
/// ```rust,ignore
/// let mut vec: Vec<MaybeUninit<T>> = Vec::with_capacity(1000);
/// unsafe { vec.set_len(1000); }
/// ```
pub UNINIT_VEC,
correctness,
"Vec with uninitialized data"
}
declare_lint_pass!(UninitVec => [UNINIT_VEC]);
impl<'tcx> LateLintPass<'tcx> for UninitVec {
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
for w in block.stmts.windows(2) {
if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = w[1].kind {
handle_pair(cx, &w[0], expr);
}
}
if let (Some(stmt), Some(expr)) = (block.stmts.last(), block.expr) {
handle_pair(cx, stmt, expr);
}
}
}
fn handle_pair(cx: &LateContext<'tcx>, first: &'tcx Stmt<'tcx>, second: &'tcx Expr<'tcx>) {
if_chain! {
if let Some(vec) = extract_with_capacity_or_reserve_target(cx, first);
if let Some((set_len_self, call_span)) = extract_set_len_self(cx, second);
if vec.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();
// Check T of Vec<T>
if !is_uninit_ty_valid(cx, substs.type_at(0));
then {
span_lint_and_note(
cx,
UNINIT_VEC,
call_span,
"calling `set_len()` immediately after reserving a buffer creates uninitialized values",
Some(first.span),
"the buffer is reserved here"
);
}
}
}
#[derive(Clone, Copy, Debug)]
enum LocalOrExpr<'tcx> {
Local(HirId),
Expr(&'tcx Expr<'tcx>),
}
impl<'tcx> LocalOrExpr<'tcx> {
fn eq_expr(self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
match self {
LocalOrExpr::Local(hir_id) => path_to_local_id(expr, hir_id),
LocalOrExpr::Expr(self_expr) => SpanlessEq::new(cx).eq_expr(self_expr, expr),
}
}
}
/// Returns the target vec of Vec::with_capacity() or Vec::reserve()
fn extract_with_capacity_or_reserve_target(cx: &LateContext<'_>, stmt: &'tcx Stmt<'_>) -> Option<LocalOrExpr<'tcx>> {
match stmt.kind {
StmtKind::Local(local) => {
// let mut x = Vec::with_capacity()
if_chain! {
if let Some(init_expr) = local.init;
if let PatKind::Binding(_, hir_id, _, None) = local.pat.kind;
if is_with_capacity(cx, init_expr);
then {
Some(LocalOrExpr::Local(hir_id))
} else {
None
}
}
},
StmtKind::Expr(expr) | StmtKind::Semi(expr) => {
match expr.kind {
ExprKind::Assign(lhs, rhs, _span) if is_with_capacity(cx, rhs) => {
// self.vec = Vec::with_capacity()
Some(LocalOrExpr::Expr(lhs))
},
ExprKind::MethodCall(_, _, [vec_expr, _], _) => {
// self.vec.reserve()
if_chain! {
if let Some(id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if match_def_path(cx, id, &paths::VEC_RESERVE);
then {
Some(LocalOrExpr::Expr(vec_expr))
} else {
None
}
}
},
_ => None,
}
},
_ => None,
}
}
fn is_with_capacity(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> bool {
if_chain! {
if let ExprKind::Call(path_expr, _) = &expr.kind;
if let ExprKind::Path(qpath) = &path_expr.kind;
if let Res::Def(_, def_id) = cx.qpath_res(qpath, path_expr.hir_id);
then {
match_def_path(cx, def_id, &paths::VEC_WITH_CAPACITY)
} else {
false
}
}
}
/// 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 {
match (block.stmts.get(0).map(|stmt| &stmt.kind), block.expr) {
(None, Some(expr)) => Some(expr),
(Some(StmtKind::Expr(expr) | StmtKind::Semi(expr)), None) => Some(expr),
_ => None,
}
} else {
None
}
});
match expr.kind {
ExprKind::MethodCall(_, _, [vec_expr, _], _) => {
cx.typeck_results().type_dependent_def_id(expr.hir_id).and_then(|id| {
if match_def_path(cx, id, &paths::VEC_SET_LEN) {
Some((vec_expr, expr.span))
} else {
None
}
})
},
_ => None,
}
}

10
clippy_utils/src/lib.rs
View File

@ -1470,6 +1470,16 @@ pub fn is_self_ty(slf: &hir::Ty<'_>) -> bool {
false
}
/// Checks if a given type looks safe to be uninitialized.
pub fn is_uninit_ty_valid(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
match ty.kind() {
rustc_ty::Array(component, _) => is_uninit_ty_valid(cx, component),
rustc_ty::Tuple(types) => types.types().all(|ty| is_uninit_ty_valid(cx, ty)),
rustc_ty::Adt(adt, _) => match_def_path(cx, adt.did, &paths::MEM_MAYBEUNINIT),
_ => false,
}
}
pub fn iter_input_pats<'tcx>(decl: &FnDecl<'_>, body: &'tcx Body<'_>) -> impl Iterator<Item = &'tcx Param<'tcx>> {
(0..decl.inputs.len()).map(move |i| &body.params[i])
}

3
clippy_utils/src/paths.rs
View File

@ -183,5 +183,8 @@ pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];
pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"];
pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"];
pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];
pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"];
pub const VEC_WITH_CAPACITY: [&str; 4] = ["alloc", "vec", "Vec", "with_capacity"];
pub const VEC_SET_LEN: [&str; 4] = ["alloc", "vec", "Vec", "set_len"];
pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"];
pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];

32
tests/ui/uninit_vec.rs Normal file
View File

@ -0,0 +1,32 @@
#![warn(clippy::uninit_vec)]
use std::mem::MaybeUninit;
fn main() {
// with_capacity() -> set_len() should be detected
let mut vec: Vec<u8> = Vec::with_capacity(1000);
unsafe {
vec.set_len(200);
}
// reserve() -> set_len() should be detected
vec.reserve(1000);
unsafe {
vec.set_len(200);
}
// test when both calls are enclosed in the same unsafe block
unsafe {
let mut vec: Vec<u8> = Vec::with_capacity(1000);
vec.set_len(200);
vec.reserve(1000);
vec.set_len(200);
}
// MaybeUninit-wrapped types should not be detected
let mut vec: Vec<MaybeUninit<u8>> = Vec::with_capacity(1000);
unsafe {
vec.set_len(200);
}
}

51
tests/ui/uninit_vec.stderr Normal file
View File

@ -0,0 +1,51 @@
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:9:9
|
LL | vec.set_len(200);
| ^^^^^^^^^^^^^^^^
|
= note: `-D clippy::uninit-vec` implied by `-D warnings`
note: the buffer is reserved here
--> $DIR/uninit_vec.rs:7:5
|
LL | let mut vec: Vec<u8> = Vec::with_capacity(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:15:9
|
LL | vec.set_len(200);
| ^^^^^^^^^^^^^^^^
|
note: the buffer is reserved here
--> $DIR/uninit_vec.rs:13:5
|
LL | vec.reserve(1000);
| ^^^^^^^^^^^^^^^^^^
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:21:9
|
LL | vec.set_len(200);
| ^^^^^^^^^^^^^^^^
|
note: the buffer is reserved here
--> $DIR/uninit_vec.rs:20:9
|
LL | let mut vec: Vec<u8> = Vec::with_capacity(1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> $DIR/uninit_vec.rs:24:9
|
LL | vec.set_len(200);
| ^^^^^^^^^^^^^^^^
|
note: the buffer is reserved here
--> $DIR/uninit_vec.rs:23:9
|
LL | vec.reserve(1000);
| ^^^^^^^^^^^^^^^^^^
error: aborting due to 4 previous errors