Skip to content

Add lint against dangling pointers from local variables #144322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,12 @@ lint_confusable_identifier_pair = found both `{$existing_sym}` and `{$sym}` as i
lint_custom_inner_attribute_unstable = custom inner attributes are unstable
lint_dangling_pointers_from_locals = a dangling pointer will be produced because the local variable `{$local_var_name}` will be dropped
.ret_ty = return type of the {$fn_kind} is `{$ret_ty}`
.local_var = `{$local_var_name}` is part the {$fn_kind} and will be dropped at the end of the {$fn_kind}
.created_at = dangling pointer created here
.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
lint_dangling_pointers_from_temporaries = a dangling pointer will be produced because the temporary `{$ty}` will be dropped
.label_ptr = this pointer will immediately be invalid
.label_temporary = this `{$ty}` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
Expand Down
150 changes: 142 additions & 8 deletions compiler/rustc_lint/src/dangling.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use rustc_ast::visit::{visit_opt, walk_list};
use rustc_hir::attrs::AttributeKind;
use rustc_hir::def::Res;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr};
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, LangItem, find_attr};
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, FnRetTy, LangItem, TyKind, find_attr};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::{declare_lint, impl_lint_pass};
use rustc_span::{Span, sym};

use crate::lints::DanglingPointersFromTemporaries;
use crate::lints::{DanglingPointersFromLocals, DanglingPointersFromTemporaries};
use crate::{LateContext, LateLintPass};

declare_lint! {
Expand Down Expand Up @@ -42,6 +43,36 @@ declare_lint! {
"detects getting a pointer from a temporary"
}

declare_lint! {
/// 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`
/// }
/// ```
///
/// {{produces}}
///
/// ### 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.
/// This is not an error (as far as the type system is concerned)
/// but probably is not what the user intended either.
///
/// If you need stronger guarantees, consider using references instead,
/// as they are statically verified by the borrow-checker to never dangle.
pub DANGLING_POINTERS_FROM_LOCALS,
Warn,
"detects returning a pointer from a local variable"
}

/// FIXME: false negatives (i.e. the lint is not emitted when it should be)
/// 1. Ways to get a temporary that are not recognized:
/// - `owning_temporary.field`
Expand All @@ -53,20 +84,123 @@ declare_lint! {
#[derive(Clone, Copy, Default)]
pub(crate) struct DanglingPointers;

impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]);
impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES, DANGLING_POINTERS_FROM_LOCALS]);

// This skips over const blocks, but they cannot use or return a dangling pointer anyways.
impl<'tcx> LateLintPass<'tcx> for DanglingPointers {
fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
_: FnKind<'tcx>,
_: &'tcx FnDecl<'tcx>,
fn_kind: FnKind<'tcx>,
fn_decl: &'tcx FnDecl<'tcx>,
body: &'tcx Body<'tcx>,
_: Span,
_: LocalDefId,
def_id: LocalDefId,
) {
DanglingPointerSearcher { cx, inside_call_args: false }.visit_body(body)
DanglingPointerSearcher { cx, inside_call_args: false }.visit_body(body);

if let FnRetTy::Return(ret_ty) = &fn_decl.output
&& let TyKind::Ptr(_) = ret_ty.kind
{
// get the return type of the function or closure
let ty = match cx.tcx.type_of(def_id).instantiate_identity().kind() {
ty::FnDef(..) => cx.tcx.fn_sig(def_id).instantiate_identity(),
ty::Closure(_, args) => args.as_closure().sig(),
_ => return,
};
let ty = ty.output();

// this type is only used for layout computation and pretty-printing, neither of them rely on regions
let ty = cx.tcx.instantiate_bound_regions_with_erased(ty);

// verify that we have a pointer type
let inner_ty = match ty.kind() {
ty::RawPtr(inner_ty, _) => *inner_ty,
_ => return,
};
Comment on lines +116 to +120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit overprecise on just pointer return types and will not catch even tuples that contain raw pointers or generic Adts like Option. Ok as a first version, but I feel like it should be more general to be really useful.


if cx
.tcx
.layout_of(cx.typing_env().as_query_input(inner_ty))
.is_ok_and(|layout| !layout.is_1zst())
{
let dcx = &DanglingPointerLocalContext {
body: def_id,
fn_ret: ty,
fn_ret_span: ret_ty.span,
fn_ret_inner: inner_ty,
fn_kind: match fn_kind {
FnKind::ItemFn(..) => "function",
FnKind::Method(..) => "method",
FnKind::Closure => "closure",
},
};

// look for `return`s
DanglingPointerReturnSearcher { cx, dcx }.visit_body(body);

// analyze implicit return expression
if let ExprKind::Block(block, None) = &body.value.kind
&& let innermost_block = block.innermost_block()
&& let Some(expr) = innermost_block.expr
{
lint_addr_of_local(cx, dcx, expr);
}
}
}
}
}

struct DanglingPointerLocalContext<'tcx> {
body: LocalDefId,
fn_ret: Ty<'tcx>,
fn_ret_span: Span,
fn_ret_inner: Ty<'tcx>,
fn_kind: &'static str,
}

struct DanglingPointerReturnSearcher<'lcx, 'tcx> {
cx: &'lcx LateContext<'tcx>,
dcx: &'lcx DanglingPointerLocalContext<'tcx>,
}

impl<'tcx> Visitor<'tcx> for DanglingPointerReturnSearcher<'_, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> Self::Result {
if let ExprKind::Ret(Some(expr)) = expr.kind {
lint_addr_of_local(self.cx, self.dcx, expr);
}
walk_expr(self, expr)
}
}

/// Look for `&<path_to_local_in_same_body>` pattern and emit lint for it
fn lint_addr_of_local<'a>(
cx: &LateContext<'a>,
dcx: &DanglingPointerLocalContext<'a>,
expr: &'a Expr<'a>,
) {
// peel casts as they do not interest us here, we want the inner expression.
let (inner, _) = super::utils::peel_casts(cx, expr);

if let ExprKind::AddrOf(_, _, inner_of) = inner.kind
&& let ExprKind::Path(ref qpath) = inner_of.peel_blocks().kind
&& let Res::Local(from) = cx.qpath_res(qpath, inner_of.hir_id)
&& cx.tcx.hir_enclosing_body_owner(from) == dcx.body
{
cx.tcx.emit_node_span_lint(
DANGLING_POINTERS_FROM_LOCALS,
expr.hir_id,
expr.span,
DanglingPointersFromLocals {
ret_ty: dcx.fn_ret,
ret_ty_span: dcx.fn_ret_span,
fn_kind: dcx.fn_kind,
local_var: cx.tcx.hir_span(from),
local_var_name: cx.tcx.hir_ident(from),
local_var_ty: dcx.fn_ret_inner,
created_at: (expr.hir_id != inner.hir_id).then_some(inner.span),
},
);
}
}

Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,22 @@ pub(crate) struct DanglingPointersFromTemporaries<'tcx> {
pub temporary_span: Span,
}

#[derive(LintDiagnostic)]
#[diag(lint_dangling_pointers_from_locals)]
#[note]
pub(crate) struct DanglingPointersFromLocals<'tcx> {
pub ret_ty: Ty<'tcx>,
#[label(lint_ret_ty)]
pub ret_ty_span: Span,
pub fn_kind: &'static str,
#[label(lint_local_var)]
pub local_var: Span,
pub local_var_name: Ident,
pub local_var_ty: Ty<'tcx>,
#[label(lint_created_at)]
pub created_at: Option<Span>,
}

// multiple_supertrait_upcastable.rs
#[derive(LintDiagnostic)]
#[diag(lint_multiple_supertrait_upcastable)]
Expand Down
3 changes: 3 additions & 0 deletions src/tools/miri/tests/fail/validity/dangling_ref3.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// Make sure we catch this even without Stacked Borrows
//@compile-flags: -Zmiri-disable-stacked-borrows

#![allow(dangling_pointers_from_locals)]

use std::mem;

fn dangling() -> *const u8 {
Expand Down
Loading
Loading