Skip to content

Commit

Permalink
Refactor inner allocation logic of temp dandling pointer lint
Browse files Browse the repository at this point in the history
  • Loading branch information
Urgau committed Nov 20, 2024
1 parent bfe809d commit 00937d7
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 29 deletions.
66 changes: 39 additions & 27 deletions compiler/rustc_lint/src/dangling.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use rustc_ast::visit::{visit_opt, walk_list};
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr};
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, LangItem};
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl};
use rustc_middle::ty::adjustment::{Adjust, Adjustment};
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint, impl_lint_pass};
use rustc_span::Span;
use rustc_span::symbol::sym;
Expand Down Expand Up @@ -134,7 +135,8 @@ fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) {
&& cx.tcx.has_attr(fn_id, sym::rustc_as_ptr)
&& is_temporary_rvalue(receiver)
&& let ty = cx.typeck_results().expr_ty(receiver)
&& owns_allocation(cx.tcx, ty)
&& let adjs = cx.typeck_results().expr_adjustments(receiver)
&& is_inner_allocation_owned(ty, adjs)
{
// FIXME: use `emit_node_lint` when `#[primary_span]` is added.
cx.tcx.emit_node_span_lint(
Expand All @@ -151,6 +153,40 @@ fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) {
}
}

/// Determine if a `#[rustc_as_ptr]` receiver type (with adjustments) is only applied
/// to owned allocations.
fn is_inner_allocation_owned(input_ty: Ty<'_>, adjs: &[Adjustment<'_>]) -> bool {
// 1. Look at all the deref-ed types and exclude any pointer types
//
// - `Box<u8>` -> `u8`: OK
// - `Box<&u8>` -> `&u8`: KO
// - `Box<Box<u8>>` -> `Box<u8>` -> `u8`: OK
// - `MaybeUninit<MaybeUninit<&u8>>`: OK
// - `Box<MaybeUninit<&u8>>` -> `MaybeUninit<&u8>`: OK
adjs.iter()
.filter_map(|adj| match adj.kind {
Adjust::Deref(_) => Some(adj.target),
Adjust::NeverToAny
| Adjust::Borrow(_)
| Adjust::Pointer(_)
| Adjust::ReborrowPin(_) => None,
})
.all(|deref_ty| !deref_ty.is_any_ptr())
// 2. Look at the top level generics at exclude any pointer types
// (mainly for types that don't deref)
//
// - `MaybeUninit<u8>` -> `u8`: OK
// - `MaybeUninit<&u8>` -> `&u8`: KO
// - `MaybeUninit<MaybeUninit<&u8>>` -> `MaybeUninit<&u8>`: OK
// - `Box<MaybeUninit<&u8>>` -> `MaybeUninit<&u8>`: OK
&& match input_ty.kind() {
ty::Adt(_def, args) => {
args.iter().filter_map(|g| g.as_type()).all(|ty| !ty.is_any_ptr())
}
_ => !input_ty.is_any_ptr(),
}
}

fn is_temporary_rvalue(expr: &Expr<'_>) -> bool {
match expr.kind {
// Const is not temporary.
Expand Down Expand Up @@ -196,27 +232,3 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool {
ExprKind::Type(..) | ExprKind::Err(..) => false,
}
}

// Array, Vec, String, CString, MaybeUninit, Cell, Box<[_]>, Box<str>, Box<CStr>, UnsafeCell,
// SyncUnsafeCell, or any of the above in arbitrary many nested Box'es.
fn owns_allocation(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool {
if ty.is_array() {
true
} else if let Some(inner) = ty.boxed_ty() {
inner.is_slice()
|| inner.is_str()
|| inner.ty_adt_def().is_some_and(|def| tcx.is_lang_item(def.did(), LangItem::CStr))
|| owns_allocation(tcx, inner)
} else if let Some(def) = ty.ty_adt_def() {
for lang_item in [LangItem::String, LangItem::MaybeUninit, LangItem::UnsafeCell] {
if tcx.is_lang_item(def.did(), lang_item) {
return true;
}
}
tcx.get_diagnostic_name(def.did()).is_some_and(|name| {
matches!(name, sym::cstring_type | sym::Vec | sym::Cell | sym::SyncUnsafeCell)
})
} else {
false
}
}
10 changes: 9 additions & 1 deletion tests/ui/lint/dangling-pointers-from-temporaries/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ fn main() {
//~^ ERROR a dangling pointer will be produced because the temporary `UnsafeCell<u8>` will be dropped
declval::<SyncUnsafeCell<u8>>().get();
//~^ ERROR a dangling pointer will be produced because the temporary `SyncUnsafeCell<u8>` will be dropped
declval::<Box<AsPtrFake>>().as_ptr();
declval::<MaybeUninit<MaybeUninit<&u8>>>().as_ptr();
//~^ ERROR a dangling pointer will be produced because the temporary `MaybeUninit<MaybeUninit<&u8>>` will be dropped
declval::<Box<MaybeUninit<&u8>>>().as_ptr();
//~^ ERROR a dangling pointer will be produced because the temporary `Box<MaybeUninit<&u8>>` will be dropped

// should not lint
declval::<&[u8]>().as_ptr();
declval::<AsPtrFake>().as_ptr();
declval::<Box<AsPtrFake>>().as_ptr();
declval::<MaybeUninit<&u8>>().as_ptr();
}
24 changes: 23 additions & 1 deletion tests/ui/lint/dangling-pointers-from-temporaries/types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -190,5 +190,27 @@ LL | declval::<SyncUnsafeCell<u8>>().get();
= note: pointers do not have a lifetime; when calling `get` the `SyncUnsafeCell<u8>` 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 17 previous errors
error: a dangling pointer will be produced because the temporary `MaybeUninit<MaybeUninit<&u8>>` will be dropped
--> $DIR/types.rs:55:48
|
LL | declval::<MaybeUninit<MaybeUninit<&u8>>>().as_ptr();
| ------------------------------------------ ^^^^^^ this pointer will immediately be invalid
| |
| this `MaybeUninit<MaybeUninit<&u8>>` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
|
= note: pointers do not have a lifetime; when calling `as_ptr` the `MaybeUninit<MaybeUninit<&u8>>` 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: a dangling pointer will be produced because the temporary `Box<MaybeUninit<&u8>>` will be dropped
--> $DIR/types.rs:57:40
|
LL | declval::<Box<MaybeUninit<&u8>>>().as_ptr();
| ---------------------------------- ^^^^^^ this pointer will immediately be invalid
| |
| this `Box<MaybeUninit<&u8>>` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
|
= note: pointers do not have a lifetime; when calling `as_ptr` the `Box<MaybeUninit<&u8>>` 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 19 previous errors

0 comments on commit 00937d7

Please sign in to comment.