Skip to content

Commit 0e65248

Browse files
committed
Check for 'static
1 parent aa2b23d commit 0e65248

File tree

3 files changed

+89
-21
lines changed

3 files changed

+89
-21
lines changed

clippy_lints/src/ptr_to_temporary.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use clippy_utils::consts::is_promotable;
22
use clippy_utils::diagnostics::span_lint_and_note;
3-
use clippy_utils::{is_from_proc_macro, is_temporary};
3+
use clippy_utils::{is_from_proc_macro, is_temporary, path_res};
44
use rustc_hir::{BorrowKind, Expr, ExprKind, ItemKind, OwnerNode};
55
use rustc_lint::{LateContext, LateLintPass, LintContext};
66
use rustc_middle::lint::in_external_macro;
7-
use rustc_middle::ty;
7+
use rustc_middle::ty::TypeVisitableExt;
88
use rustc_session::{declare_lint_pass, declare_tool_lint};
99
use rustc_span::sym;
1010

@@ -101,15 +101,24 @@ fn check_for_dangling_as_ptr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx
101101
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
102102
&& cx.tcx.fn_sig(def_id).skip_binder().output().skip_binder().is_unsafe_ptr()
103103
&& matches!(cx.tcx.crate_name(def_id.krate), sym::core | sym::alloc | sym::std)
104-
// These will almost always be promoted yet have the `as_ptr` method. Ideally we would
105-
// check if these would be promoted but our logic considers any function call to be
106-
// non-promotable, but in this case it will be as it's `'static`, soo...
107-
&& !matches!(
108-
cx.typeck_results().expr_ty(recv).peel_refs().kind(),
109-
ty::Str | ty::Array(_, _) | ty::Slice(_)
110-
)
111104
&& is_temporary(cx, recv)
112105
&& !is_from_proc_macro(cx, expr)
106+
// The typeck table only contains `ReErased`.
107+
&& let Some(def_id) = match recv.kind {
108+
ExprKind::Call(path, _) => {
109+
path_res(cx, path).opt_def_id()
110+
},
111+
ExprKind::MethodCall(..) => {
112+
cx.typeck_results().type_dependent_def_id(recv.hir_id)
113+
},
114+
// Anything else will either always have a `'static` lifetime or won't be dropped at
115+
// the end of the statement (i.e., will have a longer lifetime), so we can skip them
116+
_ => None,
117+
}
118+
&& let ty = cx.tcx.fn_sig(def_id).subst_identity().output()
119+
// afaik, any `ReLateBound` means it isn't static. Lifetimes are confusing under the
120+
// hood tho so this is likely wrong, and we'll need a smarter heuristic
121+
&& (ty.has_late_bound_regions() || !ty.skip_binder().is_ref())
113122
{
114123
span_lint_and_note(
115124
cx,

tests/ui/ptr_to_temporary.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ fn bad_11() {
7676
let pab = unsafe { AtomicBool::new(true).as_ptr() };
7777
}
7878

79+
fn bad_12() {
80+
let ps = vec![1].as_slice().as_ptr();
81+
}
82+
83+
fn bad_13() {
84+
// TODO: Not linted. We need to normalize these
85+
let ps = <[i32]>::as_ptr(Vec::as_slice(&vec![1]));
86+
}
87+
7988
// TODO: We need more tests here...
8089

8190
fn fine_1() -> *const i32 {
@@ -93,6 +102,7 @@ fn fine_3() -> *const i32 {
93102
}
94103

95104
fn fine_4() {
105+
// This shouldn't be linted!
96106
let pa = ([1],).0.as_ptr();
97107
}
98108

@@ -105,12 +115,37 @@ fn fine_5() {
105115
}
106116

107117
fn fine_6() {
118+
fn helper<'a>() -> &'a str {
119+
"i'm not ub"
120+
}
121+
122+
let ps = helper().as_ptr();
123+
}
124+
125+
fn fine_7() {
108126
fn helper() -> &'static [i32; 1] {
109127
&[1]
110128
}
111129

112130
let pa = helper().as_ptr();
113-
unsafe { *pa };
131+
}
132+
133+
fn fine_8() {
134+
let ps = "a".as_ptr();
135+
}
136+
137+
fn fine_9() {
138+
let pcstr = CString::new("asd".to_owned()).unwrap();
139+
// Not UB, as `pcstr` is not a temporary
140+
// TODO: Don't lint this! We need a check for whether a chain begins with a local...
141+
pcstr.as_c_str().as_ptr();
142+
}
143+
144+
fn fine_10() {
145+
let pcstr = CString::new("asd".to_owned()).unwrap();
146+
// Not UB, as `pcstr` is not a temporary
147+
// TODO: Same here, but this time it needs to be normalized
148+
CString::as_c_str(&pcstr).as_ptr();
114149
}
115150

116151
external! {

tests/ui/ptr_to_temporary.stderr

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,3 @@
1-
error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr`
2-
--> $DIR/ptr_to_temporary.rs:6:5
3-
|
4-
LL | clippy::temporary_cstring_as_ptr,
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr`
6-
|
7-
= note: `-D renamed-and-removed-lints` implied by `-D warnings`
8-
91
error: returning a raw pointer to a temporary value that cannot be promoted to a constant
102
--> $DIR/ptr_to_temporary.rs:22:5
113
|
@@ -55,6 +47,14 @@ LL | let pv = vec![1].as_ptr();
5547
|
5648
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
5749

50+
error: calling `as_ptr` on a temporary value
51+
--> $DIR/ptr_to_temporary.rs:59:14
52+
|
53+
LL | let pa = helper().as_ptr();
54+
| ^^^^^^^^^^^^^^^^^
55+
|
56+
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
57+
5858
error: calling `as_ptr` on a temporary value
5959
--> $DIR/ptr_to_temporary.rs:63:14
6060
|
@@ -72,20 +72,44 @@ LL | let prc = RefCell::new("oops more ub").as_ptr();
7272
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
7373

7474
error: calling `as_ptr` on a temporary value
75-
--> $DIR/ptr_to_temporary.rs:71:26
75+
--> $DIR/ptr_to_temporary.rs:72:26
7676
|
7777
LL | let pcstr = unsafe { CString::new(vec![]).unwrap().as_ptr() };
7878
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7979
|
8080
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
8181

8282
error: calling `as_ptr` on a temporary value
83-
--> $DIR/ptr_to_temporary.rs:75:24
83+
--> $DIR/ptr_to_temporary.rs:76:24
8484
|
8585
LL | let pab = unsafe { AtomicBool::new(true).as_ptr() };
8686
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8787
|
8888
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
8989

90-
error: aborting due to 11 previous errors
90+
error: calling `as_ptr` on a temporary value
91+
--> $DIR/ptr_to_temporary.rs:80:14
92+
|
93+
LL | let ps = vec![1].as_slice().as_ptr();
94+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
95+
|
96+
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
97+
98+
error: calling `as_ptr` on a temporary value
99+
--> $DIR/ptr_to_temporary.rs:141:5
100+
|
101+
LL | pcstr.as_c_str().as_ptr();
102+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
103+
|
104+
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
105+
106+
error: calling `as_ptr` on a temporary value
107+
--> $DIR/ptr_to_temporary.rs:148:5
108+
|
109+
LL | CString::as_c_str(&pcstr).as_ptr();
110+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
111+
|
112+
= note: usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer
113+
114+
error: aborting due to 14 previous errors
91115

0 commit comments

Comments
 (0)