diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 4aeaf616816..54039d56929 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -882,6 +882,12 @@ lint_unnameable_test_items = cannot test inner items lint_unnecessary_qualification = unnecessary qualification .suggestion = remove the unnecessary path segments +lint_unpredictable_fn_pointer_comparisons = function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + .note_duplicated_fn = the address of the same function can vary between different codegen units + .note_deduplicated_fn = furthermore, different functions could have the same address after being merged together + .note_visit_fn_addr_eq = for more information visit + .fn_addr_eq_suggestion = refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + lint_unqualified_local_imports = `use` of a local item without leading `self::`, `super::`, or `crate::` lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 3a854796f67..5721ef644da 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1810,6 +1810,42 @@ pub(crate) enum AmbiguousWidePointerComparisonsAddrSuggestion<'a> { }, } +#[derive(LintDiagnostic)] +pub(crate) enum UnpredictableFunctionPointerComparisons<'a> { + #[diag(lint_unpredictable_fn_pointer_comparisons)] + #[note(lint_note_duplicated_fn)] + #[note(lint_note_deduplicated_fn)] + #[note(lint_note_visit_fn_addr_eq)] + Suggestion { + #[subdiagnostic] + sugg: UnpredictableFunctionPointerComparisonsSuggestion<'a>, + }, + #[diag(lint_unpredictable_fn_pointer_comparisons)] + #[note(lint_note_duplicated_fn)] + #[note(lint_note_deduplicated_fn)] + #[note(lint_note_visit_fn_addr_eq)] + Warn, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion( + lint_fn_addr_eq_suggestion, + style = "verbose", + applicability = "maybe-incorrect" +)] +pub(crate) struct UnpredictableFunctionPointerComparisonsSuggestion<'a> { + pub ne: &'a str, + pub cast_right: String, + pub deref_left: &'a str, + pub deref_right: &'a str, + #[suggestion_part(code = "{ne}std::ptr::fn_addr_eq({deref_left}")] + pub left: Span, + #[suggestion_part(code = ", {deref_right}")] + pub middle: Span, + #[suggestion_part(code = "{cast_right})")] + pub right: Span, +} + pub(crate) 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 b1d7d4ab689..33650be056d 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -23,7 +23,9 @@ use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad, AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons, - InvalidNanComparisonsSuggestion, UnusedComparisons, VariantSizeDifferencesDiag, + InvalidNanComparisonsSuggestion, UnpredictableFunctionPointerComparisons, + UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, + VariantSizeDifferencesDiag, }; use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; @@ -166,6 +168,35 @@ declare_lint! { "detects ambiguous wide pointer comparisons" } +declare_lint! { + /// The `unpredictable_function_pointer_comparisons` lint checks comparison + /// of function pointer as the operands. + /// + /// ### Example + /// + /// ```rust + /// fn a() {} + /// fn b() {} + /// + /// let f: fn() = a; + /// let g: fn() = b; + /// + /// let _ = f == g; + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Function pointers comparisons do not produce meaningful result since + /// they are never guaranteed to be unique and could vary between different + /// code generation units. Furthermore, different functions could have the + /// same address after being merged together. + UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS, + Warn, + "detects unpredictable function pointer comparisons" +} + #[derive(Copy, Clone, Default)] pub(crate) struct TypeLimits { /// Id of the last visited negated expression @@ -178,7 +209,8 @@ impl_lint_pass!(TypeLimits => [ UNUSED_COMPARISONS, OVERFLOWING_LITERALS, INVALID_NAN_COMPARISONS, - AMBIGUOUS_WIDE_POINTER_COMPARISONS + AMBIGUOUS_WIDE_POINTER_COMPARISONS, + UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS ]); impl TypeLimits { @@ -255,7 +287,7 @@ fn lint_nan<'tcx>( cx.emit_span_lint(INVALID_NAN_COMPARISONS, e.span, lint); } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Copy, Clone)] enum ComparisonOp { BinOp(hir::BinOpKind), Other, @@ -383,6 +415,100 @@ fn lint_wide_pointer<'tcx>( ); } +fn lint_fn_pointer<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx hir::Expr<'tcx>, + cmpop: ComparisonOp, + l: &'tcx hir::Expr<'tcx>, + r: &'tcx hir::Expr<'tcx>, +) { + let peel_refs = |mut ty: Ty<'tcx>| -> (Ty<'tcx>, usize) { + let mut refs = 0; + + while let ty::Ref(_, inner_ty, _) = ty.kind() { + ty = *inner_ty; + refs += 1; + } + + (ty, refs) + }; + + // Left and right operands can have borrows, remove them + let l = l.peel_borrows(); + let r = r.peel_borrows(); + + let Some(l_ty) = cx.typeck_results().expr_ty_opt(l) else { return }; + let Some(r_ty) = cx.typeck_results().expr_ty_opt(r) else { return }; + + // Remove any references as `==` will deref through them (and count the + // number of references removed, for latter). + let (l_ty, l_ty_refs) = peel_refs(l_ty); + let (r_ty, r_ty_refs) = peel_refs(r_ty); + + if !l_ty.is_fn() || !r_ty.is_fn() { + return; + } + + // Let's try to suggest `ptr::fn_addr_eq` if/when possible. + + let is_eq_ne = matches!(cmpop, ComparisonOp::BinOp(hir::BinOpKind::Eq | hir::BinOpKind::Ne)); + + if !is_eq_ne { + // Neither `==` nor `!=`, we can't suggest `ptr::fn_addr_eq`, just show the warning. + return cx.emit_span_lint( + UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS, + e.span, + UnpredictableFunctionPointerComparisons::Warn, + ); + } + + let (Some(l_span), Some(r_span)) = + (l.span.find_ancestor_inside(e.span), r.span.find_ancestor_inside(e.span)) + else { + // No appropriate spans for the left and right operands, just show the warning. + return cx.emit_span_lint( + UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS, + e.span, + UnpredictableFunctionPointerComparisons::Warn, + ); + }; + + let ne = if cmpop == ComparisonOp::BinOp(hir::BinOpKind::Ne) { "!" } else { "" }; + + // `ptr::fn_addr_eq` only works with raw pointer, deref any references. + let deref_left = &*"*".repeat(l_ty_refs); + let deref_right = &*"*".repeat(r_ty_refs); + + let left = e.span.shrink_to_lo().until(l_span.shrink_to_lo()); + let middle = l_span.shrink_to_hi().until(r_span.shrink_to_lo()); + let right = r_span.shrink_to_hi().until(e.span.shrink_to_hi()); + + // We only check for a right cast as `FnDef` == `FnPtr` is not possible, + // only `FnPtr == FnDef` is possible. + let cast_right = if !r_ty.is_fn_ptr() { + let fn_sig = r_ty.fn_sig(cx.tcx); + format!(" as {fn_sig}") + } else { + String::new() + }; + + cx.emit_span_lint( + UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS, + e.span, + UnpredictableFunctionPointerComparisons::Suggestion { + sugg: UnpredictableFunctionPointerComparisonsSuggestion { + ne, + deref_left, + deref_right, + left, + middle, + right, + cast_right, + }, + }, + ); +} + impl<'tcx> LateLintPass<'tcx> for TypeLimits { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) { match e.kind { @@ -399,7 +525,9 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits { cx.emit_span_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons); } else { lint_nan(cx, e, binop, l, r); - lint_wide_pointer(cx, e, ComparisonOp::BinOp(binop.node), l, r); + let cmpop = ComparisonOp::BinOp(binop.node); + lint_wide_pointer(cx, e, cmpop, l, r); + lint_fn_pointer(cx, e, cmpop, l, r); } } } @@ -411,6 +539,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits { && let Some(cmpop) = diag_item_cmpop(diag_item) => { lint_wide_pointer(cx, e, cmpop, l, r); + lint_fn_pointer(cx, e, cmpop, l, r); } hir::ExprKind::MethodCall(_, l, [r], _) if let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) @@ -418,6 +547,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits { && let Some(cmpop) = diag_item_cmpop(diag_item) => { lint_wide_pointer(cx, e, cmpop, l, r); + lint_fn_pointer(cx, e, cmpop, l, r); } _ => {} }; diff --git a/tests/ui/lint/fn-ptr-comparisons-weird.rs b/tests/ui/lint/fn-ptr-comparisons-weird.rs new file mode 100644 index 00000000000..171fbfb8727 --- /dev/null +++ b/tests/ui/lint/fn-ptr-comparisons-weird.rs @@ -0,0 +1,15 @@ +//@ check-pass + +fn main() { + let f: fn() = main; + let g: fn() = main; + + let _ = f > g; + //~^ WARN function pointer comparisons + let _ = f >= g; + //~^ WARN function pointer comparisons + let _ = f <= g; + //~^ WARN function pointer comparisons + let _ = f < g; + //~^ WARN function pointer comparisons +} diff --git a/tests/ui/lint/fn-ptr-comparisons-weird.stderr b/tests/ui/lint/fn-ptr-comparisons-weird.stderr new file mode 100644 index 00000000000..f2371663922 --- /dev/null +++ b/tests/ui/lint/fn-ptr-comparisons-weird.stderr @@ -0,0 +1,43 @@ +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons-weird.rs:7:13 + | +LL | let _ = f > g; + | ^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit + = note: `#[warn(unpredictable_function_pointer_comparisons)]` on by default + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons-weird.rs:9:13 + | +LL | let _ = f >= g; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons-weird.rs:11:13 + | +LL | let _ = f <= g; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons-weird.rs:13:13 + | +LL | let _ = f < g; + | ^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit + +warning: 4 warnings emitted + diff --git a/tests/ui/lint/fn-ptr-comparisons.fixed b/tests/ui/lint/fn-ptr-comparisons.fixed new file mode 100644 index 00000000000..22f16177a04 --- /dev/null +++ b/tests/ui/lint/fn-ptr-comparisons.fixed @@ -0,0 +1,58 @@ +//@ check-pass +//@ run-rustfix + +extern "C" { + fn test(); +} + +fn a() {} + +extern "C" fn c() {} + +extern "C" fn args(_a: i32) -> i32 { 0 } + +#[derive(PartialEq, Eq)] +struct A { + f: fn(), +} + +fn main() { + let f: fn() = a; + let g: fn() = f; + + let a1 = A { f }; + let a2 = A { f }; + + let _ = std::ptr::fn_addr_eq(f, a as fn()); + //~^ WARN function pointer comparisons + let _ = !std::ptr::fn_addr_eq(f, a as fn()); + //~^ WARN function pointer comparisons + let _ = std::ptr::fn_addr_eq(f, g); + //~^ WARN function pointer comparisons + let _ = std::ptr::fn_addr_eq(f, f); + //~^ WARN function pointer comparisons + let _ = std::ptr::fn_addr_eq(g, g); + //~^ WARN function pointer comparisons + let _ = std::ptr::fn_addr_eq(g, g); + //~^ WARN function pointer comparisons + let _ = std::ptr::fn_addr_eq(g, g); + //~^ WARN function pointer comparisons + let _ = std::ptr::fn_addr_eq(a as fn(), g); + //~^ WARN function pointer comparisons + + let cfn: extern "C" fn() = c; + let _ = std::ptr::fn_addr_eq(cfn, c as extern "C" fn()); + //~^ WARN function pointer comparisons + + let argsfn: extern "C" fn(i32) -> i32 = args; + let _ = std::ptr::fn_addr_eq(argsfn, args as extern "C" fn(i32) -> i32); + //~^ WARN function pointer comparisons + + let t: unsafe extern "C" fn() = test; + let _ = std::ptr::fn_addr_eq(t, test as unsafe extern "C" fn()); + //~^ WARN function pointer comparisons + + let _ = a1 == a2; // should not warn + let _ = std::ptr::fn_addr_eq(a1.f, a2.f); + //~^ WARN function pointer comparisons +} diff --git a/tests/ui/lint/fn-ptr-comparisons.rs b/tests/ui/lint/fn-ptr-comparisons.rs new file mode 100644 index 00000000000..90a8ab5c926 --- /dev/null +++ b/tests/ui/lint/fn-ptr-comparisons.rs @@ -0,0 +1,58 @@ +//@ check-pass +//@ run-rustfix + +extern "C" { + fn test(); +} + +fn a() {} + +extern "C" fn c() {} + +extern "C" fn args(_a: i32) -> i32 { 0 } + +#[derive(PartialEq, Eq)] +struct A { + f: fn(), +} + +fn main() { + let f: fn() = a; + let g: fn() = f; + + let a1 = A { f }; + let a2 = A { f }; + + let _ = f == a; + //~^ WARN function pointer comparisons + let _ = f != a; + //~^ WARN function pointer comparisons + let _ = f == g; + //~^ WARN function pointer comparisons + let _ = f == f; + //~^ WARN function pointer comparisons + let _ = g == g; + //~^ WARN function pointer comparisons + let _ = g == g; + //~^ WARN function pointer comparisons + let _ = &g == &g; + //~^ WARN function pointer comparisons + let _ = a as fn() == g; + //~^ WARN function pointer comparisons + + let cfn: extern "C" fn() = c; + let _ = cfn == c; + //~^ WARN function pointer comparisons + + let argsfn: extern "C" fn(i32) -> i32 = args; + let _ = argsfn == args; + //~^ WARN function pointer comparisons + + let t: unsafe extern "C" fn() = test; + let _ = t == test; + //~^ WARN function pointer comparisons + + let _ = a1 == a2; // should not warn + let _ = a1.f == a2.f; + //~^ WARN function pointer comparisons +} diff --git a/tests/ui/lint/fn-ptr-comparisons.stderr b/tests/ui/lint/fn-ptr-comparisons.stderr new file mode 100644 index 00000000000..eaba23a461a --- /dev/null +++ b/tests/ui/lint/fn-ptr-comparisons.stderr @@ -0,0 +1,171 @@ +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:26:13 + | +LL | let _ = f == a; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit + = note: `#[warn(unpredictable_function_pointer_comparisons)]` on by default +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(f, a as fn()); + | +++++++++++++++++++++ ~ ++++++++ + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:28:13 + | +LL | let _ = f != a; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = !std::ptr::fn_addr_eq(f, a as fn()); + | ++++++++++++++++++++++ ~ ++++++++ + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:30:13 + | +LL | let _ = f == g; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(f, g); + | +++++++++++++++++++++ ~ + + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:32:13 + | +LL | let _ = f == f; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(f, f); + | +++++++++++++++++++++ ~ + + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:34:13 + | +LL | let _ = g == g; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(g, g); + | +++++++++++++++++++++ ~ + + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:36:13 + | +LL | let _ = g == g; + | ^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(g, g); + | +++++++++++++++++++++ ~ + + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:38:13 + | +LL | let _ = &g == &g; + | ^^^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(g, g); + | ~~~~~~~~~~~~~~~~~~~~~ ~ + + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:40:13 + | +LL | let _ = a as fn() == g; + | ^^^^^^^^^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(a as fn(), g); + | +++++++++++++++++++++ ~ + + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:44:13 + | +LL | let _ = cfn == c; + | ^^^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(cfn, c as extern "C" fn()); + | +++++++++++++++++++++ ~ +++++++++++++++++++ + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:48:13 + | +LL | let _ = argsfn == args; + | ^^^^^^^^^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(argsfn, args as extern "C" fn(i32) -> i32); + | +++++++++++++++++++++ ~ +++++++++++++++++++++++++++++ + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:52:13 + | +LL | let _ = t == test; + | ^^^^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(t, test as unsafe extern "C" fn()); + | +++++++++++++++++++++ ~ ++++++++++++++++++++++++++ + +warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique + --> $DIR/fn-ptr-comparisons.rs:56:13 + | +LL | let _ = a1.f == a2.f; + | ^^^^^^^^^^^^ + | + = note: the address of the same function can vary between different codegen units + = note: furthermore, different functions could have the same address after being merged together + = note: for more information visit +help: refactor your code, or use `std::ptr::fn_addr_eq` to suppress the lint + | +LL | let _ = std::ptr::fn_addr_eq(a1.f, a2.f); + | +++++++++++++++++++++ ~ + + +warning: 12 warnings emitted +