From 8e0126b8e9e958d25e452f708357ac63753bac51 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 9 Nov 2023 17:37:27 +0100 Subject: [PATCH] Add warn-by-default lint against pointer trait comparisons --- compiler/rustc_lint/messages.ftl | 4 + compiler/rustc_lint/src/lints.rs | 39 ++++++++ compiler/rustc_lint/src/types.rs | 95 +++++++++++++++++-- tests/ui/lint/pointer_trait_comparisons.rs | 21 ++++ .../ui/lint/pointer_trait_comparisons.stderr | 33 +++++++ 5 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 tests/ui/lint/pointer_trait_comparisons.rs create mode 100644 tests/ui/lint/pointer_trait_comparisons.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 068f1372c0ecd..51be75e89fe4e 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -315,6 +315,10 @@ lint_invalid_nan_comparisons_eq_ne = incorrect NaN comparison, NaN cannot be dir lint_invalid_nan_comparisons_lt_le_gt_ge = incorrect NaN comparison, NaN is not orderable +lint_invalid_pointer_trait_comparisons = incorrect pointer trait comparison, the comparison include metadata which is not guaranteed to be equivalent + .addr_metadata_suggestion = use explicit `std::ptr::eq` method to compare metadata and addresses + .addr_suggestion = convert to un-typed pointers to only compare their addresses + lint_invalid_reference_casting_assign_to_ref = assigning to `&T` is undefined behavior, consider using an `UnsafeCell` .label = casting happend here diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 756899e50a8cd..4dd9191a1e18b 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1585,6 +1585,45 @@ pub enum InvalidNanComparisonsSuggestion { Spanless, } +#[derive(LintDiagnostic)] +#[diag(lint_invalid_pointer_trait_comparisons)] +pub struct InvalidPointerTraitComparisons { + #[subdiagnostic] + pub addr_metadata_suggestion: InvalidPointerTraitComparisonsAddrMetadataSuggestion, + #[subdiagnostic] + pub addr_suggestion: InvalidPointerTraitComparisonsAddrSuggestion, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion( + lint_addr_metadata_suggestion, + style = "verbose", + applicability = "machine-applicable" +)] +pub struct InvalidPointerTraitComparisonsAddrMetadataSuggestion { + #[suggestion_part(code = "!std::ptr::eq(")] + pub left_ne: Option, + #[suggestion_part(code = "std::ptr::eq(")] + pub left_eq: Option, + #[suggestion_part(code = ", ")] + pub middle: Span, + #[suggestion_part(code = ")")] + pub right: Span, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion( + lint_addr_suggestion, + style = "verbose", + applicability = "machine-applicable" +)] +pub struct InvalidPointerTraitComparisonsAddrSuggestion { + #[suggestion_part(code = " as *const ()")] + pub left: Span, + #[suggestion_part(code = " as *const ()")] + pub right: Span, +} + pub struct ImproperCTypes<'a> { pub ty: Ty<'a>, pub desc: &'a str, diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index c04053d1865db..229000df6420b 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -3,10 +3,11 @@ use crate::{ lints::{ AtomicOrderingFence, AtomicOrderingLoad, AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons, InvalidNanComparisonsSuggestion, - OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign, OverflowingBinHexSignBitSub, - OverflowingBinHexSub, OverflowingInt, OverflowingIntHelp, OverflowingLiteral, - OverflowingUInt, RangeEndpointOutOfRange, UnusedComparisons, UseInclusiveRange, - VariantSizeDifferencesDiag, + InvalidPointerTraitComparisons, InvalidPointerTraitComparisonsAddrMetadataSuggestion, + InvalidPointerTraitComparisonsAddrSuggestion, OnlyCastu8ToChar, OverflowingBinHex, + OverflowingBinHexSign, OverflowingBinHexSignBitSub, OverflowingBinHexSub, OverflowingInt, + OverflowingIntHelp, OverflowingLiteral, OverflowingUInt, RangeEndpointOutOfRange, + UnusedComparisons, UseInclusiveRange, VariantSizeDifferencesDiag, }, }; use crate::{LateContext, LateLintPass, LintContext}; @@ -17,10 +18,10 @@ use rustc_errors::DiagnosticMessage; use rustc_hir as hir; use rustc_hir::{is_range_literal, Expr, ExprKind, Node}; use rustc_middle::ty::layout::{IntegerExt, LayoutOf, SizeSkeleton}; -use rustc_middle::ty::GenericArgsRef; use rustc_middle::ty::{ self, AdtKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, }; +use rustc_middle::ty::{GenericArgsRef, TypeAndMut}; use rustc_span::def_id::LocalDefId; use rustc_span::source_map; use rustc_span::symbol::sym; @@ -136,6 +137,37 @@ declare_lint! { "detects invalid floating point NaN comparisons" } +declare_lint! { + /// The `invalid_pointer_trait_comparisons` lint checks comparison + /// of `*const dyn Trait` as the operands. + /// + /// ### Example + /// + /// ```rust + /// # struct A; + /// # struct B; + /// # + /// # trait T {} + /// # impl T for A {} + /// # impl T for B {} + /// + /// let ab = (A, B); + /// let a = &ab.0 as *const dyn T; + /// let b = &ab.1 as *const dyn T; + /// + /// let _ = a == b; + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// The comparison include metadata which is not guaranteed to be equivalent. + INVALID_POINTER_TRAIT_COMPARISONS, + Warn, + "detects invalid pointer trait comparisons" +} + #[derive(Copy, Clone)] pub struct TypeLimits { /// Id of the last visited negated expression @@ -144,7 +176,12 @@ pub struct TypeLimits { negated_expr_span: Option, } -impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS, INVALID_NAN_COMPARISONS]); +impl_lint_pass!(TypeLimits => [ + UNUSED_COMPARISONS, + OVERFLOWING_LITERALS, + INVALID_NAN_COMPARISONS, + INVALID_POINTER_TRAIT_COMPARISONS +]); impl TypeLimits { pub fn new() -> TypeLimits { @@ -620,6 +657,51 @@ fn lint_nan<'tcx>( cx.emit_spanned_lint(INVALID_NAN_COMPARISONS, e.span, lint); } +fn lint_pointer_trait<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx hir::Expr<'tcx>, + binop: hir::BinOp, + l: &'tcx hir::Expr<'tcx>, + r: &'tcx hir::Expr<'tcx>, +) { + let Some(l_ty) = cx.typeck_results().expr_ty_adjusted_opt(l) else { + return; + }; + let Some(r_ty) = cx.typeck_results().expr_ty_adjusted_opt(r) else { + return; + }; + + fn is_ptr_trait(ty: Ty<'_>) -> bool { + match ty.kind() { + ty::RawPtr(TypeAndMut { mutbl: _, ty }) => { + matches!(ty.kind(), ty::Dynamic(_, _, ty::Dyn)) + } + _ => false, + } + } + + if !is_ptr_trait(l_ty) || !is_ptr_trait(r_ty) { + return; + } + + cx.emit_spanned_lint( + INVALID_POINTER_TRAIT_COMPARISONS, + e.span, + InvalidPointerTraitComparisons { + addr_metadata_suggestion: InvalidPointerTraitComparisonsAddrMetadataSuggestion { + left_ne: (binop.node == hir::BinOpKind::Ne).then(|| l.span.shrink_to_lo()), + left_eq: (binop.node != hir::BinOpKind::Ne).then(|| l.span.shrink_to_lo()), + middle: l.span.shrink_to_hi().until(r.span.shrink_to_lo()), + right: r.span.shrink_to_hi(), + }, + addr_suggestion: InvalidPointerTraitComparisonsAddrSuggestion { + left: l.span.shrink_to_hi(), + right: r.span.shrink_to_hi(), + }, + }, + ); +} + impl<'tcx> LateLintPass<'tcx> for TypeLimits { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) { match e.kind { @@ -636,6 +718,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits { cx.emit_spanned_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons); } else { lint_nan(cx, e, binop, l, r); + lint_pointer_trait(cx, e, binop, l, r); } } } diff --git a/tests/ui/lint/pointer_trait_comparisons.rs b/tests/ui/lint/pointer_trait_comparisons.rs new file mode 100644 index 0000000000000..09fa5fc627feb --- /dev/null +++ b/tests/ui/lint/pointer_trait_comparisons.rs @@ -0,0 +1,21 @@ +// check-pass + +struct A; +struct B; + +trait T {} +impl T for A {} +impl T for B {} + +fn main() { + let ab = (A, B); + let a = &ab.0 as *const dyn T; + let b = &ab.1 as *const dyn T; + + let _ = a == b; + //~^ WARN incorrect pointer trait comparison + let _ = a != b; + //~^ WARN incorrect pointer trait comparison + + let _ = a as *const () == b as *const (); +} diff --git a/tests/ui/lint/pointer_trait_comparisons.stderr b/tests/ui/lint/pointer_trait_comparisons.stderr new file mode 100644 index 0000000000000..ba1b96602beab --- /dev/null +++ b/tests/ui/lint/pointer_trait_comparisons.stderr @@ -0,0 +1,33 @@ +warning: incorrect pointer trait comparison, the comparison include metadata which is not guaranteed to be equivalent + --> $DIR/pointer_trait_comparisons.rs:15:13 + | +LL | let _ = a == b; + | ^^^^^^ + | + = note: `#[warn(invalid_pointer_trait_comparisons)]` on by default +help: use explicit `std::ptr::eq` method to compare metadata and addresses + | +LL | let _ = std::ptr::eq(a, b); + | +++++++++++++ ~ + +help: convert to un-typed pointers to only compare their addresses + | +LL | let _ = a as *const () == b as *const (); + | ++++++++++++ ++++++++++++ + +warning: incorrect pointer trait comparison, the comparison include metadata which is not guaranteed to be equivalent + --> $DIR/pointer_trait_comparisons.rs:17:13 + | +LL | let _ = a != b; + | ^^^^^^ + | +help: use explicit `std::ptr::eq` method to compare metadata and addresses + | +LL | let _ = !std::ptr::eq(a, b); + | ++++++++++++++ ~ + +help: convert to un-typed pointers to only compare their addresses + | +LL | let _ = a as *const () != b as *const (); + | ++++++++++++ ++++++++++++ + +warning: 2 warnings emitted +