Add proper support for all kinds of unsafe ops to the lint

WaffleLapkin · WaffleLapkin · commit 0df43db50c8a · 2024-05-02T03:49:49.000+02:00
(never_type_fallback_flowing_into_unsafe)
diff --git a/compiler/rustc_hir_typeck/messages.ftl b/compiler/rustc_hir_typeck/messages.ftl
@@ -100,7 +100,14 @@ hir_typeck_lossy_provenance_ptr2int =
 hir_typeck_missing_parentheses_in_range = can't call method `{$method_name}` on type `{$ty_str}`
 
 hir_typeck_never_type_fallback_flowing_into_unsafe =
-    never type fallback affects this call to an `unsafe` function
+    never type fallback affects this {$reason ->
+    [call] call to an `unsafe` function
+    [union_field] union access
+    [deref] raw pointer dereference
+    [path] `unsafe` function
+    [method] call to an `unsafe` method
+    *[other] THIS SHOULD NOT BE REACHABLE
+    }
     .help = specify the type explicitly
 
 hir_typeck_no_associated_item = no {$item_kind} named `{$item_name}` found for {$ty_prefix} `{$ty_str}`{$trait_missing_method ->
diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs
@@ -1,7 +1,7 @@
 //! Errors emitted by `rustc_hir_typeck`.
 use std::borrow::Cow;
 
-use crate::fluent_generated as fluent;
+use crate::{fallback::UnsafeUseReason, fluent_generated as fluent};
 use rustc_errors::{
     codes::*, Applicability, Diag, DiagArgValue, EmissionGuarantee, IntoDiagArg, MultiSpan,
     SubdiagMessageOp, Subdiagnostic,
@@ -167,7 +167,9 @@ pub struct MissingParenthesesInRange {
 #[derive(LintDiagnostic)]
 #[diag(hir_typeck_never_type_fallback_flowing_into_unsafe)]
 #[help]
-pub struct NeverTypeFallbackFlowingIntoUnsafe {}
+pub struct NeverTypeFallbackFlowingIntoUnsafe {
+    pub reason: UnsafeUseReason,
+}
 
 #[derive(Subdiagnostic)]
 #[multipart_suggestion(
diff --git a/compiler/rustc_hir_typeck/src/fallback.rs b/compiler/rustc_hir_typeck/src/fallback.rs
@@ -1,10 +1,11 @@
-use std::cell::OnceCell;
+use std::{borrow::Cow, cell::OnceCell};
 
 use crate::{errors, FnCtxt, TypeckRootCtxt};
 use rustc_data_structures::{
     graph::{self, iterate::DepthFirstSearch, vec_graph::VecGraph},
     unord::{UnordBag, UnordMap, UnordSet},
 };
+use rustc_errors::{DiagArgValue, IntoDiagArg};
 use rustc_hir as hir;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::HirId;
@@ -374,12 +375,12 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
                         .filter_map(|x| unsafe_infer_vars.get(&x).copied())
                         .collect::<Vec<_>>();
 
-                for (hir_id, span) in affected_unsafe_infer_vars {
+                for (hir_id, span, reason) in affected_unsafe_infer_vars {
                     self.tcx.emit_node_span_lint(
                         lint::builtin::NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE,
                         hir_id,
                         span,
-                        errors::NeverTypeFallbackFlowingIntoUnsafe {},
+                        errors::NeverTypeFallbackFlowingIntoUnsafe { reason },
                     );
                 }
 
@@ -493,77 +494,169 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
     }
 }
 
-/// Finds all type variables which are passed to an `unsafe` function.
+#[derive(Debug, Copy, Clone)]
+pub(crate) enum UnsafeUseReason {
+    Call,
+    Method,
+    Path,
+    UnionField,
+    Deref,
+}
+
+impl IntoDiagArg for UnsafeUseReason {
+    fn into_diag_arg(self) -> DiagArgValue {
+        let s = match self {
+            UnsafeUseReason::Call => "call",
+            UnsafeUseReason::Method => "method",
+            UnsafeUseReason::Path => "path",
+            UnsafeUseReason::UnionField => "union_field",
+            UnsafeUseReason::Deref => "deref",
+        };
+        DiagArgValue::Str(Cow::Borrowed(s))
+    }
+}
+
+/// Finds all type variables which are passed to an `unsafe` operation.
 ///
 /// For example, for this function `f`:
 /// ```ignore (demonstrative)
 /// fn f() {
 ///     unsafe {
 ///         let x /* ?X */ = core::mem::zeroed();
-///         //               ^^^^^^^^^^^^^^^^^^^ -- hir_id, span
+///         //               ^^^^^^^^^^^^^^^^^^^ -- hir_id, span, reason
 ///
 ///         let y = core::mem::zeroed::<Option<_ /* ?Y */>>();
-///         //               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- hir_id, span
+///         //      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- hir_id, span, reason
 ///     }
 /// }
 /// ```
 ///
-/// Will return `{ id(?X) -> (hir_id, span) }`
+/// `compute_unsafe_infer_vars` will return `{ id(?X) -> (hir_id, span, Call) }`
 fn compute_unsafe_infer_vars<'a, 'tcx>(
     root_ctxt: &'a TypeckRootCtxt<'tcx>,
     body_id: LocalDefId,
-) -> UnordMap<ty::TyVid, (HirId, Span)> {
-    let tcx = root_ctxt.infcx.tcx;
-    let body_id = tcx.hir().maybe_body_owned_by(body_id).expect("body id must have an owner");
-    let body = tcx.hir().body(body_id);
+) -> UnordMap<ty::TyVid, (HirId, Span, UnsafeUseReason)> {
+    let body_id =
+        root_ctxt.tcx.hir().maybe_body_owned_by(body_id).expect("body id must have an owner");
+    let body = root_ctxt.tcx.hir().body(body_id);
     let mut res = UnordMap::default();
 
     struct UnsafeInferVarsVisitor<'a, 'tcx, 'r> {
         root_ctxt: &'a TypeckRootCtxt<'tcx>,
-        res: &'r mut UnordMap<ty::TyVid, (HirId, Span)>,
+        res: &'r mut UnordMap<ty::TyVid, (HirId, Span, UnsafeUseReason)>,
     }
 
     impl Visitor<'_> for UnsafeInferVarsVisitor<'_, '_, '_> {
         fn visit_expr(&mut self, ex: &'_ hir::Expr<'_>) {
-            // FIXME: method calls
-            if let hir::ExprKind::Call(func, ..) = ex.kind {
-                let typeck_results = self.root_ctxt.typeck_results.borrow();
-
-                let func_ty = typeck_results.expr_ty(func);
-
-                // `is_fn` is required to ignore closures (which can't be unsafe)
-                if func_ty.is_fn()
-                    && let sig = func_ty.fn_sig(self.root_ctxt.infcx.tcx)
-                    && let hir::Unsafety::Unsafe = sig.unsafety()
-                {
-                    let mut collector =
-                        InferVarCollector { hir_id: ex.hir_id, call_span: ex.span, res: self.res };
-
-                    // Collect generic arguments of the function which are inference variables
-                    typeck_results
-                        .node_args(ex.hir_id)
-                        .types()
-                        .for_each(|t| t.visit_with(&mut collector));
-
-                    // Also check the return type, for cases like `(unsafe_fn::<_> as unsafe fn() -> _)()`
-                    sig.output().visit_with(&mut collector);
+            let typeck_results = self.root_ctxt.typeck_results.borrow();
+
+            match ex.kind {
+                hir::ExprKind::MethodCall(..) => {
+                    if let Some(def_id) = typeck_results.type_dependent_def_id(ex.hir_id)
+                        && let method_ty = self.root_ctxt.tcx.type_of(def_id).instantiate_identity()
+                        && let sig = method_ty.fn_sig(self.root_ctxt.tcx)
+                        && let hir::Unsafety::Unsafe = sig.unsafety()
+                    {
+                        let mut collector = InferVarCollector {
+                            value: (ex.hir_id, ex.span, UnsafeUseReason::Method),
+                            res: self.res,
+                        };
+
+                        // Collect generic arguments (incl. `Self`) of the method
+                        typeck_results
+                            .node_args(ex.hir_id)
+                            .types()
+                            .for_each(|t| t.visit_with(&mut collector));
+                    }
                 }
-            }
+
+                hir::ExprKind::Call(func, ..) => {
+                    let func_ty = typeck_results.expr_ty(func);
+
+                    if func_ty.is_fn()
+                        && let sig = func_ty.fn_sig(self.root_ctxt.tcx)
+                        && let hir::Unsafety::Unsafe = sig.unsafety()
+                    {
+                        let mut collector = InferVarCollector {
+                            value: (ex.hir_id, ex.span, UnsafeUseReason::Call),
+                            res: self.res,
+                        };
+
+                        // Try collecting generic arguments of the function.
+                        // Note that we do this below for any paths (that don't have to be called),
+                        // but there we do it with a different span/reason.
+                        // This takes priority.
+                        typeck_results
+                            .node_args(func.hir_id)
+                            .types()
+                            .for_each(|t| t.visit_with(&mut collector));
+
+                        // Also check the return type, for cases like `returns_unsafe_fn_ptr()()`
+                        sig.output().visit_with(&mut collector);
+                    }
+                }
+
+                // Check paths which refer to functions.
+                // We do this, instead of only checking `Call` to make sure the lint can't be
+                // avoided by storing unsafe function in a variable.
+                hir::ExprKind::Path(_) => {
+                    let ty = typeck_results.expr_ty(ex);
+
+                    // If this path refers to an unsafe function, collect inference variables which may affect it.
+                    // `is_fn` excludes closures, but those can't be unsafe.
+                    if ty.is_fn()
+                        && let sig = ty.fn_sig(self.root_ctxt.tcx)
+                        && let hir::Unsafety::Unsafe = sig.unsafety()
+                    {
+                        let mut collector = InferVarCollector {
+                            value: (ex.hir_id, ex.span, UnsafeUseReason::Path),
+                            res: self.res,
+                        };
+
+                        // Collect generic arguments of the function
+                        typeck_results
+                            .node_args(ex.hir_id)
+                            .types()
+                            .for_each(|t| t.visit_with(&mut collector));
+                    }
+                }
+
+                hir::ExprKind::Unary(hir::UnOp::Deref, pointer) => {
+                    if let ty::RawPtr(pointee, _) = typeck_results.expr_ty(pointer).kind() {
+                        pointee.visit_with(&mut InferVarCollector {
+                            value: (ex.hir_id, ex.span, UnsafeUseReason::Deref),
+                            res: self.res,
+                        });
+                    }
+                }
+
+                hir::ExprKind::Field(base, _) => {
+                    let base_ty = typeck_results.expr_ty(base);
+
+                    if base_ty.is_union() {
+                        typeck_results.expr_ty(ex).visit_with(&mut InferVarCollector {
+                            value: (ex.hir_id, ex.span, UnsafeUseReason::UnionField),
+                            res: self.res,
+                        });
+                    }
+                }
+
+                _ => (),
+            };
 
             hir::intravisit::walk_expr(self, ex);
         }
     }
 
-    struct InferVarCollector<'r> {
-        hir_id: HirId,
-        call_span: Span,
-        res: &'r mut UnordMap<ty::TyVid, (HirId, Span)>,
+    struct InferVarCollector<'r, V> {
+        value: V,
+        res: &'r mut UnordMap<ty::TyVid, V>,
     }
 
-    impl<'tcx> ty::TypeVisitor<TyCtxt<'tcx>> for InferVarCollector<'_> {
+    impl<'tcx, V: Copy> ty::TypeVisitor<TyCtxt<'tcx>> for InferVarCollector<'_, V> {
         fn visit_ty(&mut self, t: Ty<'tcx>) {
             if let Some(vid) = t.ty_vid() {
-                self.res.insert(vid, (self.hir_id, self.call_span));
+                _ = self.res.try_insert(vid, self.value);
             } else {
                 t.super_visit_with(self)
             }
diff --git a/tests/ui/never_type/lint-never-type-fallback-flowing-into-unsafe.rs b/tests/ui/never_type/lint-never-type-fallback-flowing-into-unsafe.rs
@@ -1,7 +1,9 @@
 //@ check-pass
-use std::mem;
+use std::{marker, mem, ptr};
 
-fn main() {
+fn main() {}
+
+fn _zero() {
     if false {
         unsafe { mem::zeroed() }
         //~^ warn: never type fallback affects this call to an `unsafe` function
@@ -13,6 +15,110 @@ fn main() {
     if true { unsafe { mem::zeroed() } } else { return }
 }
 
+fn _trans() {
+    if false {
+        unsafe {
+            struct Zst;
+            core::mem::transmute(Zst)
+            //~^ warn: never type fallback affects this call to an `unsafe` function
+        }
+    } else {
+        return;
+    };
+}
+
+fn _union() {
+    if false {
+        union Union<T: Copy> {
+            a: (),
+            b: T,
+        }
+
+        unsafe { Union { a: () }.b }
+        //~^ warn: never type fallback affects this union access
+    } else {
+        return;
+    };
+}
+
+fn _deref() {
+    if false {
+        unsafe { *ptr::from_ref(&()).cast() }
+        //~^ warn: never type fallback affects this raw pointer dereference
+    } else {
+        return;
+    };
+}
+
+fn _only_generics() {
+    if false {
+        unsafe fn internally_create<T>(_: Option<T>) {
+            let _ = mem::zeroed::<T>();
+        }
+
+        // We need the option (and unwrap later) to call a function in a way,
+        // which makes it affected by the fallback, but without having it return anything
+        let x = None;
+
+        unsafe { internally_create(x) }
+        //~^ warn: never type fallback affects this call to an `unsafe` function
+
+        x.unwrap()
+    } else {
+        return;
+    };
+}
+
+fn _stored_function() {
+    if false {
+        let zeroed = mem::zeroed;
+        //~^ warn: never type fallback affects this `unsafe` function
+
+        unsafe { zeroed() }
+        //~^ warn: never type fallback affects this call to an `unsafe` function
+    } else {
+        return;
+    };
+}
+
+fn _only_generics_stored_function() {
+    if false {
+        unsafe fn internally_create<T>(_: Option<T>) {
+            let _ = mem::zeroed::<T>();
+        }
+
+        let x = None;
+        let f = internally_create;
+        //~^ warn: never type fallback affects this `unsafe` function
+
+        unsafe { f(x) }
+
+        x.unwrap()
+    } else {
+        return;
+    };
+}
+
+fn _method() {
+    struct S<T>(marker::PhantomData<T>);
+
+    impl<T> S<T> {
+        #[allow(unused)] // FIXME: the unused lint is probably incorrect here
+        unsafe fn create_out_of_thin_air(&self) -> T {
+            todo!()
+        }
+    }
+
+    if false {
+        unsafe {
+            S(marker::PhantomData).create_out_of_thin_air()
+            //~^ warn: never type fallback affects this call to an `unsafe` method
+        }
+    } else {
+        return;
+    };
+}
+
 // Minimization of the famous `objc` crate issue
 fn _objc() {
     pub unsafe fn send_message<R>() -> Result<R, ()> {
diff --git a/tests/ui/never_type/lint-never-type-fallback-flowing-into-unsafe.stderr b/tests/ui/never_type/lint-never-type-fallback-flowing-into-unsafe.stderr
