feat(trusted_asref): new lint

CHANGELOG.md

@@ -7406,6 +7406,7 @@ Released 2018-09-13
 [`trim_split_whitespace`]: https://rust-lang.github.io/rust-clippy/master/index.html#trim_split_whitespace
 [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
 [`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
+[`trusted_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trusted_asref
 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err
 [`tuple_array_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions
 [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

clippy_lints/src/declared_lints.rs

@@ -752,6 +752,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::transmute::UNSOUND_COLLECTION_TRANSMUTE_INFO,
     crate::transmute::USELESS_TRANSMUTE_INFO,
     crate::transmute::WRONG_TRANSMUTE_INFO,
+    crate::trusted_asref::TRUSTED_ASREF_INFO,
     crate::tuple_array_conversions::TUPLE_ARRAY_CONVERSIONS_INFO,
     crate::types::BORROWED_BOX_INFO,
     crate::types::BOX_COLLECTION_INFO,

clippy_lints/src/lib.rs

@@ -363,6 +363,7 @@ mod toplevel_ref_arg;
 mod trailing_empty_array;
 mod trait_bounds;
 mod transmute;
+mod trusted_asref;
 mod tuple_array_conversions;
 mod types;
 mod unconditional_recursion;
@@ -862,6 +863,7 @@ rustc_lint::late_lint_methods!(
         ManualAssertEq: manual_assert_eq::ManualAssertEq = manual_assert_eq::ManualAssertEq,
         WithCapacityZero: with_capacity_zero::WithCapacityZero = with_capacity_zero::WithCapacityZero,
         RefPatterns: ref_patterns::RefPatterns = ref_patterns::RefPatterns,
+        TrustedAsref: trusted_asref::TrustedAsref = trusted_asref::TrustedAsref,
         // add late passes here, used by `cargo dev new_lint`
     ]]
 );

clippy_lints/src/trusted_asref.rs

@@ -0,0 +1,140 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::res::MaybeDef;
+use clippy_utils::{SpanlessEq, hash_expr};
+use rustc_data_structures::fx::FxIndexMap;
+use rustc_hir as hir;
+use rustc_hir::intravisit::{Visitor, walk_expr};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::declare_lint_pass;
+use rustc_span::{Span, sym};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for unsafe code calling `.as_ref()` multiple times
+    /// on the same generic receiver.
+    ///
+    /// ### Why is this bad?
+    /// Usually `AsRef::as_ref()` is pure and multiple calls will return a
+    /// reference to the exact same value. However, unsafe code cannot always
+    /// soundly rely on this.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # unsafe fn ffi_call(_ptr: *const u8, _len: usize) {}
+    /// fn safe_ffi_call(data: impl AsRef<[u8]>) {
+    ///     unsafe {
+    ///         ffi_call(data.as_ref().as_ptr(), data.as_ref().len());
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # unsafe fn ffi_call(_ptr: *const u8, _len: usize) {}
+    /// fn safe_ffi_call(data: impl AsRef<[u8]>) {
+    ///     let data = data.as_ref();
+    ///     unsafe {
+    ///         ffi_call(data.as_ptr(), data.len());
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ### Known problems
+    /// False negatives: Such code can also be unsound when the receiver
+    /// is a concrete type from a foreign crate.
+    ///
+    /// False positives: Not all generic implementations are provided by
+    /// foreign code. Not all unsafe code actually relies on the purity of
+    /// `as_ref()`.
+    #[clippy::version = "1.98.0"]
+    pub TRUSTED_ASREF,
+    suspicious,
+    "unsafe block calls `.as_ref()` multiple times on the same generic receiver"
+}
+
+declare_lint_pass!(TrustedAsref => [TRUSTED_ASREF]);
+
+impl<'tcx> LateLintPass<'tcx> for TrustedAsref {
+    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'tcx>) {
+        if block.rules == hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::UserProvided)
+            && !block.span.in_external_macro(cx.tcx.sess.source_map())
+        {
+            let mut visitor = ForeignAsRefCallVisitor {
+                cx,
+                receiver_buckets: FxIndexMap::default(),
+            };
+            visitor.visit_block(block);
+
+            for bucket in visitor.receiver_buckets.into_values() {
+                for calls in bucket {
+                    check_calls(cx, calls);
+                }
+            }
+        }
+    }
+}
+
+fn check_calls(cx: &LateContext<'_>, calls: Calls<'_>) {
+    if calls.spans.len() < 2 {
+        return;
+    }
+
+    span_lint_and_help(
+        cx,
+        TRUSTED_ASREF,
+        calls.spans,
+        "unsafe block calls `as_ref` multiple times on the same receiver",
+        None,
+        "consider calling `as_ref()` once and reusing the result to avoid potential unsoundness",
+    );
+}
+
+struct Calls<'tcx> {
+    receiver: &'tcx hir::Expr<'tcx>,
+    spans: Vec<Span>,
+}
+
+struct ForeignAsRefCallVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    receiver_buckets: FxIndexMap<u64, Vec<Calls<'tcx>>>,
+}
+
+impl<'tcx> Visitor<'tcx> for ForeignAsRefCallVisitor<'_, 'tcx> {
+    fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
+        if matches!(
+            expr.kind,
+            hir::ExprKind::Block(
+                hir::Block {
+                    rules: hir::BlockCheckMode::UnsafeBlock(_),
+                    ..
+                },
+                _
+            )
+        ) {
+            return; // Do not descend into nested unsafe block
+        }
+
+        if let hir::ExprKind::MethodCall(_, receiver, _, _) = expr.kind
+            && let Some(def_id) = self.cx.typeck_results().type_dependent_def_id(expr.hir_id)
+            && def_id.opt_parent(self.cx).is_diag_item(self.cx, sym::AsRef)
+            && let ty::Param(_) = self.cx.typeck_results().expr_ty(receiver).peel_refs().kind()
+        {
+            let receiver_bucket = self.receiver_buckets.entry(hash_expr(self.cx, receiver)).or_default();
+
+            let calls = match receiver_bucket
+                .iter_mut()
+                .find(|calls| SpanlessEq::new(self.cx).eq_expr(receiver.span.ctxt(), calls.receiver, receiver))
+            {
+                Some(calls) => calls,
+                None => receiver_bucket.push_mut(Calls {
+                    receiver,
+                    spans: Vec::new(),
+                }),
+            };
+
+            calls.spans.push(expr.span);
+        }
+
+        walk_expr(self, expr);
+    }
+}

tests/ui/trusted_asref.rs

@@ -0,0 +1,113 @@
+#![warn(clippy::trusted_asref)]
+
+unsafe fn use_slice<T>(_s: *const T, _len: usize) {}
+
+unsafe fn use_ptr<T>(_s: *const T) {}
+
+fn bad_generic<T: AsRef<str>>(s: T) {
+    unsafe {
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+        //~^ trusted_asref
+    }
+}
+
+fn bad_impl(s: impl AsRef<[u8]>) {
+    unsafe {
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+        //~^ trusted_asref
+    }
+}
+
+fn bad_where<T>(s: T)
+where
+    T: AsRef<[u8]>,
+{
+    unsafe {
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+        //~^ trusted_asref
+    }
+}
+
+fn bad_nested_unsafe<T: AsRef<str>>(s: T) {
+    unsafe {
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+        //~^ trusted_asref
+        unsafe {
+            use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+            //~^ trusted_asref
+        }
+    }
+}
+
+fn bad_multiple_expr<T: AsRef<[u8]>>(s: T) {
+    unsafe {
+        let p = s.as_ref().as_ptr();
+        //~^ trusted_asref
+        let len = s.as_ref().len();
+        use_slice(p, len);
+    }
+}
+
+fn bad_multiple_calls<T: AsRef<[u8]>>(s: T) {
+    unsafe {
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+        //~^ trusted_asref
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+    }
+}
+
+fn good_generic<T: AsRef<str>>(s: T) {
+    let s = s.as_ref();
+    unsafe {
+        use_slice(s.as_ptr(), s.len());
+    }
+}
+
+fn good_impl(s: impl AsRef<[u8]>) {
+    let s = s.as_ref();
+    unsafe {
+        use_slice(s.as_ptr(), s.len());
+    }
+}
+
+fn good_where<T>(s: T)
+where
+    T: AsRef<[u8]>,
+{
+    let s = s.as_ref();
+    unsafe {
+        use_slice(s.as_ptr(), s.len());
+    }
+}
+
+fn good_trustworthy_std(s: std::rc::Rc<str>) {
+    unsafe {
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+    }
+}
+
+struct TrustworthyStr(String);
+
+impl AsRef<str> for TrustworthyStr {
+    fn as_ref(&self) -> &str {
+        &self.0
+    }
+}
+
+fn good_trustworthy_crate_local(s: TrustworthyStr) {
+    unsafe {
+        use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+    }
+}
+
+fn good_unrelated_calls(a: impl AsRef<u32>, b: impl AsRef<u32>) {
+    unsafe {
+        use_ptr(a.as_ref() as *const u32);
+        use_ptr(b.as_ref() as *const u32);
+    }
+}
+
+fn good_no_unsafe(s: impl AsRef<str>) {
+    let _ptr = s.as_ref().as_ptr();
+    let _len = s.as_ref().len();
+}

tests/ui/trusted_asref.stderr

@@ -0,0 +1,66 @@
+error: unsafe block calls `as_ref` multiple times on the same receiver
+  --> tests/ui/trusted_asref.rs:9:19
+   |
+LL |         use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+   |                   ^^^^^^^^^^           ^^^^^^^^^^
+   |
+   = help: consider calling `as_ref()` once and reusing the result to avoid potential unsoundness
+   = note: `-D clippy::trusted-asref` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::trusted_asref)]`
+
+error: unsafe block calls `as_ref` multiple times on the same receiver
+  --> tests/ui/trusted_asref.rs:16:19
+   |
+LL |         use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+   |                   ^^^^^^^^^^           ^^^^^^^^^^
+   |
+   = help: consider calling `as_ref()` once and reusing the result to avoid potential unsoundness
+
+error: unsafe block calls `as_ref` multiple times on the same receiver
+  --> tests/ui/trusted_asref.rs:26:19
+   |
+LL |         use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+   |                   ^^^^^^^^^^           ^^^^^^^^^^
+   |
+   = help: consider calling `as_ref()` once and reusing the result to avoid potential unsoundness
+
+error: unsafe block calls `as_ref` multiple times on the same receiver
+  --> tests/ui/trusted_asref.rs:33:19
+   |
+LL |         use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+   |                   ^^^^^^^^^^           ^^^^^^^^^^
+   |
+   = help: consider calling `as_ref()` once and reusing the result to avoid potential unsoundness
+
+error: unsafe block calls `as_ref` multiple times on the same receiver
+  --> tests/ui/trusted_asref.rs:36:23
+   |
+LL |             use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+   |                       ^^^^^^^^^^           ^^^^^^^^^^
+   |
+   = help: consider calling `as_ref()` once and reusing the result to avoid potential unsoundness
+
+error: unsafe block calls `as_ref` multiple times on the same receiver
+  --> tests/ui/trusted_asref.rs:44:17
+   |
+LL |         let p = s.as_ref().as_ptr();
+   |                 ^^^^^^^^^^
+LL |
+LL |         let len = s.as_ref().len();
+   |                   ^^^^^^^^^^
+   |
+   = help: consider calling `as_ref()` once and reusing the result to avoid potential unsoundness
+
+error: unsafe block calls `as_ref` multiple times on the same receiver
+  --> tests/ui/trusted_asref.rs:53:19
+   |
+LL |         use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+   |                   ^^^^^^^^^^           ^^^^^^^^^^
+LL |
+LL |         use_slice(s.as_ref().as_ptr(), s.as_ref().len());
+   |                   ^^^^^^^^^^           ^^^^^^^^^^
+   |
+   = help: consider calling `as_ref()` once and reusing the result to avoid potential unsoundness
+
+error: aborting due to 7 previous errors
+