Skip to content

Commit 3c08b6f

Browse files
committed
Initial implementation
1 parent b556398 commit 3c08b6f

File tree

4 files changed

+225
-0
lines changed

4 files changed

+225
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,6 +2839,7 @@ Released 2018-09-13
28392839
[`no_effect`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect
28402840
[`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
28412841
[`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
2842+
[`non_send_field_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_field_in_send_ty
28422843
[`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
28432844
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
28442845
[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces

clippy_lints/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ mod no_effect;
302302
mod non_copy_const;
303303
mod non_expressive_names;
304304
mod non_octal_unix_permissions;
305+
mod non_send_field_in_send_ty;
305306
mod nonstandard_macro_braces;
306307
mod open_options;
307308
mod option_env_unwrap;
@@ -867,6 +868,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
867868
non_expressive_names::MANY_SINGLE_CHAR_NAMES,
868869
non_expressive_names::SIMILAR_NAMES,
869870
non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS,
871+
non_send_field_in_send_ty::NON_SEND_FIELD_IN_SEND_TY,
870872
nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES,
871873
open_options::NONSENSICAL_OPEN_OPTIONS,
872874
option_env_unwrap::OPTION_ENV_UNWRAP,
@@ -1818,6 +1820,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
18181820
LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
18191821
LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
18201822
LintId::of(mutex_atomic::MUTEX_INTEGER),
1823+
LintId::of(non_send_field_in_send_ty::NON_SEND_FIELD_IN_SEND_TY),
18211824
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
18221825
LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
18231826
LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
@@ -2135,6 +2138,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
21352138
store.register_late_pass(move || Box::new(self_named_constructors::SelfNamedConstructors));
21362139
store.register_late_pass(move || Box::new(feature_name::FeatureName));
21372140
store.register_late_pass(move || Box::new(iter_not_returning_iterator::IterNotReturningIterator));
2141+
store.register_late_pass(|| Box::new(non_send_field_in_send_ty::NonSendFieldInSendTy));
21382142
}
21392143

21402144
#[rustfmt::skip]
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
2+
use clippy_utils::ty::{implements_trait, is_copy};
3+
use rustc_ast::ImplPolarity;
4+
use rustc_hir::{Item, ItemKind};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::sym;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Warns about a field in a `Send` struct that is neither `Send` nor `Copy`.
13+
///
14+
/// ### Why is this bad?
15+
/// Sending the struct to another thread and drops it there will also drop
16+
/// the field in the new thread. This effectively changes the ownership of
17+
/// the field type to the new thread and creates a soundness issue by
18+
/// breaking breaks the non-`Send` invariant.
19+
///
20+
/// ### Known Problems
21+
/// Data structures that contain raw pointers may cause false positives.
22+
/// They are sometimes safe to be sent across threads but do not implement
23+
/// the `Send` trait. This lint has a heuristic to filter out basic cases
24+
/// such as `Vec<*const T>`, but it's not perfect.
25+
///
26+
/// ### Example
27+
/// ```rust
28+
/// use std::sync::Arc;
29+
///
30+
/// // There is no `RC: Send` bound here
31+
/// unsafe impl<RC, T: Send> Send for ArcGuard<RC, T> {}
32+
///
33+
/// #[derive(Debug, Clone)]
34+
/// pub struct ArcGuard<RC, T> {
35+
/// inner: T,
36+
/// // Possibly drops `Arc<RC>` (and in turn `RC`) on a wrong thread
37+
/// head: Arc<RC>
38+
/// }
39+
/// ```
40+
pub NON_SEND_FIELD_IN_SEND_TY,
41+
nursery,
42+
"a field in a `Send` struct does not implement `Send`"
43+
}
44+
45+
declare_lint_pass!(NonSendFieldInSendTy => [NON_SEND_FIELD_IN_SEND_TY]);
46+
47+
impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
48+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
49+
let send_trait = cx.tcx.get_diagnostic_item(sym::send_trait).unwrap();
50+
51+
// Check if we are in `Send` impl item
52+
if_chain! {
53+
if let ItemKind::Impl(hir_impl) = &item.kind;
54+
if let Some(trait_ref) = &hir_impl.of_trait;
55+
if let Some(trait_id) = trait_ref.trait_def_id();
56+
if send_trait == trait_id;
57+
if let ImplPolarity::Positive = hir_impl.polarity;
58+
if let Some(ty_trait_ref) = cx.tcx.impl_trait_ref(item.def_id);
59+
if let self_ty = ty_trait_ref.self_ty();
60+
if let ty::Adt(adt_def, impl_trait_substs) = self_ty.kind();
61+
then {
62+
for variant in &adt_def.variants {
63+
for field in &variant.fields {
64+
let field_ty = field.ty(cx.tcx, impl_trait_substs);
65+
66+
// TODO: substs rebase_onto
67+
68+
if raw_pointer_in_ty_def(cx, field_ty)
69+
|| implements_trait(cx, field_ty, send_trait, &[])
70+
|| is_copy(cx, field_ty)
71+
{
72+
continue;
73+
}
74+
75+
if let Some(field_span) = cx.tcx.hir().span_if_local(field.did) {
76+
if is_ty_param(field_ty) {
77+
span_lint_and_help(
78+
cx,
79+
NON_SEND_FIELD_IN_SEND_TY,
80+
field_span,
81+
"a field in a `Send` struct does not implement `Send`",
82+
Some(item.span),
83+
&format!("add `{}: Send` in `Send` impl for `{}`", field_ty, self_ty),
84+
)
85+
} else {
86+
span_lint_and_note(
87+
cx,
88+
NON_SEND_FIELD_IN_SEND_TY,
89+
field_span,
90+
"a field in a `Send` struct does not implement `Send`",
91+
Some(item.span),
92+
&format!(
93+
"type `{}` doesn't implement `Send` when `{}` is `Send`",
94+
field_ty, self_ty
95+
),
96+
)
97+
}
98+
}
99+
}
100+
}
101+
}
102+
}
103+
}
104+
}
105+
106+
/// Returns `true` if the type itself is a raw pointer or has a raw pointer as a
107+
/// generic parameter, e.g., `Vec<*const u8>`.
108+
/// Note that it does not look into enum variants or struct fields.
109+
fn raw_pointer_in_ty_def<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
110+
for ty_node in target_ty.walk(cx.tcx) {
111+
if_chain! {
112+
if let GenericArgKind::Type(inner_ty) = ty_node.unpack();
113+
if let ty::RawPtr(_) = inner_ty.kind();
114+
then {
115+
return true;
116+
}
117+
}
118+
}
119+
120+
false
121+
}
122+
123+
/// Returns `true` if the type is a type parameter such as `T`.
124+
fn is_ty_param<'tcx>(target_ty: Ty<'tcx>) -> bool {
125+
matches!(target_ty.kind(), ty::Param(_))
126+
}

tests/ui/non_send_field_in_send_ty.rs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
#![warn(clippy::non_send_field_in_send_ty)]
2+
#![feature(extern_types)]
3+
4+
use std::cell::UnsafeCell;
5+
use std::ptr::NonNull;
6+
use std::sync::{Arc, Mutex};
7+
8+
// disrustor / RUSTSEC-2020-0150
9+
pub struct RingBuffer<T> {
10+
data: Vec<UnsafeCell<T>>,
11+
capacity: usize,
12+
mask: usize,
13+
}
14+
15+
unsafe impl<T> Send for RingBuffer<T> {}
16+
17+
// noise_search / RUSTSEC-2020-0141
18+
pub struct MvccRwLock<T> {
19+
raw: *const T,
20+
lock: Mutex<Box<T>>,
21+
}
22+
23+
unsafe impl<T> Send for MvccRwLock<T> {}
24+
25+
// async-coap / RUSTSEC-2020-0124
26+
pub struct ArcGuard<RC, T> {
27+
inner: T,
28+
head: Arc<RC>,
29+
}
30+
31+
unsafe impl<RC, T: Send> Send for ArcGuard<RC, T> {}
32+
33+
// rusb / RUSTSEC-2020-0098
34+
extern "C" {
35+
type libusb_device_handle;
36+
}
37+
38+
pub trait UsbContext {
39+
// some user trait that does not guarantee `Send`
40+
}
41+
42+
pub struct DeviceHandle<T: UsbContext> {
43+
context: T,
44+
handle: NonNull<libusb_device_handle>,
45+
}
46+
47+
unsafe impl<T: UsbContext> Send for DeviceHandle<T> {}
48+
49+
// Raw pointers are allowed
50+
extern "C" {
51+
type SomeFfiType;
52+
}
53+
54+
pub struct FpTest {
55+
vec: Vec<*const SomeFfiType>,
56+
}
57+
58+
unsafe impl Send for FpTest {}
59+
60+
// Check raw pointer false positive
61+
#[allow(clippy::non_send_field_in_send_ty)]
62+
pub struct AttrTest1<T>(T);
63+
64+
pub struct AttrTest2<T> {
65+
#[allow(clippy::non_send_field_in_send_ty)]
66+
field: T,
67+
}
68+
69+
pub enum AttrTest3<T> {
70+
#[allow(clippy::non_send_field_in_send_ty)]
71+
Enum1(T),
72+
Enum2(T),
73+
}
74+
75+
unsafe impl<T> Send for AttrTest1<T> {}
76+
unsafe impl<T> Send for AttrTest2<T> {}
77+
unsafe impl<T> Send for AttrTest3<T> {}
78+
79+
pub struct MultiField<T> {
80+
field1: T,
81+
field2: T,
82+
field3: T,
83+
}
84+
85+
unsafe impl<T> Send for MultiField<T> {}
86+
87+
pub enum MyOption<T> {
88+
MySome(T),
89+
MyNone,
90+
}
91+
92+
unsafe impl<T> Send for MyOption<T> {}
93+
94+
fn main() {}

0 commit comments

Comments
 (0)