From 935fc3642a61e1545e1b98bf4716acbb7bf12e91 Mon Sep 17 00:00:00 2001 From: Ayrton Date: Wed, 21 Oct 2020 17:19:21 -0400 Subject: [PATCH] Added documentation for `function_item_references` lint Added documentation for `function_item_references` lint to the rustc book and fixed comments in the lint checker itself. --- ...erences.rs => function_item_references.rs} | 92 ++++++++++--------- compiler/rustc_mir/src/transform/mod.rs | 4 +- compiler/rustc_session/src/lint/builtin.rs | 24 +++++ ...erences.rs => function-item-references.rs} | 0 ...stderr => function-item-references.stderr} | 58 ++++++------ 5 files changed, 105 insertions(+), 73 deletions(-) rename compiler/rustc_mir/src/transform/{function_references.rs => function_item_references.rs} (66%) rename src/test/ui/lint/{function-references.rs => function-item-references.rs} (100%) rename src/test/ui/lint/{function-references.stderr => function-item-references.stderr} (77%) diff --git a/compiler/rustc_mir/src/transform/function_references.rs b/compiler/rustc_mir/src/transform/function_item_references.rs similarity index 66% rename from compiler/rustc_mir/src/transform/function_references.rs rename to compiler/rustc_mir/src/transform/function_item_references.rs index 6e94fad5d0d..3a30b41d681 100644 --- a/compiler/rustc_mir/src/transform/function_references.rs +++ b/compiler/rustc_mir/src/transform/function_item_references.rs @@ -3,7 +3,7 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; use rustc_middle::ty::{ self, - subst::{GenericArgKind, Subst}, + subst::{GenericArgKind, Subst, SubstsRef}, PredicateAtom, Ty, TyCtxt, TyS, }; use rustc_session::lint::builtin::FUNCTION_ITEM_REFERENCES; @@ -27,6 +27,9 @@ struct FunctionItemRefChecker<'a, 'tcx> { } impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> { + /// Emits a lint for function reference arguments bound by `fmt::Pointer` or passed to + /// `transmute`. This only handles arguments in calls outside macro expansions to avoid double + /// counting function references formatted as pointers by macros. fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { if let TerminatorKind::Call { func, @@ -38,11 +41,11 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> { } = &terminator.kind { let source_info = *self.body.source_info(location); - //this handles all function calls outside macros + // Only handle function calls outside macros if !source_info.span.from_expansion() { let func_ty = func.ty(self.body, self.tcx); if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() { - //handle `std::mem::transmute` + // Handle calls to `transmute` if self.tcx.is_diagnostic_item(sym::transmute, def_id) { let arg_ty = args[0].ty(self.body, self.tcx); for generic_inner_ty in arg_ty.walk() { @@ -55,48 +58,16 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> { } } } else { - //handle any function call with `std::fmt::Pointer` as a bound trait - //this includes calls to `std::fmt::Pointer::fmt` outside of macros - let param_env = self.tcx.param_env(def_id); - let bounds = param_env.caller_bounds(); - for bound in bounds { - if let Some(bound_ty) = self.is_pointer_trait(&bound.skip_binders()) { - //get the argument types as they appear in the function signature - let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs(); - for (arg_num, arg_def) in arg_defs.iter().enumerate() { - //for all types reachable from the argument type in the fn sig - for generic_inner_ty in arg_def.walk() { - if let GenericArgKind::Type(inner_ty) = - generic_inner_ty.unpack() - { - //if the inner type matches the type bound by `Pointer` - if TyS::same_type(inner_ty, bound_ty) { - //do a substitution using the parameters from the callsite - let subst_ty = inner_ty.subst(self.tcx, substs_ref); - if let Some(fn_id) = - FunctionItemRefChecker::is_fn_ref(subst_ty) - { - let ident = - self.tcx.item_name(fn_id).to_ident_string(); - let span = self.nth_arg_span(&args, arg_num); - self.emit_lint(ident, fn_id, source_info, span); - } - } - } - } - } - } - } + self.check_bound_args(def_id, substs_ref, &args, source_info); } } } } self.super_terminator(terminator, location); } - //This handles `std::fmt::Pointer::fmt` when it's used in the formatting macros. - //It's handled as an operand instead of a Call terminator so it won't depend on - //whether the formatting macros call `fmt` directly, transmute it first or other - //internal fmt details. + /// Emits a lint for function references formatted with `fmt::Pointer::fmt` by macros. These + /// cases are handled as operands instead of call terminators to avoid any dependence on + /// unstable, internal formatting details like whether `fmt` is called directly or not. fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) { let source_info = *self.body.source_info(location); if source_info.span.from_expansion() { @@ -105,8 +76,8 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> { if self.tcx.is_diagnostic_item(sym::pointer_trait_fmt, def_id) { let param_ty = substs_ref.type_at(0); if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(param_ty) { - //the operand's ctxt wouldn't display the lint since it's inside a macro - //so we have to use the callsite's ctxt + // The operand's ctxt wouldn't display the lint since it's inside a macro so + // we have to use the callsite's ctxt. let callsite_ctxt = source_info.span.source_callsite().ctxt(); let span = source_info.span.with_ctxt(callsite_ctxt); let ident = self.tcx.item_name(fn_id).to_ident_string(); @@ -120,7 +91,42 @@ impl<'a, 'tcx> Visitor<'tcx> for FunctionItemRefChecker<'a, 'tcx> { } impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> { - //return the bound parameter type if the trait is `std::fmt::Pointer` + /// Emits a lint for function reference arguments bound by `fmt::Pointer` in calls to the + /// function defined by `def_id` with the substitutions `substs_ref`. + fn check_bound_args( + &self, + def_id: DefId, + substs_ref: SubstsRef<'tcx>, + args: &Vec>, + source_info: SourceInfo, + ) { + let param_env = self.tcx.param_env(def_id); + let bounds = param_env.caller_bounds(); + for bound in bounds { + if let Some(bound_ty) = self.is_pointer_trait(&bound.skip_binders()) { + // Get the argument types as they appear in the function signature. + let arg_defs = self.tcx.fn_sig(def_id).skip_binder().inputs(); + for (arg_num, arg_def) in arg_defs.iter().enumerate() { + // For all types reachable from the argument type in the fn sig + for generic_inner_ty in arg_def.walk() { + if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() { + // If the inner type matches the type bound by `Pointer` + if TyS::same_type(inner_ty, bound_ty) { + // Do a substitution using the parameters from the callsite + let subst_ty = inner_ty.subst(self.tcx, substs_ref); + if let Some(fn_id) = FunctionItemRefChecker::is_fn_ref(subst_ty) { + let ident = self.tcx.item_name(fn_id).to_ident_string(); + let span = self.nth_arg_span(args, arg_num); + self.emit_lint(ident, fn_id, source_info, span); + } + } + } + } + } + } + } + } + /// If the given predicate is the trait `fmt::Pointer`, returns the bound parameter type. fn is_pointer_trait(&self, bound: &PredicateAtom<'tcx>) -> Option> { if let ty::PredicateAtom::Trait(predicate, _) = bound { if self.tcx.is_diagnostic_item(sym::pointer_trait, predicate.def_id()) { @@ -132,6 +138,8 @@ impl<'a, 'tcx> FunctionItemRefChecker<'a, 'tcx> { None } } + /// If a type is a reference or raw pointer to the anonymous type of a function definition, + /// returns that function's `DefId`. fn is_fn_ref(ty: Ty<'tcx>) -> Option { let referent_ty = match ty.kind() { ty::Ref(_, referent_ty, _) => Some(referent_ty), diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs index e43a238c1ba..89db6bb13ca 100644 --- a/compiler/rustc_mir/src/transform/mod.rs +++ b/compiler/rustc_mir/src/transform/mod.rs @@ -27,7 +27,7 @@ pub mod dest_prop; pub mod dump_mir; pub mod early_otherwise_branch; pub mod elaborate_drops; -pub mod function_references; +pub mod function_item_references; pub mod generator; pub mod inline; pub mod instcombine; @@ -267,7 +267,7 @@ fn mir_const<'tcx>( // MIR-level lints. &check_packed_ref::CheckPackedRef, &check_const_item_mutation::CheckConstItemMutation, - &function_references::FunctionItemReferences, + &function_item_references::FunctionItemReferences, // What we need to do constant evaluation. &simplify::SimplifyCfg::new("initial"), &rustc_peek::SanityCheck, diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index 41abdacb54c..806f79af277 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -2648,6 +2648,30 @@ declare_lint! { } declare_lint! { + /// The `function_item_references` lint detects function references that are + /// formatted with [`fmt::Pointer`] or transmuted. + /// + /// [`fmt::Pointer`]: https://doc.rust-lang.org/std/fmt/trait.Pointer.html + /// + /// ### Example + /// + /// ```rust + /// fn foo() { } + /// + /// fn main() { + /// println!("{:p}", &foo); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Taking a reference to a function may be mistaken as a way to obtain a + /// pointer to that function. This can give unexpected results when + /// formatting the reference as a pointer or transmuting it. This lint is + /// issued when function references are formatted as pointers, passed as + /// arguments bound by [`fmt::Pointer`] or transmuted. pub FUNCTION_ITEM_REFERENCES, Warn, "suggest casting functions to pointers when attempting to take references", diff --git a/src/test/ui/lint/function-references.rs b/src/test/ui/lint/function-item-references.rs similarity index 100% rename from src/test/ui/lint/function-references.rs rename to src/test/ui/lint/function-item-references.rs diff --git a/src/test/ui/lint/function-references.stderr b/src/test/ui/lint/function-item-references.stderr similarity index 77% rename from src/test/ui/lint/function-references.stderr rename to src/test/ui/lint/function-item-references.stderr index 5e399c16305..a96b134ed58 100644 --- a/src/test/ui/lint/function-references.stderr +++ b/src/test/ui/lint/function-item-references.stderr @@ -1,173 +1,173 @@ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:40:18 + --> $DIR/function-item-references.rs:40:18 | LL | Pointer::fmt(&zst_ref, f) | ^^^^^^^^ | note: the lint level is defined here - --> $DIR/function-references.rs:3:9 + --> $DIR/function-item-references.rs:3:9 | LL | #![warn(function_item_references)] | ^^^^^^^^^^^^^^^^^^^^^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:77:22 + --> $DIR/function-item-references.rs:77:22 | LL | println!("{:p}", &foo); | ^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:79:20 + --> $DIR/function-item-references.rs:79:20 | LL | print!("{:p}", &foo); | ^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:81:21 + --> $DIR/function-item-references.rs:81:21 | LL | format!("{:p}", &foo); | ^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:84:22 + --> $DIR/function-item-references.rs:84:22 | LL | println!("{:p}", &foo as *const _); | ^^^^^^^^^^^^^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:86:22 + --> $DIR/function-item-references.rs:86:22 | LL | println!("{:p}", zst_ref); | ^^^^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:88:22 + --> $DIR/function-item-references.rs:88:22 | LL | println!("{:p}", cast_zst_ptr); | ^^^^^^^^^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:90:22 + --> $DIR/function-item-references.rs:90:22 | LL | println!("{:p}", coerced_zst_ptr); | ^^^^^^^^^^^^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:93:22 + --> $DIR/function-item-references.rs:93:22 | LL | println!("{:p}", &fn_item); | ^^^^^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:95:22 + --> $DIR/function-item-references.rs:95:22 | LL | println!("{:p}", indirect_ref); | ^^^^^^^^^^^^ warning: cast `nop` with `as fn()` to obtain a function pointer - --> $DIR/function-references.rs:98:22 + --> $DIR/function-item-references.rs:98:22 | LL | println!("{:p}", &nop); | ^^^^ warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer - --> $DIR/function-references.rs:100:22 + --> $DIR/function-item-references.rs:100:22 | LL | println!("{:p}", &bar); | ^^^^ warning: cast `baz` with `as fn(_, _) -> _` to obtain a function pointer - --> $DIR/function-references.rs:102:22 + --> $DIR/function-item-references.rs:102:22 | LL | println!("{:p}", &baz); | ^^^^ warning: cast `unsafe_fn` with `as unsafe fn()` to obtain a function pointer - --> $DIR/function-references.rs:104:22 + --> $DIR/function-item-references.rs:104:22 | LL | println!("{:p}", &unsafe_fn); | ^^^^^^^^^^ warning: cast `c_fn` with `as extern "C" fn()` to obtain a function pointer - --> $DIR/function-references.rs:106:22 + --> $DIR/function-item-references.rs:106:22 | LL | println!("{:p}", &c_fn); | ^^^^^ warning: cast `unsafe_c_fn` with `as unsafe extern "C" fn()` to obtain a function pointer - --> $DIR/function-references.rs:108:22 + --> $DIR/function-item-references.rs:108:22 | LL | println!("{:p}", &unsafe_c_fn); | ^^^^^^^^^^^^ warning: cast `variadic` with `as unsafe extern "C" fn(_, ...)` to obtain a function pointer - --> $DIR/function-references.rs:110:22 + --> $DIR/function-item-references.rs:110:22 | LL | println!("{:p}", &variadic); | ^^^^^^^^^ warning: cast `var` with `as fn(_) -> _` to obtain a function pointer - --> $DIR/function-references.rs:112:22 + --> $DIR/function-item-references.rs:112:22 | LL | println!("{:p}", &std::env::var::); | ^^^^^^^^^^^^^^^^^^^^^^^^ warning: cast `nop` with `as fn()` to obtain a function pointer - --> $DIR/function-references.rs:115:32 + --> $DIR/function-item-references.rs:115:32 | LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar); | ^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:115:38 + --> $DIR/function-item-references.rs:115:38 | LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar); | ^^^^ warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer - --> $DIR/function-references.rs:115:44 + --> $DIR/function-item-references.rs:115:44 | LL | println!("{:p} {:p} {:p}", &nop, &foo, &bar); | ^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:130:41 + --> $DIR/function-item-references.rs:130:41 | LL | std::mem::transmute::<_, usize>(&foo); | ^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:132:50 + --> $DIR/function-item-references.rs:132:50 | LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar)); | ^^^^^^^^^^^^ warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer - --> $DIR/function-references.rs:132:50 + --> $DIR/function-item-references.rs:132:50 | LL | std::mem::transmute::<_, (usize, usize)>((&foo, &bar)); | ^^^^^^^^^^^^ warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer - --> $DIR/function-references.rs:142:15 + --> $DIR/function-item-references.rs:142:15 | LL | print_ptr(&bar); | ^^^^ warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer - --> $DIR/function-references.rs:144:24 + --> $DIR/function-item-references.rs:144:24 | LL | bound_by_ptr_trait(&bar); | ^^^^ warning: cast `bar` with `as fn(_) -> _` to obtain a function pointer - --> $DIR/function-references.rs:146:30 + --> $DIR/function-item-references.rs:146:30 | LL | bound_by_ptr_trait_tuple((&foo, &bar)); | ^^^^^^^^^^^^ warning: cast `foo` with `as fn() -> _` to obtain a function pointer - --> $DIR/function-references.rs:146:30 + --> $DIR/function-item-references.rs:146:30 | LL | bound_by_ptr_trait_tuple((&foo, &bar)); | ^^^^^^^^^^^^