forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rollup merge of rust-lang#75671 - nathanwhit:cstring-temp-lint, r=oli…
…-obk Uplift `temporary-cstring-as-ptr` lint from `clippy` into rustc The general consensus seems to be that this lint covers a common enough mistake to warrant inclusion in rustc. The diagnostic message might need some tweaking, as I'm not sure the use of second-person perspective matches the rest of rustc, but I'd like to hear others' thoughts on that. (cc rust-lang#53224). r? @oli-obk
- Loading branch information
Showing
15 changed files
with
177 additions
and
141 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| use crate::LateContext; | ||
| use crate::LateLintPass; | ||
| use crate::LintContext; | ||
| use rustc_hir::{Expr, ExprKind, PathSegment}; | ||
| use rustc_middle::ty; | ||
| use rustc_span::{symbol::sym, ExpnKind, Span}; | ||
|
|
||
| declare_lint! { | ||
| /// The `temporary_cstring_as_ptr` lint detects getting the inner pointer of | ||
| /// a temporary `CString`. | ||
| /// | ||
| /// ### Example | ||
| /// | ||
| /// ```rust | ||
| /// # #![allow(unused)] | ||
| /// # use std::ffi::CString; | ||
| /// let c_str = CString::new("foo").unwrap().as_ptr(); | ||
| /// ``` | ||
| /// | ||
| /// {{produces}} | ||
| /// | ||
| /// ### Explanation | ||
| /// | ||
| /// The inner pointer of a `CString` lives only as long as the `CString` it | ||
| /// points to. Getting the inner pointer of a *temporary* `CString` allows the `CString` | ||
| /// to be dropped at the end of the statement, as it is not being referenced as far as the typesystem | ||
| /// is concerned. This means outside of the statement the pointer will point to freed memory, which | ||
| /// causes undefined behavior if the pointer is later dereferenced. | ||
| pub TEMPORARY_CSTRING_AS_PTR, | ||
| Warn, | ||
| "detects getting the inner pointer of a temporary `CString`" | ||
| } | ||
|
|
||
| declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR]); | ||
|
|
||
| fn in_macro(span: Span) -> bool { | ||
| if span.from_expansion() { | ||
| !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
|
|
||
| fn first_method_call<'tcx>( | ||
| expr: &'tcx Expr<'tcx>, | ||
| ) -> Option<(&'tcx PathSegment<'tcx>, &'tcx [Expr<'tcx>])> { | ||
| if let ExprKind::MethodCall(path, _, args, _) = &expr.kind { | ||
| if args.iter().any(|e| e.span.from_expansion()) { None } else { Some((path, *args)) } | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
| impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr { | ||
| fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | ||
| if in_macro(expr.span) { | ||
| return; | ||
| } | ||
|
|
||
| match first_method_call(expr) { | ||
| Some((path, args)) if path.ident.name == sym::as_ptr => { | ||
| let unwrap_arg = &args[0]; | ||
| let as_ptr_span = path.ident.span; | ||
| match first_method_call(unwrap_arg) { | ||
| Some((path, args)) | ||
| if path.ident.name == sym::unwrap || path.ident.name == sym::expect => | ||
| { | ||
| let source_arg = &args[0]; | ||
| lint_cstring_as_ptr(cx, as_ptr_span, source_arg, unwrap_arg); | ||
| } | ||
| _ => return, | ||
| } | ||
| } | ||
| _ => return, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn lint_cstring_as_ptr( | ||
| cx: &LateContext<'_>, | ||
| as_ptr_span: Span, | ||
| source: &rustc_hir::Expr<'_>, | ||
| unwrap: &rustc_hir::Expr<'_>, | ||
| ) { | ||
| let source_type = cx.typeck_results().expr_ty(source); | ||
| if let ty::Adt(def, substs) = source_type.kind() { | ||
| if cx.tcx.is_diagnostic_item(sym::result_type, def.did) { | ||
| if let ty::Adt(adt, _) = substs.type_at(0).kind() { | ||
| if cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did) { | ||
| cx.struct_span_lint(TEMPORARY_CSTRING_AS_PTR, as_ptr_span, |diag| { | ||
| let mut diag = diag | ||
| .build("getting the inner pointer of a temporary `CString`"); | ||
| diag.span_label(as_ptr_span, "this pointer will be invalid"); | ||
| diag.span_label( | ||
| unwrap.span, | ||
| "this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime", | ||
| ); | ||
| diag.note("pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned"); | ||
| diag.help("for more information, see https://doc.rust-lang.org/reference/destructors.html"); | ||
| diag.emit(); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
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,10 @@ | ||
| // ignore-tidy-linelength | ||
| #![deny(temporary_cstring_as_ptr)] | ||
|
|
||
| use std::ffi::CString; | ||
|
|
||
| fn some_function(data: *const i8) {} | ||
|
|
||
| fn main() { | ||
| some_function(CString::new("").unwrap().as_ptr()); //~ ERROR getting the inner pointer of a temporary `CString` | ||
| } |
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,18 @@ | ||
| error: getting the inner pointer of a temporary `CString` | ||
| --> $DIR/lint-temporary-cstring-as-param.rs:9:45 | ||
| | | ||
| LL | some_function(CString::new("").unwrap().as_ptr()); | ||
| | ------------------------- ^^^^^^ this pointer will be invalid | ||
| | | | ||
| | this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime | ||
| | | ||
| note: the lint level is defined here | ||
| --> $DIR/lint-temporary-cstring-as-param.rs:2:9 | ||
| | | ||
| LL | #![deny(temporary_cstring_as_ptr)] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| = note: pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned | ||
| = help: for more information, see https://doc.rust-lang.org/reference/destructors.html | ||
|
|
||
| error: aborting due to previous error | ||
|
|
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,9 @@ | ||
| // ignore-tidy-linelength | ||
| // this program is not technically incorrect, but is an obscure enough style to be worth linting | ||
| #![deny(temporary_cstring_as_ptr)] | ||
|
|
||
| use std::ffi::CString; | ||
|
|
||
| fn main() { | ||
| let s = CString::new("some text").unwrap().as_ptr(); //~ ERROR getting the inner pointer of a temporary `CString` | ||
| } |
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,18 @@ | ||
| error: getting the inner pointer of a temporary `CString` | ||
| --> $DIR/lint-temporary-cstring-as-ptr.rs:8:48 | ||
| | | ||
| LL | let s = CString::new("some text").unwrap().as_ptr(); | ||
| | ---------------------------------- ^^^^^^ this pointer will be invalid | ||
| | | | ||
| | this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime | ||
| | | ||
| note: the lint level is defined here | ||
| --> $DIR/lint-temporary-cstring-as-ptr.rs:3:9 | ||
| | | ||
| LL | #![deny(temporary_cstring_as_ptr)] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| = note: pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned | ||
| = help: for more information, see https://doc.rust-lang.org/reference/destructors.html | ||
|
|
||
| error: aborting due to previous error | ||
|
|
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
Oops, something went wrong.