Skip to content

Commit

Permalink
Auto merge of rust-lang#111807 - erikdesjardins:noalias, r=oli-obk
Browse files Browse the repository at this point in the history
[rustc_ty_utils] Treat `drop_in_place`'s *mut argument like &mut when adding LLVM attributes

This resurrects PR rust-lang#103614, which has sat idle for a while.

This could probably use a new perf run, since we're on a new LLVM version now.

r? `@oli-obk`
cc `@RalfJung`

---

LLVM can make use of the `noalias` parameter attribute on the parameter to `drop_in_place` in areas like argument promotion. Because the Rust compiler fully controls the code for `drop_in_place`, it can soundly deduce parameter attributes on it.

In rust-lang#103957, Miri was changed to retag `drop_in_place`'s argument as if it was `&mut`, matching this change.
  • Loading branch information
bors committed May 23, 2023
2 parents cda5bec + fb7f1d2 commit f3d597b
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 10 deletions.
35 changes: 31 additions & 4 deletions compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ fn adjust_for_rust_scalar<'tcx>(
layout: TyAndLayout<'tcx>,
offset: Size,
is_return: bool,
drop_target_pointee: Option<Ty<'tcx>>,
) {
// Booleans are always a noundef i1 that needs to be zero-extended.
if scalar.is_bool() {
Expand All @@ -251,14 +252,24 @@ fn adjust_for_rust_scalar<'tcx>(
}

// Only pointer types handled below.
let Scalar::Initialized { value: Pointer(_), valid_range} = scalar else { return };
let Scalar::Initialized { value: Pointer(_), valid_range } = scalar else { return };

if !valid_range.contains(0) {
// Set `nonnull` if the validity range excludes zero, or for the argument to `drop_in_place`,
// which must be nonnull per its documented safety requirements.
if !valid_range.contains(0) || drop_target_pointee.is_some() {
attrs.set(ArgAttribute::NonNull);
}

if let Some(pointee) = layout.pointee_info_at(&cx, offset) {
if let Some(kind) = pointee.safe {
let kind = if let Some(kind) = pointee.safe {
Some(kind)
} else if let Some(pointee) = drop_target_pointee {
// The argument to `drop_in_place` is semantically equivalent to a mutable reference.
Some(PointerKind::MutableRef { unpin: pointee.is_unpin(cx.tcx, cx.param_env()) })
} else {
None
};
if let Some(kind) = kind {
attrs.pointee_align = Some(pointee.align);

// `Box` are not necessarily dereferenceable for the entire duration of the function as
Expand Down Expand Up @@ -362,10 +373,18 @@ fn fn_abi_new_uncached<'tcx>(
use SpecAbi::*;
let rust_abi = matches!(sig.abi, RustIntrinsic | PlatformIntrinsic | Rust | RustCall);

let is_drop_in_place =
fn_def_id.is_some() && fn_def_id == cx.tcx.lang_items().drop_in_place_fn();

let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| -> Result<_, FnAbiError<'tcx>> {
let span = tracing::debug_span!("arg_of");
let _entered = span.enter();
let is_return = arg_idx.is_none();
let is_drop_target = is_drop_in_place && arg_idx == Some(0);
let drop_target_pointee = is_drop_target.then(|| match ty.kind() {
ty::RawPtr(ty::TypeAndMut { ty, .. }) => *ty,
_ => bug!("argument to drop_in_place is not a raw ptr: {:?}", ty),
});

let layout = cx.layout_of(ty)?;
let layout = if force_thin_self_ptr && arg_idx == Some(0) {
Expand All @@ -379,7 +398,15 @@ fn fn_abi_new_uncached<'tcx>(

let mut arg = ArgAbi::new(cx, layout, |layout, scalar, offset| {
let mut attrs = ArgAttributes::new();
adjust_for_rust_scalar(*cx, &mut attrs, scalar, *layout, offset, is_return);
adjust_for_rust_scalar(
*cx,
&mut attrs,
scalar,
*layout,
offset,
is_return,
drop_target_pointee,
);
attrs
});

Expand Down
16 changes: 11 additions & 5 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,19 +441,25 @@ mod mut_ptr;
///
/// * `to_drop` must be [valid] for both reads and writes.
///
/// * `to_drop` must be properly aligned.
/// * `to_drop` must be properly aligned, even if `T` has size 0.
///
/// * The value `to_drop` points to must be valid for dropping, which may mean it must uphold
/// additional invariants - this is type-dependent.
/// * `to_drop` must be nonnull, even if `T` has size 0.
///
/// * The value `to_drop` points to must be valid for dropping, which may mean
/// it must uphold additional invariants. These invariants depend on the type
/// of the value being dropped. For instance, when dropping a Box, the box's
/// pointer to the heap must be valid.
///
/// * While `drop_in_place` is executing, the only way to access parts of
/// `to_drop` is through the `&mut self` references supplied to the
/// `Drop::drop` methods that `drop_in_place` invokes.
///
/// Additionally, if `T` is not [`Copy`], using the pointed-to value after
/// calling `drop_in_place` can cause undefined behavior. Note that `*to_drop =
/// foo` counts as a use because it will cause the value to be dropped
/// again. [`write()`] can be used to overwrite data without causing it to be
/// dropped.
///
/// Note that even if `T` has size `0`, the pointer must be non-null and properly aligned.
///
/// [valid]: self#safety
///
/// # Examples
Expand Down
38 changes: 38 additions & 0 deletions tests/codegen/drop-in-place-noalias.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// compile-flags: -O -C no-prepopulate-passes

// Tests that the compiler can apply `noalias` and other &mut attributes to `drop_in_place`.
// Note that non-Unpin types should not get `noalias`, matching &mut behavior.

#![crate_type="lib"]

use std::marker::PhantomPinned;

// CHECK: define internal void @{{.*}}core{{.*}}ptr{{.*}}drop_in_place{{.*}}StructUnpin{{.*}}({{.*\*|ptr}} noalias noundef align 4 dereferenceable(12) %{{.+}})

// CHECK: define internal void @{{.*}}core{{.*}}ptr{{.*}}drop_in_place{{.*}}StructNotUnpin{{.*}}({{.*\*|ptr}} noundef nonnull align 4 %{{.+}})

pub struct StructUnpin {
a: i32,
b: i32,
c: i32,
}

impl Drop for StructUnpin {
fn drop(&mut self) {}
}

pub struct StructNotUnpin {
a: i32,
b: i32,
c: i32,
p: PhantomPinned,
}

impl Drop for StructNotUnpin {
fn drop(&mut self) {}
}

pub unsafe fn main(x: StructUnpin, y: StructNotUnpin) {
drop(x);
drop(y);
}
5 changes: 4 additions & 1 deletion tests/codegen/noalias-box-off.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@

// CHECK-LABEL: @box_should_not_have_noalias_if_disabled(
// CHECK-NOT: noalias
// CHECK-SAME: %foo)
#[no_mangle]
pub fn box_should_not_have_noalias_if_disabled(_b: Box<u8>) {}
pub fn box_should_not_have_noalias_if_disabled(foo: Box<u8>) {
drop(foo);
}

0 comments on commit f3d597b

Please sign in to comment.