Implement lint against dangerous implicit autorefs

Author: Urgau

compiler/rustc_lint/messages.ftl

@@ -280,6 +280,10 @@ lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than pos
 lint_impl_trait_redundant_captures = all possible in-scope parameters are already captured, so `use<...>` syntax is redundant
     .suggestion = remove the `use<...>` syntax
 
+lint_implicit_unsafe_autorefs = implicit auto-ref creates a reference to a dereference of a raw pointer
+    .note = creating a reference requires the pointer to be valid and imposes aliasing requirements
+    .suggestion = try using a raw pointer method instead; or if this reference is intentional, make it explicit
+
 lint_improper_ctypes = `extern` {$desc} uses type `{$ty}`, which is not FFI-safe
     .label = not FFI-safe
     .note = the type is defined here

compiler/rustc_lint/src/autorefs.rs

@@ -0,0 +1,180 @@
+use rustc_ast::{BorrowKind, UnOp};
+use rustc_hir::{Expr, ExprKind, Mutability};
+use rustc_middle::ty::{
+    adjustment::{Adjust, Adjustment, AutoBorrow, OverloadedDeref},
+    TyCtxt, TypeckResults,
+};
+use rustc_session::declare_lint;
+use rustc_session::declare_lint_pass;
+use rustc_span::sym;
+
+use crate::{
+    lints::{ImplicitUnsafeAutorefsDiag, ImplicitUnsafeAutorefsSuggestion},
+    LateContext, LateLintPass, LintContext,
+};
+
+declare_lint! {
+    /// The `dangerous_implicit_autorefs` lint checks for implicitly taken references
+    /// to dereferences of raw pointers.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// use std::ptr::addr_of_mut;
+    ///
+    /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
+    ///     addr_of_mut!((*ptr)[..16])
+    ///     //                 ^^^^^^ this calls `IndexMut::index_mut(&mut ..., ..16)`,
+    ///     //                        implicitly creating a reference
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// When working with raw pointers it's usually undesirable to create references,
+    /// since they inflict a lot of safety requirement. Unfortunately, it's possible
+    /// to take a reference to a dereference of a raw pointer implicitly, which inflicts
+    /// the usual reference requirements without you even knowing that.
+    ///
+    /// If you are sure, you can soundly take a reference, then you can take it explicitly:
+    /// ```rust
+    /// # use std::ptr::addr_of_mut;
+    /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
+    ///     addr_of_mut!((&mut *ptr)[..16])
+    /// }
+    /// ```
+    ///
+    /// Otherwise try to find an alternative way to achive your goals that work only with
+    /// raw pointers:
+    /// ```rust
+    /// #![feature(slice_ptr_get)]
+    ///
+    /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
+    ///     ptr.get_unchecked_mut(..16)
+    /// }
+    /// ```
+    pub DANGEROUS_IMPLICIT_AUTOREFS,
+    Warn,
+    "implicit reference to a dereference of a raw pointer",
+    report_in_external_macro
+}
+
+declare_lint_pass!(ImplicitAutorefs => [DANGEROUS_IMPLICIT_AUTOREFS]);
+
+impl<'tcx> LateLintPass<'tcx> for ImplicitAutorefs {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        // This logic has mostly been taken from
+        // https://github.com/rust-lang/rust/pull/103735#issuecomment-1370420305
+
+        // 4. Either of the following:
+        //   a. A deref followed by any non-deref place projection (that intermediate
+        //   deref will typically be auto-inserted)
+        //   b. A method call annotated with `#[rustc_no_implicit_refs]`.
+        //   c. A deref followed by a `addr_of!` or `addr_of_mut!`.
+        let mut is_coming_from_deref = false;
+        let inner = match expr.kind {
+            ExprKind::AddrOf(BorrowKind::Raw, _, inner) => match inner.kind {
+                ExprKind::Unary(UnOp::Deref, inner) => {
+                    is_coming_from_deref = true;
+                    inner
+                }
+                _ => return,
+            },
+            ExprKind::Index(base, _idx, _) => base,
+            ExprKind::MethodCall(_, inner, _, _)
+                if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
+                    && cx.tcx.has_attr(def_id, sym::rustc_no_implicit_autorefs) =>
+            {
+                inner
+            }
+            ExprKind::Call(path, [expr, ..])
+                if let ExprKind::Path(ref qpath) = path.kind
+                    && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
+                    && cx.tcx.has_attr(def_id, sym::rustc_no_implicit_autorefs) =>
+            {
+                expr
+            }
+            ExprKind::Field(inner, _) => {
+                let typeck = cx.typeck_results();
+                let adjustments_table = typeck.adjustments();
+                if let Some(adjustments) = adjustments_table.get(inner.hir_id)
+                    && let [adjustment] = &**adjustments
+                    && let &Adjust::Deref(Some(OverloadedDeref { .. })) = &adjustment.kind
+                {
+                    inner
+                } else {
+                    return;
+                }
+            }
+            _ => return,
+        };
+
+        let typeck = cx.typeck_results();
+        let adjustments_table = typeck.adjustments();
+
+        if let Some(adjustments) = adjustments_table.get(inner.hir_id)
+            && let [adjustment] = &**adjustments
+            // 3. An automatically inserted reference.
+            && let Some((mutbl, _implicit_borrow)) = has_implicit_borrow(adjustment)
+            && let ExprKind::Unary(UnOp::Deref, dereferenced) =
+                // 2. Any number of place projections
+                peel_place_mappers(cx.tcx, typeck, inner).kind
+            // 1. Deref of a raw pointer
+            && typeck.expr_ty(dereferenced).is_unsafe_ptr()
+        {
+            cx.emit_span_lint(
+                DANGEROUS_IMPLICIT_AUTOREFS,
+                expr.span.source_callsite(),
+                ImplicitUnsafeAutorefsDiag {
+                    suggestion: ImplicitUnsafeAutorefsSuggestion {
+                        mutbl: mutbl.ref_prefix_str(),
+                        deref: if is_coming_from_deref { "*" } else { "" },
+                        start_span: inner.span.shrink_to_lo(),
+                        end_span: inner.span.shrink_to_hi(),
+                    },
+                },
+            )
+        }
+    }
+}
+
+/// Peels expressions from `expr` that can map a place.
+fn peel_place_mappers<'tcx>(
+    _tcx: TyCtxt<'tcx>,
+    _typeck: &TypeckResults<'tcx>,
+    mut expr: &'tcx Expr<'tcx>,
+) -> &'tcx Expr<'tcx> {
+    loop {
+        match expr.kind {
+            ExprKind::Index(base, _idx, _) => {
+                expr = &base;
+            }
+            ExprKind::Field(e, _) => expr = &e,
+            _ => break expr,
+        }
+    }
+}
+
+enum ImplicitBorrowKind {
+    Deref,
+    Borrow,
+}
+
+/// Test if some adjustment has some implicit borrow
+///
+/// Returns `Some(mutability)` if the argument adjustment has implicit borrow in it.
+fn has_implicit_borrow(
+    Adjustment { kind, .. }: &Adjustment<'_>,
+) -> Option<(Mutability, ImplicitBorrowKind)> {
+    match kind {
+        &Adjust::Deref(Some(OverloadedDeref { mutbl, .. })) => {
+            Some((mutbl, ImplicitBorrowKind::Deref))
+        }
+        &Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) => {
+            Some((mutbl.into(), ImplicitBorrowKind::Borrow))
+        }
+        _ => None,
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -44,6 +44,7 @@ extern crate tracing;
 
 mod array_into_iter;
 mod async_fn_in_trait;
+mod autorefs;
 pub mod builtin;
 mod context;
 mod deref_into_dyn_supertrait;
@@ -89,6 +90,7 @@ use rustc_middle::ty::TyCtxt;
 
 use array_into_iter::ArrayIntoIter;
 use async_fn_in_trait::AsyncFnInTrait;
+use autorefs::*;
 use builtin::*;
 use deref_into_dyn_supertrait::*;
 use drop_forget_useless::*;
@@ -192,6 +194,7 @@ late_lint_methods!(
             PathStatements: PathStatements,
             LetUnderscore: LetUnderscore,
             InvalidReferenceCasting: InvalidReferenceCasting,
+            ImplicitAutorefs: ImplicitAutorefs,
             // Depends on referenced function signatures in expressions
             UnusedResults: UnusedResults,
             UnitBindings: UnitBindings,

compiler/rustc_lint/src/lints.rs

@@ -50,6 +50,26 @@ pub enum ArrayIntoIterDiagSub {
     },
 }
 
+// autorefs.rs
+#[derive(LintDiagnostic)]
+#[diag(lint_implicit_unsafe_autorefs)]
+#[note]
+pub struct ImplicitUnsafeAutorefsDiag {
+    #[subdiagnostic]
+    pub suggestion: ImplicitUnsafeAutorefsSuggestion,
+}
+
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(lint_suggestion, applicability = "maybe-incorrect")]
+pub struct ImplicitUnsafeAutorefsSuggestion {
+    pub mutbl: &'static str,
+    pub deref: &'static str,
+    #[suggestion_part(code = "({mutbl}{deref}")]
+    pub start_span: Span,
+    #[suggestion_part(code = ")")]
+    pub end_span: Span,
+}
+
 // builtin.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_builtin_while_true)]

library/alloc/src/vec/mod.rs

@@ -2141,7 +2141,7 @@ impl<T, A: Allocator> Vec<T, A> {
     #[cfg(not(no_global_oom_handling))]
     #[inline]
     unsafe fn append_elements(&mut self, other: *const [T]) {
-        let count = unsafe { (*other).len() };
+        let count = other.len();
         self.reserve(count);
         let len = self.len();
         unsafe { ptr::copy_nonoverlapping(other as *const T, self.as_mut_ptr().add(len), count) };

tests/ui/lint/implicit_autorefs.fixed

@@ -0,0 +1,66 @@
+//@ check-pass
+//@ run-rustfix
+
+#![allow(dead_code)] // for the rustfix-ed code
+
+use std::mem::ManuallyDrop;
+use std::ptr::addr_of_mut;
+use std::ptr::addr_of;
+use std::ops::Deref;
+
+unsafe fn test_const(ptr: *const [u8]) -> *const [u8] {
+    addr_of!((&(*ptr))[..16])
+    //~^ WARN implicit auto-ref
+}
+
+struct Test {
+    field: [u8],
+}
+
+unsafe fn test_field(ptr: *const Test) -> *const [u8] {
+    let l = (&(*ptr).field).len();
+    //~^ WARN implicit auto-ref
+
+    addr_of!((&(*ptr).field)[..l - 1])
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_builtin_index(a: *mut [String]) {
+    _ = (&(*a)[0]).len();
+    //~^ WARN implicit auto-ref
+
+    _ = (&(&(*a))[..1][0]).len();
+    //~^ WARN implicit auto-ref
+    //~^^ WARN implicit auto-ref
+}
+
+unsafe fn test_overloaded_deref_const(ptr: *const ManuallyDrop<Test>) {
+    _ = addr_of!((&(*ptr)).field);
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_overloaded_deref_mut(ptr: *mut ManuallyDrop<Test>) {
+    _ = addr_of_mut!((&mut (*ptr)).field);
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_manually_overloaded_deref() {
+    struct W<T>(T);
+
+    impl<T> Deref for W<T> {
+        type Target = T;
+        fn deref(&self) -> &T { &self.0 }
+    }
+
+    let w: W<i32> = W(5);
+    let w = addr_of!(w);
+    let _p: *const i32 = addr_of!(*(&**w));
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_no_attr(ptr: *mut ManuallyDrop<u8>) {
+    ptr.write(ManuallyDrop::new(1)); // should not warn, as `ManuallyDrop::write` is not
+                                     // annotated with `#[rustc_no_implicit_auto_ref]`
+}
+
+fn main() {}

tests/ui/lint/implicit_autorefs.rs

@@ -0,0 +1,66 @@
+//@ check-pass
+//@ run-rustfix
+
+#![allow(dead_code)] // for the rustfix-ed code
+
+use std::mem::ManuallyDrop;
+use std::ptr::addr_of_mut;
+use std::ptr::addr_of;
+use std::ops::Deref;
+
+unsafe fn test_const(ptr: *const [u8]) -> *const [u8] {
+    addr_of!((*ptr)[..16])
+    //~^ WARN implicit auto-ref
+}
+
+struct Test {
+    field: [u8],
+}
+
+unsafe fn test_field(ptr: *const Test) -> *const [u8] {
+    let l = (*ptr).field.len();
+    //~^ WARN implicit auto-ref
+
+    addr_of!((*ptr).field[..l - 1])
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_builtin_index(a: *mut [String]) {
+    _ = (*a)[0].len();
+    //~^ WARN implicit auto-ref
+
+    _ = (*a)[..1][0].len();
+    //~^ WARN implicit auto-ref
+    //~^^ WARN implicit auto-ref
+}
+
+unsafe fn test_overloaded_deref_const(ptr: *const ManuallyDrop<Test>) {
+    _ = addr_of!((*ptr).field);
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_overloaded_deref_mut(ptr: *mut ManuallyDrop<Test>) {
+    _ = addr_of_mut!((*ptr).field);
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_manually_overloaded_deref() {
+    struct W<T>(T);
+
+    impl<T> Deref for W<T> {
+        type Target = T;
+        fn deref(&self) -> &T { &self.0 }
+    }
+
+    let w: W<i32> = W(5);
+    let w = addr_of!(w);
+    let _p: *const i32 = addr_of!(**w);
+    //~^ WARN implicit auto-ref
+}
+
+unsafe fn test_no_attr(ptr: *mut ManuallyDrop<u8>) {
+    ptr.write(ManuallyDrop::new(1)); // should not warn, as `ManuallyDrop::write` is not
+                                     // annotated with `#[rustc_no_implicit_auto_ref]`
+}
+
+fn main() {}