Skip to content

Commit a45a118

Browse files
committed
Auto merge of #144322 - Urgau:dangling-ptr-from-locals, r=oli-obk
Add lint against dangling pointers from local variables ## `dangling_pointers_from_locals` *warn-by-default* The `dangling_pointers_from_locals` lint detects getting a pointer to data of a local that will be dropped at the end of the function. ### Example ```rust fn f() -> *const u8 { let x = 0; &x // returns a dangling ptr to `x` } ``` ```text warning: a dangling pointer will be produced because the local variable `x` will be dropped --> $DIR/dangling-pointers-from-locals.rs:10:5 | LL | fn simple() -> *const u8 { | --------- return type of the function is `*const u8` LL | let x = 0; | - `x` is defined inside the function and will be drop at the end of the function LL | &x | ^^ | = note: pointers do not have a lifetime; after returning, the `u8` will be deallocated at the end of the function because nothing is referencing it as far as the type system is concerned = note: `#[warn(dangling_pointers_from_locals)]` on by default ``` ### Explanation Returning a pointer from a local value will not prolong its lifetime, which means that the value can be dropped and the allocation freed while the pointer still exists, making the pointer dangling. If you need stronger guarantees, consider using references instead, as they are statically verified by the borrow-checker to never dangle. ------ This is related to GitHub codeql [CWE-825](https://github.com/github/codeql/blob/main/rust/ql/src/queries/security/CWE-825/AccessAfterLifetimeBad.rs) which shows examples of such simple miss-use. It should be noted that C compilers warns against such patterns even without `-Wall`, https://godbolt.org/z/P7z98arrc. ------ `@rustbot` labels +I-lang-nominated +T-lang cc `@traviscross` r? compiler
2 parents 383b9c4 + 21ec0d5 commit a45a118

File tree

6 files changed

+602
-8
lines changed

6 files changed

+602
-8
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,12 @@ lint_confusable_identifier_pair = found both `{$existing_sym}` and `{$sym}` as i
207207
208208
lint_custom_inner_attribute_unstable = custom inner attributes are unstable
209209
210+
lint_dangling_pointers_from_locals = a dangling pointer will be produced because the local variable `{$local_var_name}` will be dropped
211+
.ret_ty = return type of the {$fn_kind} is `{$ret_ty}`
212+
.local_var = `{$local_var_name}` is part the {$fn_kind} and will be dropped at the end of the {$fn_kind}
213+
.created_at = dangling pointer created here
214+
.note = pointers do not have a lifetime; after returning, the `{$local_var_ty}` will be deallocated at the end of the {$fn_kind} because nothing is referencing it as far as the type system is concerned
215+
210216
lint_dangling_pointers_from_temporaries = a dangling pointer will be produced because the temporary `{$ty}` will be dropped
211217
.label_ptr = this pointer will immediately be invalid
212218
.label_temporary = this `{$ty}` is deallocated at the end of the statement, bind it to a variable to extend its lifetime

compiler/rustc_lint/src/dangling.rs

Lines changed: 142 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
use rustc_ast::visit::{visit_opt, walk_list};
22
use rustc_hir::attrs::AttributeKind;
3+
use rustc_hir::def::Res;
34
use rustc_hir::def_id::LocalDefId;
45
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr};
5-
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, LangItem, find_attr};
6-
use rustc_middle::ty::{Ty, TyCtxt};
6+
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, FnRetTy, LangItem, TyKind, find_attr};
7+
use rustc_middle::ty::{self, Ty, TyCtxt};
78
use rustc_session::{declare_lint, impl_lint_pass};
89
use rustc_span::{Span, sym};
910

10-
use crate::lints::DanglingPointersFromTemporaries;
11+
use crate::lints::{DanglingPointersFromLocals, DanglingPointersFromTemporaries};
1112
use crate::{LateContext, LateLintPass};
1213

1314
declare_lint! {
@@ -42,6 +43,36 @@ declare_lint! {
4243
"detects getting a pointer from a temporary"
4344
}
4445

46+
declare_lint! {
47+
/// The `dangling_pointers_from_locals` lint detects getting a pointer to data
48+
/// of a local that will be dropped at the end of the function.
49+
///
50+
/// ### Example
51+
///
52+
/// ```rust
53+
/// fn f() -> *const u8 {
54+
/// let x = 0;
55+
/// &x // returns a dangling ptr to `x`
56+
/// }
57+
/// ```
58+
///
59+
/// {{produces}}
60+
///
61+
/// ### Explanation
62+
///
63+
/// Returning a pointer from a local value will not prolong its lifetime,
64+
/// which means that the value can be dropped and the allocation freed
65+
/// while the pointer still exists, making the pointer dangling.
66+
/// This is not an error (as far as the type system is concerned)
67+
/// but probably is not what the user intended either.
68+
///
69+
/// If you need stronger guarantees, consider using references instead,
70+
/// as they are statically verified by the borrow-checker to never dangle.
71+
pub DANGLING_POINTERS_FROM_LOCALS,
72+
Warn,
73+
"detects returning a pointer from a local variable"
74+
}
75+
4576
/// FIXME: false negatives (i.e. the lint is not emitted when it should be)
4677
/// 1. Ways to get a temporary that are not recognized:
4778
/// - `owning_temporary.field`
@@ -53,20 +84,123 @@ declare_lint! {
5384
#[derive(Clone, Copy, Default)]
5485
pub(crate) struct DanglingPointers;
5586

56-
impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]);
87+
impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES, DANGLING_POINTERS_FROM_LOCALS]);
5788

5889
// This skips over const blocks, but they cannot use or return a dangling pointer anyways.
5990
impl<'tcx> LateLintPass<'tcx> for DanglingPointers {
6091
fn check_fn(
6192
&mut self,
6293
cx: &LateContext<'tcx>,
63-
_: FnKind<'tcx>,
64-
_: &'tcx FnDecl<'tcx>,
94+
fn_kind: FnKind<'tcx>,
95+
fn_decl: &'tcx FnDecl<'tcx>,
6596
body: &'tcx Body<'tcx>,
6697
_: Span,
67-
_: LocalDefId,
98+
def_id: LocalDefId,
6899
) {
69-
DanglingPointerSearcher { cx, inside_call_args: false }.visit_body(body)
100+
DanglingPointerSearcher { cx, inside_call_args: false }.visit_body(body);
101+
102+
if let FnRetTy::Return(ret_ty) = &fn_decl.output
103+
&& let TyKind::Ptr(_) = ret_ty.kind
104+
{
105+
// get the return type of the function or closure
106+
let ty = match cx.tcx.type_of(def_id).instantiate_identity().kind() {
107+
ty::FnDef(..) => cx.tcx.fn_sig(def_id).instantiate_identity(),
108+
ty::Closure(_, args) => args.as_closure().sig(),
109+
_ => return,
110+
};
111+
let ty = ty.output();
112+
113+
// this type is only used for layout computation and pretty-printing, neither of them rely on regions
114+
let ty = cx.tcx.instantiate_bound_regions_with_erased(ty);
115+
116+
// verify that we have a pointer type
117+
let inner_ty = match ty.kind() {
118+
ty::RawPtr(inner_ty, _) => *inner_ty,
119+
_ => return,
120+
};
121+
122+
if cx
123+
.tcx
124+
.layout_of(cx.typing_env().as_query_input(inner_ty))
125+
.is_ok_and(|layout| !layout.is_1zst())
126+
{
127+
let dcx = &DanglingPointerLocalContext {
128+
body: def_id,
129+
fn_ret: ty,
130+
fn_ret_span: ret_ty.span,
131+
fn_ret_inner: inner_ty,
132+
fn_kind: match fn_kind {
133+
FnKind::ItemFn(..) => "function",
134+
FnKind::Method(..) => "method",
135+
FnKind::Closure => "closure",
136+
},
137+
};
138+
139+
// look for `return`s
140+
DanglingPointerReturnSearcher { cx, dcx }.visit_body(body);
141+
142+
// analyze implicit return expression
143+
if let ExprKind::Block(block, None) = &body.value.kind
144+
&& let innermost_block = block.innermost_block()
145+
&& let Some(expr) = innermost_block.expr
146+
{
147+
lint_addr_of_local(cx, dcx, expr);
148+
}
149+
}
150+
}
151+
}
152+
}
153+
154+
struct DanglingPointerLocalContext<'tcx> {
155+
body: LocalDefId,
156+
fn_ret: Ty<'tcx>,
157+
fn_ret_span: Span,
158+
fn_ret_inner: Ty<'tcx>,
159+
fn_kind: &'static str,
160+
}
161+
162+
struct DanglingPointerReturnSearcher<'lcx, 'tcx> {
163+
cx: &'lcx LateContext<'tcx>,
164+
dcx: &'lcx DanglingPointerLocalContext<'tcx>,
165+
}
166+
167+
impl<'tcx> Visitor<'tcx> for DanglingPointerReturnSearcher<'_, 'tcx> {
168+
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> Self::Result {
169+
if let ExprKind::Ret(Some(expr)) = expr.kind {
170+
lint_addr_of_local(self.cx, self.dcx, expr);
171+
}
172+
walk_expr(self, expr)
173+
}
174+
}
175+
176+
/// Look for `&<path_to_local_in_same_body>` pattern and emit lint for it
177+
fn lint_addr_of_local<'a>(
178+
cx: &LateContext<'a>,
179+
dcx: &DanglingPointerLocalContext<'a>,
180+
expr: &'a Expr<'a>,
181+
) {
182+
// peel casts as they do not interest us here, we want the inner expression.
183+
let (inner, _) = super::utils::peel_casts(cx, expr);
184+
185+
if let ExprKind::AddrOf(_, _, inner_of) = inner.kind
186+
&& let ExprKind::Path(ref qpath) = inner_of.peel_blocks().kind
187+
&& let Res::Local(from) = cx.qpath_res(qpath, inner_of.hir_id)
188+
&& cx.tcx.hir_enclosing_body_owner(from) == dcx.body
189+
{
190+
cx.tcx.emit_node_span_lint(
191+
DANGLING_POINTERS_FROM_LOCALS,
192+
expr.hir_id,
193+
expr.span,
194+
DanglingPointersFromLocals {
195+
ret_ty: dcx.fn_ret,
196+
ret_ty_span: dcx.fn_ret_span,
197+
fn_kind: dcx.fn_kind,
198+
local_var: cx.tcx.hir_span(from),
199+
local_var_name: cx.tcx.hir_ident(from),
200+
local_var_ty: dcx.fn_ret_inner,
201+
created_at: (expr.hir_id != inner.hir_id).then_some(inner.span),
202+
},
203+
);
70204
}
71205
}
72206

compiler/rustc_lint/src/lints.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,6 +1188,22 @@ pub(crate) struct DanglingPointersFromTemporaries<'tcx> {
11881188
pub temporary_span: Span,
11891189
}
11901190

1191+
#[derive(LintDiagnostic)]
1192+
#[diag(lint_dangling_pointers_from_locals)]
1193+
#[note]
1194+
pub(crate) struct DanglingPointersFromLocals<'tcx> {
1195+
pub ret_ty: Ty<'tcx>,
1196+
#[label(lint_ret_ty)]
1197+
pub ret_ty_span: Span,
1198+
pub fn_kind: &'static str,
1199+
#[label(lint_local_var)]
1200+
pub local_var: Span,
1201+
pub local_var_name: Ident,
1202+
pub local_var_ty: Ty<'tcx>,
1203+
#[label(lint_created_at)]
1204+
pub created_at: Option<Span>,
1205+
}
1206+
11911207
// multiple_supertrait_upcastable.rs
11921208
#[derive(LintDiagnostic)]
11931209
#[diag(lint_multiple_supertrait_upcastable)]

src/tools/miri/tests/fail/validity/dangling_ref3.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
// Make sure we catch this even without Stacked Borrows
22
//@compile-flags: -Zmiri-disable-stacked-borrows
3+
4+
#![allow(dangling_pointers_from_locals)]
5+
36
use std::mem;
47

58
fn dangling() -> *const u8 {

0 commit comments

Comments
 (0)