about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-05-23 10:12:46 +0000
committerbors <bors@rust-lang.org>2023-05-23 10:12:46 +0000
commitf3d597b31c0f101a02c230798afa31a36bdacbc6 (patch)
treee1155c3755cf98eb4c0a85607099171616930ae8
parentcda5becc27cbc7106646fbc40aacea5e7896d954 (diff)
parentfb7f1d220c28dd86000d52f846ceb9055ae0ace4 (diff)
downloadrust-f3d597b31c0f101a02c230798afa31a36bdacbc6.tar.gz
rust-f3d597b31c0f101a02c230798afa31a36bdacbc6.zip
Auto merge of #111807 - erikdesjardins:noalias, r=oli-obk
[rustc_ty_utils] Treat `drop_in_place`'s *mut argument like &mut when adding LLVM attributes

This resurrects PR #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 #103957, Miri was changed to retag `drop_in_place`'s argument as if it was `&mut`, matching this change.
-rw-r--r--compiler/rustc_ty_utils/src/abi.rs35
-rw-r--r--library/core/src/ptr/mod.rs16
-rw-r--r--tests/codegen/drop-in-place-noalias.rs38
-rw-r--r--tests/codegen/noalias-box-off.rs5
4 files changed, 84 insertions, 10 deletions
diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs
index 442d041a8a7..15c19104616 100644
--- a/compiler/rustc_ty_utils/src/abi.rs
+++ b/compiler/rustc_ty_utils/src/abi.rs
@@ -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() {
@@ -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
@@ -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) {
@@ -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
         });
 
diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs
index ecbf4e66fa4..ff9fa48f311 100644
--- a/library/core/src/ptr/mod.rs
+++ b/library/core/src/ptr/mod.rs
@@ -441,10 +441,18 @@ 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 =
@@ -452,8 +460,6 @@ mod mut_ptr;
 /// 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
diff --git a/tests/codegen/drop-in-place-noalias.rs b/tests/codegen/drop-in-place-noalias.rs
new file mode 100644
index 00000000000..725e6fc048d
--- /dev/null
+++ b/tests/codegen/drop-in-place-noalias.rs
@@ -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);
+}
diff --git a/tests/codegen/noalias-box-off.rs b/tests/codegen/noalias-box-off.rs
index afd17c7c160..c82c53b2a48 100644
--- a/tests/codegen/noalias-box-off.rs
+++ b/tests/codegen/noalias-box-off.rs
@@ -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);
+}