Skip to content

Commit

Permalink
Add warn-by-default lint against pointer trait comparisons
Browse files Browse the repository at this point in the history
  • Loading branch information
Urgau committed Nov 9, 2023
1 parent 287ae4d commit 8e0126b
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 6 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Span>,
#[suggestion_part(code = "std::ptr::eq(")]
pub left_eq: Option<Span>,
#[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,
Expand Down
95 changes: 89 additions & 6 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -144,7 +176,12 @@ pub struct TypeLimits {
negated_expr_span: Option<Span>,
}

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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/lint/pointer_trait_comparisons.rs
Original file line number Diff line number Diff line change
@@ -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 ();
}
33 changes: 33 additions & 0 deletions tests/ui/lint/pointer_trait_comparisons.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 8e0126b

Please sign in to comment.