diff --git a/CHANGELOG.md b/CHANGELOG.md index 903ee938d9d..3c2801cfb5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4590,6 +4590,7 @@ Released 2018-09-13 [`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool [`transmute_int_to_char`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_char [`transmute_int_to_float`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_float +[`transmute_null_to_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_null_to_fn [`transmute_num_to_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_num_to_bytes [`transmute_ptr_to_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr [`transmute_ptr_to_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 3cd7d1d7e72..5bae62ce24f 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -568,6 +568,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::transmute::TRANSMUTE_INT_TO_BOOL_INFO, crate::transmute::TRANSMUTE_INT_TO_CHAR_INFO, crate::transmute::TRANSMUTE_INT_TO_FLOAT_INFO, + crate::transmute::TRANSMUTE_NULL_TO_FN_INFO, crate::transmute::TRANSMUTE_NUM_TO_BYTES_INFO, crate::transmute::TRANSMUTE_PTR_TO_PTR_INFO, crate::transmute::TRANSMUTE_PTR_TO_REF_INFO, diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index 83e651aba8e..7a2ab2bb4c4 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -3,6 +3,7 @@ mod transmute_float_to_int; mod transmute_int_to_bool; mod transmute_int_to_char; mod transmute_int_to_float; +mod transmute_null_to_fn; mod transmute_num_to_bytes; mod transmute_ptr_to_ptr; mod transmute_ptr_to_ref; @@ -409,6 +410,34 @@ declare_clippy_lint! { "transmutes from a null pointer to a reference, which is undefined behavior" } +declare_clippy_lint! { + /// ### What it does + /// Checks for null function pointer creation through transmute. + /// + /// ### Why is this bad? + /// Creating a null function pointer is undefined behavior. + /// + /// More info: https://doc.rust-lang.org/nomicon/ffi.html#the-nullable-pointer-optimization + /// + /// ### Known problems + /// Not all cases can be detected at the moment of this writing. + /// For example, variables which hold a null pointer and are then fed to a `transmute` + /// call, aren't detectable yet. + /// + /// ### Example + /// ```rust + /// let null_fn: fn() = unsafe { std::mem::transmute( std::ptr::null() ) }; + /// ``` + /// Use instead: + /// ```rust + /// let null_fn: Option = None; + /// ``` + #[clippy::version = "1.67.0"] + pub TRANSMUTE_NULL_TO_FN, + correctness, + "transmute results in a null function pointer, which is undefined behavior" +} + pub struct Transmute { msrv: Msrv, } @@ -428,6 +457,7 @@ impl_lint_pass!(Transmute => [ TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, TRANSMUTE_UNDEFINED_REPR, TRANSMUTING_NULL, + TRANSMUTE_NULL_TO_FN, ]); impl Transmute { #[must_use] @@ -461,6 +491,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { let linted = wrong_transmute::check(cx, e, from_ty, to_ty) | crosspointer_transmute::check(cx, e, from_ty, to_ty) | transmuting_null::check(cx, e, arg, to_ty) + | transmute_null_to_fn::check(cx, e, arg, to_ty) | transmute_ptr_to_ref::check(cx, e, from_ty, to_ty, arg, path, &self.msrv) | transmute_int_to_char::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_ref_to_ref::check(cx, e, from_ty, to_ty, arg, const_context) diff --git a/clippy_lints/src/transmute/transmute_null_to_fn.rs b/clippy_lints/src/transmute/transmute_null_to_fn.rs new file mode 100644 index 00000000000..db89078f657 --- /dev/null +++ b/clippy_lints/src/transmute/transmute_null_to_fn.rs @@ -0,0 +1,62 @@ +use clippy_utils::consts::{constant_context, Constant}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::{is_integer_literal, is_path_diagnostic_item}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; +use rustc_span::symbol::sym; + +use super::TRANSMUTE_NULL_TO_FN; + +const LINT_MSG: &str = "transmuting a known null pointer into a function pointer"; +const NOTE_MSG: &str = "this transmute results in a null function pointer"; +const HELP_MSG: &str = + "try wrapping your function pointer type in `Option` instead, and using `None` as a null pointer value"; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'tcx Expr<'_>, to_ty: Ty<'tcx>) -> bool { + if !to_ty.is_fn() { + return false; + } + + // Catching transmute over constants that resolve to `null`. + let mut const_eval_context = constant_context(cx, cx.typeck_results()); + if let ExprKind::Path(ref _qpath) = arg.kind && + let Some(Constant::RawPtr(x)) = const_eval_context.expr(arg) && + x == 0 + { + span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { + diag.span_label(expr.span, NOTE_MSG); + diag.help(HELP_MSG); + }); + return true; + } + + // Catching: + // `std::mem::transmute(0 as *const i32)` + if let ExprKind::Cast(inner_expr, _cast_ty) = arg.kind && is_integer_literal(inner_expr, 0) { + span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { + diag.span_label(expr.span, NOTE_MSG); + diag.help(HELP_MSG); + }); + return true; + } + + // Catching: + // `std::mem::transmute(std::ptr::null::())` + if let ExprKind::Call(func1, []) = arg.kind && + is_path_diagnostic_item(cx, func1, sym::ptr_null) + { + span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { + diag.span_label(expr.span, NOTE_MSG); + diag.help(HELP_MSG); + }); + return true; + } + + // FIXME: + // Also catch transmutations of variables which are known nulls. + // To do this, MIR const propagation seems to be the better tool. + // Whenever MIR const prop routines are more developed, this will + // become available. As of this writing (25/03/19) it is not yet. + false +} diff --git a/tests/ui/transmute_null_to_fn.rs b/tests/ui/transmute_null_to_fn.rs new file mode 100644 index 00000000000..b3ea3d9039e --- /dev/null +++ b/tests/ui/transmute_null_to_fn.rs @@ -0,0 +1,28 @@ +#![allow(dead_code)] +#![warn(clippy::transmute_null_to_fn)] +#![allow(clippy::zero_ptr)] + +// Easy to lint because these only span one line. +fn one_liners() { + unsafe { + let _: fn() = std::mem::transmute(0 as *const ()); + let _: fn() = std::mem::transmute(std::ptr::null::<()>()); + } +} + +pub const ZPTR: *const usize = 0 as *const _; +pub const NOT_ZPTR: *const usize = 1 as *const _; + +fn transmute_const() { + unsafe { + // Should raise a lint. + let _: fn() = std::mem::transmute(ZPTR); + // Should NOT raise a lint. + let _: fn() = std::mem::transmute(NOT_ZPTR); + } +} + +fn main() { + one_liners(); + transmute_const(); +} diff --git a/tests/ui/transmute_null_to_fn.stderr b/tests/ui/transmute_null_to_fn.stderr new file mode 100644 index 00000000000..f0c65497d75 --- /dev/null +++ b/tests/ui/transmute_null_to_fn.stderr @@ -0,0 +1,27 @@ +error: transmuting a known null pointer into a function pointer + --> $DIR/transmute_null_to_fn.rs:8:23 + | +LL | let _: fn() = std::mem::transmute(0 as *const ()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior + | + = help: try wrapping your function pointer type in `Option` instead, and using `None` as a null pointer value + = note: `-D clippy::transmute-null-to-fn` implied by `-D warnings` + +error: transmuting a known null pointer into a function pointer + --> $DIR/transmute_null_to_fn.rs:9:23 + | +LL | let _: fn() = std::mem::transmute(std::ptr::null::<()>()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior + | + = help: try wrapping your function pointer type in `Option` instead, and using `None` as a null pointer value + +error: transmuting a known null pointer into a function pointer + --> $DIR/transmute_null_to_fn.rs:19:23 + | +LL | let _: fn() = std::mem::transmute(ZPTR); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior + | + = help: try wrapping your function pointer type in `Option` instead, and using `None` as a null pointer value + +error: aborting due to 3 previous errors +