-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add a new lint for catching unbound lifetimes in return values
This lint is based on some unsoundness found in Libra during security audits. Some function signatures can look sound, but based on the concrete types that end up being used, lifetimes may be unbound instead of anchored to the references of the arguments or the fields of a struct. When combined with unsafe code, this can cause transmuted lifetimes to be not what was intended.
- Loading branch information
Showing
8 changed files
with
210 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| use if_chain::if_chain; | ||
| use rustc::declare_lint_pass; | ||
| use rustc::hir::*; | ||
| use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; | ||
| use rustc_session::declare_tool_lint; | ||
|
|
||
| use crate::utils::span_lint; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// **What it does:** Checks for lifetime annotations on return values | ||
| /// which might not always get bound. | ||
| /// | ||
| /// **Why is this bad?** If the function contains unsafe code which | ||
| /// transmutes to the lifetime, the resulting lifetime may live longer | ||
| /// than was intended. | ||
| /// | ||
| /// **Known problems*: None. | ||
| /// | ||
| /// **Example:** | ||
| /// ```rust | ||
| /// // Bad: unbound return lifetime causing unsoundnes | ||
| /// fn foo<'a>(x: impl AsRef<str> + 'a) -> &'a str { | ||
| /// let s = x.as_ref(); | ||
| /// unsafe { &*(s as *const str) } | ||
| /// } | ||
| /// // Good: bound return lifetime is sound | ||
| /// struct WrappedStr(str); | ||
| /// fn foo<'a>(x: &'a str) -> &'a WrappedStr { | ||
| /// unsafe { &*(x as *const str as *const WrappedStr) } | ||
| /// } | ||
| /// ``` | ||
| pub UNBOUND_RETURN_LIFETIMES, | ||
| correctness, | ||
| "unbound lifetimes in function return values" | ||
| } | ||
|
|
||
| declare_lint_pass!(UnboundReturnLifetimes => [UNBOUND_RETURN_LIFETIMES]); | ||
|
|
||
| impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnboundReturnLifetimes { | ||
| fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) { | ||
| if let ItemKind::Fn(ref sig, ref generics, _id) = item.kind { | ||
| check_fn_inner(cx, &sig.decl, generics, None); | ||
| } | ||
| } | ||
|
|
||
| fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx ImplItem) { | ||
| if let ImplItemKind::Method(ref sig, _) = item.kind { | ||
| let parent_generics = impl_generics(cx, item.hir_id); | ||
| check_fn_inner(cx, &sig.decl, &item.generics, parent_generics); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn check_fn_inner<'a, 'tcx>( | ||
| cx: &LateContext<'a, 'tcx>, | ||
| decl: &'tcx FnDecl, | ||
| generics: &'tcx Generics, | ||
| parent_generics: Option<(&'tcx Generics, Option<&'tcx Generics>, Option<&'tcx Generics>)>, | ||
| ) { | ||
| if let FunctionRetTy::Return(ref typ) = decl.output { | ||
| if let TyKind::Rptr(ref lifetime, _) = typ.kind { | ||
| if let LifetimeName::Param(_) = lifetime.name { | ||
| let target_lifetime = lifetime; | ||
|
|
||
| // check function generics | ||
| // the target lifetime parameter must appear on the left of some outlives relation | ||
| if let Some(param) = generics.get_named(target_lifetime.name.ident().name) { | ||
| if param.bounds.iter().any(|b| if let GenericBound::Outlives(_) = b { true } else { false }) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // check parent generics | ||
| // the target lifetime parameter must appear on the left of some outlives relation | ||
| if let Some((ref parent_generics, _, _)) = parent_generics { | ||
| if let Some(param) = parent_generics.get_named(target_lifetime.name.ident().name) { | ||
| if param.bounds.iter().any(|b| if let GenericBound::Outlives(_) = b { true } else { false }) { | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // check type generics | ||
| // the target lifetime parameter must be included in the struct | ||
| if let Some((_, _, Some(ref typ_generics))) = parent_generics { | ||
| if typ_generics.get_named(target_lifetime.name.ident().name).is_some() { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // check arguments | ||
| // the target lifetime parameter must be included as a lifetime of a reference | ||
| for input in decl.inputs.iter() { | ||
| if let TyKind::Rptr(ref lifetime, _) = input.kind { | ||
| if lifetime.name == target_lifetime.name { | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| span_lint( | ||
| cx, | ||
| UNBOUND_RETURN_LIFETIMES, | ||
| target_lifetime.span, | ||
| "lifetime is unconstrained" | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn impl_generics<'tcx>(cx: &LateContext<'_, 'tcx>, hir_id: HirId) -> Option<(&'tcx Generics, Option<&'tcx Generics>, Option<&'tcx Generics>)> { | ||
| let parent_impl = cx.tcx.hir().get_parent_item(hir_id); | ||
| if_chain! { | ||
| if parent_impl != CRATE_HIR_ID; | ||
| if let Node::Item(item) = cx.tcx.hir().get(parent_impl); | ||
| if let ItemKind::Impl(_, _, _, ref parent_generics, ref _trait_ref, ref ty, _) = item.kind; | ||
| then { | ||
| if let TyKind::Path(ref qpath) = ty.kind { | ||
| if let Some(typ_def_id) = cx.tables.qpath_res(qpath, ty.hir_id).opt_def_id() { | ||
| let typ_generics = cx.tcx.hir().get_generics(typ_def_id); | ||
| return Some((parent_generics, None, typ_generics)) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| None | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| #![allow( | ||
| unused, | ||
| dead_code, | ||
| clippy::needless_lifetimes, | ||
| )] | ||
| #![warn(clippy::unbound_return_lifetimes)] | ||
|
|
||
| use std::hash::Hash; | ||
| use std::collections::HashMap; | ||
|
|
||
| struct FooStr(str); | ||
|
|
||
| fn unbound_fun<'a>(x: impl AsRef<str> + 'a) -> &'a FooStr { | ||
| let x = x.as_ref(); | ||
| unsafe { &*(x as *const str as *const FooStr) } | ||
| } | ||
|
|
||
| fn bound_fun<'a>(x: &'a str) -> &'a FooStr { | ||
| unsafe { &*(x as *const str as *const FooStr) } | ||
| } | ||
|
|
||
| fn bound_fun2<'a, 'b: 'a, S: 'b>(s: &'a S) -> &'b str { | ||
| unreachable!() | ||
| } | ||
|
|
||
| type BarType<'a, T> = Bar<'a, &'a T>; | ||
|
|
||
| struct Bar<'a, T> { | ||
| baz: &'a T, | ||
| } | ||
|
|
||
| impl<'a, T> BarType<'a, T> { | ||
| fn bound_impl_fun(&self, _f: bool) -> &'a T { | ||
| unreachable!() | ||
| } | ||
| } | ||
|
|
||
| pub struct BazMap<'a, K, V, W> { | ||
| alloc: &'a Vec<V>, | ||
| map: HashMap<K, W>, | ||
| } | ||
|
|
||
| pub type BazRefMap<'a, K, V> = BazMap<'a, K, V, &'a V>; | ||
|
|
||
| impl<'a, K, V> BazRefMap<'a, K, V> | ||
| where | ||
| K: Hash + PartialEq, | ||
| { | ||
| pub fn or_insert(&self, key: K, value: V) -> &'a V { | ||
| unreachable!() | ||
| } | ||
| } | ||
|
|
||
|
|
||
| fn main() {} | ||
|
|
||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| error: lifetime is unconstrained | ||
| --> $DIR/unbound_return_lifetimes.rs:13:49 | ||
| | | ||
| LL | fn unbound_fun<'a>(x: impl AsRef<str> + 'a) -> &'a FooStr { | ||
| | ^^ | ||
| | | ||
| = note: `-D clippy::unbound-return-lifetimes` implied by `-D warnings` | ||
|
|
||
| error: aborting due to previous error | ||
|
|
Empty file.