Skip to content

Commit

Permalink
Rollup merge of #132732 - gavincrawford:as_ptr_attribute, r=Urgau
Browse files Browse the repository at this point in the history
Use attributes for `dangling_pointers_from_temporaries` lint

Checking for dangling pointers by function name isn't ideal, and leaves out certain pointer-returning methods that don't follow the `as_ptr` naming convention. Using an attribute for this lint cleans things up and allows more thorough coverage of other methods, such as `UnsafeCell::get()`.
  • Loading branch information
jhpratt authored Nov 20, 2024
2 parents bcfea1f + 01fd384 commit 25dc4d0
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 32 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
lang, Normal, template!(NameValueStr: "name"), DuplicatesOk, EncodeCrossCrate::No, lang_items,
"lang items are subject to change",
),
rustc_attr!(
rustc_as_ptr, Normal, template!(Word), ErrorFollowing,
EncodeCrossCrate::Yes,
"#[rustc_as_ptr] is used to mark functions returning pointers to their inner allocations."
),
rustc_attr!(
rustc_pass_by_value, Normal, template!(Word), ErrorFollowing,
EncodeCrossCrate::Yes,
Expand Down
27 changes: 13 additions & 14 deletions compiler/rustc_lint/src/dangling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,10 @@ declare_lint! {
}

/// FIXME: false negatives (i.e. the lint is not emitted when it should be)
/// 1. Method calls that are not checked for:
/// - [`temporary_unsafe_cell.get()`][`core::cell::UnsafeCell::get()`]
/// - [`temporary_sync_unsafe_cell.get()`][`core::cell::SyncUnsafeCell::get()`]
/// 2. Ways to get a temporary that are not recognized:
/// 1. Ways to get a temporary that are not recognized:
/// - `owning_temporary.field`
/// - `owning_temporary[index]`
/// 3. No checks for ref-to-ptr conversions:
/// 2. No checks for ref-to-ptr conversions:
/// - `&raw [mut] temporary`
/// - `&temporary as *(const|mut) _`
/// - `ptr::from_ref(&temporary)` and friends
Expand Down Expand Up @@ -133,10 +130,11 @@ impl DanglingPointerSearcher<'_, '_> {

fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::MethodCall(method, receiver, _args, _span) = expr.kind
&& matches!(method.ident.name, sym::as_ptr | sym::as_mut_ptr)
&& let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
&& cx.tcx.has_attr(fn_id, sym::rustc_as_ptr)
&& is_temporary_rvalue(receiver)
&& let ty = cx.typeck_results().expr_ty(receiver)
&& is_interesting(cx.tcx, ty)
&& owns_allocation(cx.tcx, ty)
{
// FIXME: use `emit_node_lint` when `#[primary_span]` is added.
cx.tcx.emit_node_span_lint(
Expand Down Expand Up @@ -199,24 +197,25 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool {
}
}

// Array, Vec, String, CString, MaybeUninit, Cell, Box<[_]>, Box<str>, Box<CStr>,
// or any of the above in arbitrary many nested Box'es.
fn is_interesting(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool {
// 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))
|| is_interesting(tcx, inner)
|| owns_allocation(tcx, inner)
} else if let Some(def) = ty.ty_adt_def() {
for lang_item in [LangItem::String, LangItem::MaybeUninit] {
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))
tcx.get_diagnostic_name(def.did()).is_some_and(|name| {
matches!(name, sym::cstring_type | sym::Vec | sym::Cell | sym::SyncUnsafeCell)
})
} else {
false
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.check_rustc_std_internal_symbol(attr, span, target)
}
[sym::naked, ..] => self.check_naked(hir_id, attr, span, target, attrs),
[sym::rustc_as_ptr, ..] => {
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}
[sym::rustc_never_returns_null_ptr, ..] => {
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ symbols! {
SubdiagMessage,
Subdiagnostic,
Sync,
SyncUnsafeCell,
T,
Target,
ToOwned,
Expand Down Expand Up @@ -409,7 +410,6 @@ symbols! {
arm,
arm_target_feature,
array,
as_mut_ptr,
as_ptr,
as_ref,
as_str,
Expand Down Expand Up @@ -1655,6 +1655,7 @@ symbols! {
rustc_allow_const_fn_unstable,
rustc_allow_incoherent_impl,
rustc_allowed_through_unstable_modules,
rustc_as_ptr,
rustc_attrs,
rustc_autodiff,
rustc_box,
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1500,6 +1500,7 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
/// [`as_ptr`]: Self::as_ptr
#[unstable(feature = "box_as_ptr", issue = "129090")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline]
pub fn as_mut_ptr(b: &mut Self) -> *mut T {
// This is a primitive deref, not going through `DerefMut`, and therefore not materializing
Expand Down Expand Up @@ -1548,6 +1549,7 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
/// [`as_ptr`]: Self::as_ptr
#[unstable(feature = "box_as_ptr", issue = "129090")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline]
pub fn as_ptr(b: &Self) -> *const T {
// This is a primitive deref, not going through `DerefMut`, and therefore not materializing
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1662,6 +1662,7 @@ impl<T, A: Allocator> Vec<T, A> {
#[stable(feature = "vec_as_ptr", since = "1.37.0")]
#[rustc_const_unstable(feature = "const_vec_string_slice", issue = "129041")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline]
pub const fn as_ptr(&self) -> *const T {
// We shadow the slice method of the same name to avoid going through
Expand Down Expand Up @@ -1724,6 +1725,7 @@ impl<T, A: Allocator> Vec<T, A> {
#[stable(feature = "vec_as_ptr", since = "1.37.0")]
#[rustc_const_unstable(feature = "const_vec_string_slice", issue = "129041")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline]
pub const fn as_mut_ptr(&mut self) -> *mut T {
// We shadow the slice method of the same name to avoid going through
Expand Down
5 changes: 5 additions & 0 deletions library/core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ impl<T: ?Sized> Cell<T> {
#[inline]
#[stable(feature = "cell_as_ptr", since = "1.12.0")]
#[rustc_const_stable(feature = "const_cell_as_ptr", since = "1.32.0")]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[rustc_never_returns_null_ptr]
pub const fn as_ptr(&self) -> *mut T {
self.value.get()
Expand Down Expand Up @@ -1149,6 +1150,7 @@ impl<T: ?Sized> RefCell<T> {
/// ```
#[inline]
#[stable(feature = "cell_as_ptr", since = "1.12.0")]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[rustc_never_returns_null_ptr]
pub fn as_ptr(&self) -> *mut T {
self.value.get()
Expand Down Expand Up @@ -2158,6 +2160,7 @@ impl<T: ?Sized> UnsafeCell<T> {
#[inline(always)]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_unsafecell_get", since = "1.32.0")]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[rustc_never_returns_null_ptr]
pub const fn get(&self) -> *mut T {
// We can just cast the pointer from `UnsafeCell<T>` to `T` because of
Expand Down Expand Up @@ -2271,6 +2274,7 @@ impl<T: DispatchFromDyn<U>, U> DispatchFromDyn<UnsafeCell<U>> for UnsafeCell<T>
/// See [`UnsafeCell`] for details.
#[unstable(feature = "sync_unsafe_cell", issue = "95439")]
#[repr(transparent)]
#[rustc_diagnostic_item = "SyncUnsafeCell"]
#[rustc_pub_transparent]
pub struct SyncUnsafeCell<T: ?Sized> {
value: UnsafeCell<T>,
Expand Down Expand Up @@ -2304,6 +2308,7 @@ impl<T: ?Sized> SyncUnsafeCell<T> {
/// when casting to `&mut T`, and ensure that there are no mutations
/// or mutable aliases going on when casting to `&T`
#[inline]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[rustc_never_returns_null_ptr]
pub const fn get(&self) -> *mut T {
self.value.get()
Expand Down
1 change: 1 addition & 0 deletions library/core/src/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ impl CStr {
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_str_as_ptr", since = "1.32.0")]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[rustc_never_returns_null_ptr]
pub const fn as_ptr(&self) -> *const c_char {
self.inner.as_ptr()
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/mem/maybe_uninit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ impl<T> MaybeUninit<T> {
/// until they are, it is advisable to avoid them.)
#[stable(feature = "maybe_uninit", since = "1.36.0")]
#[rustc_const_stable(feature = "const_maybe_uninit_as_ptr", since = "1.59.0")]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline(always)]
pub const fn as_ptr(&self) -> *const T {
// `MaybeUninit` and `ManuallyDrop` are both `repr(transparent)` so we can cast the pointer.
Expand Down Expand Up @@ -566,6 +567,7 @@ impl<T> MaybeUninit<T> {
/// until they are, it is advisable to avoid them.)
#[stable(feature = "maybe_uninit", since = "1.36.0")]
#[rustc_const_stable(feature = "const_maybe_uninit_as_mut_ptr", since = "1.83.0")]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline(always)]
pub const fn as_mut_ptr(&mut self) -> *mut T {
// `MaybeUninit` and `ManuallyDrop` are both `repr(transparent)` so we can cast the pointer.
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,7 @@ impl<T> [T] {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_slice_as_ptr", since = "1.32.0")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline(always)]
#[must_use]
pub const fn as_ptr(&self) -> *const T {
Expand Down Expand Up @@ -765,6 +766,7 @@ impl<T> [T] {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[inline(always)]
#[must_use]
pub const fn as_mut_ptr(&mut self) -> *mut T {
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/str/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ impl str {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "rustc_str_as_ptr", since = "1.32.0")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[must_use]
#[inline(always)]
pub const fn as_ptr(&self) -> *const u8 {
Expand All @@ -390,6 +391,7 @@ impl str {
#[stable(feature = "str_as_mut_ptr", since = "1.36.0")]
#[rustc_const_stable(feature = "const_str_as_mut", since = "1.83.0")]
#[rustc_never_returns_null_ptr]
#[cfg_attr(not(bootstrap), rustc_as_ptr)]
#[must_use]
#[inline(always)]
pub const fn as_mut_ptr(&mut self) -> *mut u8 {
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/lint/dangling-pointers-from-temporaries/types.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![deny(dangling_pointers_from_temporaries)]
#![feature(sync_unsafe_cell)]

use std::cell::Cell;
use std::cell::{Cell, SyncUnsafeCell, UnsafeCell};
use std::ffi::{CStr, CString};
use std::mem::MaybeUninit;

Expand Down Expand Up @@ -47,6 +48,10 @@ fn main() {
//~^ ERROR a dangling pointer will be produced because the temporary `MaybeUninit<u8>` will be dropped
declval::<Vec<AsPtrFake>>().as_ptr();
//~^ ERROR a dangling pointer will be produced because the temporary `Vec<AsPtrFake>` will be dropped
declval::<UnsafeCell<u8>>().get();
//~^ 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::<AsPtrFake>().as_ptr();
}
Loading

0 comments on commit 25dc4d0

Please sign in to comment.