Skip to content

Commit af9aa86

Browse files
committed
Auto merge of rust-lang#123239 - Urgau:dangerous_implicit_autorefs, r=jdonszelmann,traviscross
Implement a lint for implicit autoref of raw pointer dereference - take 2 *[t-lang nomination comment](rust-lang#123239 (comment) This PR aims at implementing a lint for implicit autoref of raw pointer dereference, it is based on rust-lang#103735 with suggestion and improvements from rust-lang#103735 (comment). The goal is to catch cases like this, where the user probably doesn't realise it just created a reference. ```rust pub struct Test { data: [u8], } pub fn test_len(t: *const Test) -> usize { unsafe { (*t).data.len() } // this calls <[T]>::len(&self) } ``` Since rust-lang#103735 already went 2 times through T-lang, where they T-lang ended-up asking for a more restricted version (which is what this PR does), I would prefer this PR to be reviewed first before re-nominating it for T-lang. ---- Compared to the PR it is as based on, this PR adds 3 restrictions on the outer most expression, which must either be: 1. A deref followed by any non-deref place projection (that intermediate deref will typically be auto-inserted) 2. A method call annotated with `#[rustc_no_implicit_refs]`. 3. A deref followed by a `addr_of!` or `addr_of_mut!`. See bottom of post for details. There are several points that are not 100% clear to me when implementing the modifications: - ~~"4. Any number of automatically inserted deref/derefmut calls." I as never able to trigger this. Am I missing something?~~ Fixed - Are "index" and "field" enough? ---- cc `@JakobDegen` `@WaffleLapkin` r? `@RalfJung` try-job: dist-various-1 try-job: dist-various-2
2 parents cb31a00 + 05f2b22 commit af9aa86

File tree

20 files changed

+626
-3
lines changed

20 files changed

+626
-3
lines changed

compiler/rustc_feature/src/builtin_attrs.rs

+4
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,10 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
918918
EncodeCrossCrate::Yes,
919919
"#[rustc_never_returns_null_ptr] is used to mark functions returning non-null pointers."
920920
),
921+
rustc_attr!(
922+
rustc_no_implicit_autorefs, AttributeType::Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::Yes,
923+
"`#[rustc_no_implicit_autorefs]` is used to mark functions for which an autoref to the dereference of a raw pointer should not be used as an argument."
924+
),
921925
rustc_attr!(
922926
rustc_coherence_is_core, AttributeType::CrateLevel, template!(Word), ErrorFollowing, EncodeCrossCrate::No,
923927
"#![rustc_coherence_is_core] allows inherent methods on builtin types, only intended to be used in `core`."

compiler/rustc_lint/messages.ftl

+4
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,10 @@ lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than pos
360360
lint_impl_trait_redundant_captures = all possible in-scope parameters are already captured, so `use<...>` syntax is redundant
361361
.suggestion = remove the `use<...>` syntax
362362
363+
lint_implicit_unsafe_autorefs = implicit autoref creates a reference to the dereference of a raw pointer
364+
.note = creating a reference requires the pointer target to be valid and imposes aliasing requirements
365+
.suggestion = try using a raw pointer method instead; or if this reference is intentional, make it explicit
366+
363367
lint_improper_ctypes = `extern` {$desc} uses type `{$ty}`, which is not FFI-safe
364368
.label = not FFI-safe
365369
.note = the type is defined here

compiler/rustc_lint/src/autorefs.rs

+153
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
use rustc_ast::{BorrowKind, UnOp};
2+
use rustc_hir::{Expr, ExprKind, Mutability};
3+
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, OverloadedDeref};
4+
use rustc_session::{declare_lint, declare_lint_pass};
5+
use rustc_span::sym;
6+
7+
use crate::lints::{ImplicitUnsafeAutorefsDiag, ImplicitUnsafeAutorefsSuggestion};
8+
use crate::{LateContext, LateLintPass, LintContext};
9+
10+
declare_lint! {
11+
/// The `dangerous_implicit_autorefs` lint checks for implicitly taken references
12+
/// to dereferences of raw pointers.
13+
///
14+
/// ### Example
15+
///
16+
/// ```rust
17+
/// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
18+
/// &raw mut (*ptr)[..16]
19+
/// // ^^^^^^ this calls `IndexMut::index_mut(&mut ..., ..16)`,
20+
/// // implicitly creating a reference
21+
/// }
22+
/// ```
23+
///
24+
/// {{produces}}
25+
///
26+
/// ### Explanation
27+
///
28+
/// When working with raw pointers it's usually undesirable to create references,
29+
/// since they inflict additional safety requirements. Unfortunately, it's possible
30+
/// to take a reference to the dereference of a raw pointer implicitly, which inflicts
31+
/// the usual reference requirements.
32+
///
33+
/// If you are sure that you can soundly take a reference, then you can take it explicitly:
34+
///
35+
/// ```rust
36+
/// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
37+
/// &raw mut (&mut *ptr)[..16]
38+
/// }
39+
/// ```
40+
///
41+
/// Otherwise try to find an alternative way to achive your goals using only raw pointers:
42+
///
43+
/// ```rust
44+
/// use std::slice;
45+
///
46+
/// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
47+
/// slice::from_raw_parts_mut(ptr.cast(), 16)
48+
/// }
49+
/// ```
50+
pub DANGEROUS_IMPLICIT_AUTOREFS,
51+
Warn,
52+
"implicit reference to a dereference of a raw pointer",
53+
report_in_external_macro
54+
}
55+
56+
declare_lint_pass!(ImplicitAutorefs => [DANGEROUS_IMPLICIT_AUTOREFS]);
57+
58+
impl<'tcx> LateLintPass<'tcx> for ImplicitAutorefs {
59+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
60+
// This logic has mostly been taken from
61+
// <https://github.com/rust-lang/rust/pull/103735#issuecomment-1370420305>
62+
63+
// 5. Either of the following:
64+
// a. A deref followed by any non-deref place projection (that intermediate
65+
// deref will typically be auto-inserted).
66+
// b. A method call annotated with `#[rustc_no_implicit_refs]`.
67+
// c. A deref followed by a `&raw const` or `&raw mut`.
68+
let mut is_coming_from_deref = false;
69+
let inner = match expr.kind {
70+
ExprKind::AddrOf(BorrowKind::Raw, _, inner) => match inner.kind {
71+
ExprKind::Unary(UnOp::Deref, inner) => {
72+
is_coming_from_deref = true;
73+
inner
74+
}
75+
_ => return,
76+
},
77+
ExprKind::Index(base, _, _) => base,
78+
ExprKind::MethodCall(_, inner, _, _)
79+
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
80+
&& cx.tcx.has_attr(def_id, sym::rustc_no_implicit_autorefs) =>
81+
{
82+
inner
83+
}
84+
ExprKind::Field(inner, _) => inner,
85+
_ => return,
86+
};
87+
88+
let typeck = cx.typeck_results();
89+
let adjustments_table = typeck.adjustments();
90+
91+
if let Some(adjustments) = adjustments_table.get(inner.hir_id)
92+
// 4. Any number of automatically inserted deref/derefmut calls.
93+
&& let adjustments = peel_derefs_adjustments(&**adjustments)
94+
// 3. An automatically inserted reference (might come from a deref).
95+
&& let [adjustment] = adjustments
96+
&& let Some(borrow_mutbl) = has_implicit_borrow(adjustment)
97+
&& let ExprKind::Unary(UnOp::Deref, dereferenced) =
98+
// 2. Any number of place projections.
99+
peel_place_mappers(inner).kind
100+
// 1. Deref of a raw pointer.
101+
&& typeck.expr_ty(dereferenced).is_raw_ptr()
102+
{
103+
cx.emit_span_lint(
104+
DANGEROUS_IMPLICIT_AUTOREFS,
105+
expr.span.source_callsite(),
106+
ImplicitUnsafeAutorefsDiag {
107+
suggestion: ImplicitUnsafeAutorefsSuggestion {
108+
mutbl: borrow_mutbl.ref_prefix_str(),
109+
deref: if is_coming_from_deref { "*" } else { "" },
110+
start_span: inner.span.shrink_to_lo(),
111+
end_span: inner.span.shrink_to_hi(),
112+
},
113+
},
114+
)
115+
}
116+
}
117+
}
118+
119+
/// Peels expressions from `expr` that can map a place.
120+
fn peel_place_mappers<'tcx>(mut expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
121+
loop {
122+
match expr.kind {
123+
ExprKind::Index(base, _idx, _) => expr = &base,
124+
ExprKind::Field(e, _) => expr = &e,
125+
_ => break expr,
126+
}
127+
}
128+
}
129+
130+
/// Peel derefs adjustments until the last last element.
131+
fn peel_derefs_adjustments<'a>(mut adjs: &'a [Adjustment<'a>]) -> &'a [Adjustment<'a>] {
132+
while let [Adjustment { kind: Adjust::Deref(_), .. }, end @ ..] = adjs
133+
&& !end.is_empty()
134+
{
135+
adjs = end;
136+
}
137+
adjs
138+
}
139+
140+
/// Test if some adjustment has some implicit borrow.
141+
///
142+
/// Returns `Some(mutability)` if the argument adjustment has implicit borrow in it.
143+
fn has_implicit_borrow(Adjustment { kind, .. }: &Adjustment<'_>) -> Option<Mutability> {
144+
match kind {
145+
&Adjust::Deref(Some(OverloadedDeref { mutbl, .. })) => Some(mutbl),
146+
&Adjust::Borrow(AutoBorrow::Ref(mutbl)) => Some(mutbl.into()),
147+
Adjust::NeverToAny
148+
| Adjust::Pointer(..)
149+
| Adjust::ReborrowPin(..)
150+
| Adjust::Deref(None)
151+
| Adjust::Borrow(AutoBorrow::RawPtr(..)) => None,
152+
}
153+
}

compiler/rustc_lint/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
mod async_closures;
3838
mod async_fn_in_trait;
39+
mod autorefs;
3940
pub mod builtin;
4041
mod context;
4142
mod dangling;
@@ -83,6 +84,7 @@ mod utils;
8384

8485
use async_closures::AsyncClosureUsage;
8586
use async_fn_in_trait::AsyncFnInTrait;
87+
use autorefs::*;
8688
use builtin::*;
8789
use dangling::*;
8890
use default_could_be_derived::DefaultCouldBeDerived;
@@ -200,6 +202,7 @@ late_lint_methods!(
200202
PathStatements: PathStatements,
201203
LetUnderscore: LetUnderscore,
202204
InvalidReferenceCasting: InvalidReferenceCasting,
205+
ImplicitAutorefs: ImplicitAutorefs,
203206
// Depends on referenced function signatures in expressions
204207
UnusedResults: UnusedResults,
205208
UnitBindings: UnitBindings,

compiler/rustc_lint/src/lints.rs

+20
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,26 @@ pub(crate) enum ShadowedIntoIterDiagSub {
5555
},
5656
}
5757

58+
// autorefs.rs
59+
#[derive(LintDiagnostic)]
60+
#[diag(lint_implicit_unsafe_autorefs)]
61+
#[note]
62+
pub(crate) struct ImplicitUnsafeAutorefsDiag {
63+
#[subdiagnostic]
64+
pub suggestion: ImplicitUnsafeAutorefsSuggestion,
65+
}
66+
67+
#[derive(Subdiagnostic)]
68+
#[multipart_suggestion(lint_suggestion, applicability = "maybe-incorrect")]
69+
pub(crate) struct ImplicitUnsafeAutorefsSuggestion {
70+
pub mutbl: &'static str,
71+
pub deref: &'static str,
72+
#[suggestion_part(code = "({mutbl}{deref}")]
73+
pub start_span: Span,
74+
#[suggestion_part(code = ")")]
75+
pub end_span: Span,
76+
}
77+
5878
// builtin.rs
5979
#[derive(LintDiagnostic)]
6080
#[diag(lint_builtin_while_true)]

compiler/rustc_passes/src/check_attr.rs

+3
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
179179
[sym::rustc_as_ptr, ..] => {
180180
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
181181
}
182+
[sym::rustc_no_implicit_autorefs, ..] => {
183+
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
184+
}
182185
[sym::rustc_never_returns_null_ptr, ..] => {
183186
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
184187
}

compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1830,6 +1830,7 @@ symbols! {
18301830
rustc_must_implement_one_of,
18311831
rustc_never_returns_null_ptr,
18321832
rustc_never_type_options,
1833+
rustc_no_implicit_autorefs,
18331834
rustc_no_mir_inline,
18341835
rustc_nonnull_optimization_guaranteed,
18351836
rustc_nounwind,

library/alloc/src/string.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1832,6 +1832,7 @@ impl String {
18321832
#[stable(feature = "rust1", since = "1.0.0")]
18331833
#[rustc_const_stable(feature = "const_vec_string_slice", since = "1.87.0")]
18341834
#[rustc_confusables("length", "size")]
1835+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
18351836
pub const fn len(&self) -> usize {
18361837
self.vec.len()
18371838
}
@@ -1851,6 +1852,7 @@ impl String {
18511852
#[must_use]
18521853
#[stable(feature = "rust1", since = "1.0.0")]
18531854
#[rustc_const_stable(feature = "const_vec_string_slice", since = "1.87.0")]
1855+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
18541856
pub const fn is_empty(&self) -> bool {
18551857
self.len() == 0
18561858
}

library/alloc/src/vec/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2588,7 +2588,7 @@ impl<T, A: Allocator> Vec<T, A> {
25882588
#[inline]
25892589
#[track_caller]
25902590
unsafe fn append_elements(&mut self, other: *const [T]) {
2591-
let count = unsafe { (*other).len() };
2591+
let count = other.len();
25922592
self.reserve(count);
25932593
let len = self.len();
25942594
unsafe { ptr::copy_nonoverlapping(other as *const T, self.as_mut_ptr().add(len), count) };

library/core/src/ops/index.rs

+2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ pub trait Index<Idx: ?Sized> {
6767
///
6868
/// May panic if the index is out of bounds.
6969
#[stable(feature = "rust1", since = "1.0.0")]
70+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
7071
#[track_caller]
7172
fn index(&self, index: Idx) -> &Self::Output;
7273
}
@@ -171,6 +172,7 @@ pub trait IndexMut<Idx: ?Sized>: Index<Idx> {
171172
///
172173
/// May panic if the index is out of bounds.
173174
#[stable(feature = "rust1", since = "1.0.0")]
175+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
174176
#[track_caller]
175177
fn index_mut(&mut self, index: Idx) -> &mut Self::Output;
176178
}

library/core/src/pin.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@
676676
//! let data_ptr = unpinned_src.data.as_ptr() as *const u8;
677677
//! let slice_ptr = unpinned_src.slice.as_ptr() as *const u8;
678678
//! let offset = slice_ptr.offset_from(data_ptr) as usize;
679-
//! let len = (*unpinned_src.slice.as_ptr()).len();
679+
//! let len = unpinned_src.slice.as_ptr().len();
680680
//!
681681
//! unpinned_self.slice = NonNull::from(&mut unpinned_self.data[offset..offset+len]);
682682
//! }

library/core/src/slice/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ impl<T> [T] {
109109
#[lang = "slice_len_fn"]
110110
#[stable(feature = "rust1", since = "1.0.0")]
111111
#[rustc_const_stable(feature = "const_slice_len", since = "1.39.0")]
112+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
112113
#[inline]
113114
#[must_use]
114115
pub const fn len(&self) -> usize {
@@ -128,6 +129,7 @@ impl<T> [T] {
128129
/// ```
129130
#[stable(feature = "rust1", since = "1.0.0")]
130131
#[rustc_const_stable(feature = "const_slice_is_empty", since = "1.39.0")]
132+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
131133
#[inline]
132134
#[must_use]
133135
pub const fn is_empty(&self) -> bool {
@@ -562,6 +564,7 @@ impl<T> [T] {
562564
/// assert_eq!(None, v.get(0..4));
563565
/// ```
564566
#[stable(feature = "rust1", since = "1.0.0")]
567+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
565568
#[inline]
566569
#[must_use]
567570
pub fn get<I>(&self, index: I) -> Option<&I::Output>
@@ -587,6 +590,7 @@ impl<T> [T] {
587590
/// assert_eq!(x, &[0, 42, 2]);
588591
/// ```
589592
#[stable(feature = "rust1", since = "1.0.0")]
593+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
590594
#[inline]
591595
#[must_use]
592596
pub fn get_mut<I>(&mut self, index: I) -> Option<&mut I::Output>
@@ -624,6 +628,7 @@ impl<T> [T] {
624628
/// }
625629
/// ```
626630
#[stable(feature = "rust1", since = "1.0.0")]
631+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
627632
#[inline]
628633
#[must_use]
629634
pub unsafe fn get_unchecked<I>(&self, index: I) -> &I::Output
@@ -666,6 +671,7 @@ impl<T> [T] {
666671
/// assert_eq!(x, &[1, 13, 4]);
667672
/// ```
668673
#[stable(feature = "rust1", since = "1.0.0")]
674+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
669675
#[inline]
670676
#[must_use]
671677
pub unsafe fn get_unchecked_mut<I>(&mut self, index: I) -> &mut I::Output

library/core/src/str/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ impl str {
134134
#[stable(feature = "rust1", since = "1.0.0")]
135135
#[rustc_const_stable(feature = "const_str_len", since = "1.39.0")]
136136
#[rustc_diagnostic_item = "str_len"]
137+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
137138
#[must_use]
138139
#[inline]
139140
pub const fn len(&self) -> usize {
@@ -153,6 +154,7 @@ impl str {
153154
/// ```
154155
#[stable(feature = "rust1", since = "1.0.0")]
155156
#[rustc_const_stable(feature = "const_str_is_empty", since = "1.39.0")]
157+
#[cfg_attr(not(bootstrap), rustc_no_implicit_autorefs)]
156158
#[must_use]
157159
#[inline]
158160
pub const fn is_empty(&self) -> bool {

library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ where
675675

676676
/// Obtain the number of elements in this user slice.
677677
pub fn len(&self) -> usize {
678-
unsafe { (*self.0.get()).len() }
678+
unsafe { self.0.get().len() }
679679
}
680680

681681
/// Copies the value from user memory and appends it to `dest`.

src/tools/miri/tests/pass/dst-raw.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Test DST raw pointers
22

3+
#![allow(dangerous_implicit_autorefs)]
4+
35
trait Trait {
46
fn foo(&self) -> isize;
57
}

src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![allow(dangerous_implicit_autorefs)]
2+
13
use std::cell::{Cell, Ref, RefCell, RefMut, UnsafeCell};
24
use std::mem::{self, MaybeUninit};
35

tests/ui/dynamically-sized-types/dst-raw.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//@ run-pass
22
// Test DST raw pointers
33

4+
#![allow(dangerous_implicit_autorefs)]
5+
46
trait Trait {
57
fn foo(&self) -> isize;
68
}

0 commit comments

Comments
 (0)