Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit a932eb3

Browse files
committed
Auto merge of #123239 - Urgau:dangerous_implicit_autorefs, r=jdonszelmann,traviscross
Implement a lint for implicit autoref of raw pointer dereference - take 2 *[t-lang nomination comment](#123239 (comment) This PR aims at implementing a lint for implicit autoref of raw pointer dereference, it is based on #103735 with suggestion and improvements from #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 #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 0134651 + 05f2b22 commit a932eb3

File tree

20 files changed

+626
-3
lines changed

20 files changed

+626
-3
lines changed

‎compiler/rustc_feature/src/builtin_attrs.rs‎

Lines changed: 4 additions & 0 deletions
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‎

Lines changed: 4 additions & 0 deletions
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‎

Lines changed: 153 additions & 0 deletions
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‎

Lines changed: 3 additions & 0 deletions
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‎

Lines changed: 20 additions & 0 deletions
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‎

Lines changed: 3 additions & 0 deletions
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‎

Lines changed: 1 addition & 0 deletions
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‎

Lines changed: 2 additions & 0 deletions
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‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 2 additions & 0 deletions
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
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /