Emit nofree attribute

compiler/rustc_codegen_llvm/src/abi.rs

@@ -38,12 +38,15 @@ trait ArgAttributesExt {
 const ABI_AFFECTING_ATTRIBUTES: [(ArgAttribute, llvm::AttributeKind); 1] =
     [(ArgAttribute::InReg, llvm::AttributeKind::InReg)];
 
-const OPTIMIZATION_ATTRIBUTES: [(ArgAttribute, llvm::AttributeKind); 5] = [
+const OPTIMIZATION_ATTRIBUTES: [(ArgAttribute, llvm::AttributeKind); 6] = [
     (ArgAttribute::NoAlias, llvm::AttributeKind::NoAlias),
     (ArgAttribute::NonNull, llvm::AttributeKind::NonNull),
     (ArgAttribute::ReadOnly, llvm::AttributeKind::ReadOnly),
     (ArgAttribute::NoUndef, llvm::AttributeKind::NoUndef),
     (ArgAttribute::Writable, llvm::AttributeKind::Writable),
+    // We can map our internal NoFree attribute (which forbids freeing the underlying allocation)
+    // to LLVM's weaker attribute (which forbids freeing it through this specific pointer).
+    (ArgAttribute::NoFree, llvm::AttributeKind::NoFree),
 ];
 
 const CAPTURES_ATTRIBUTES: [(ArgAttribute, llvm::AttributeKind); 3] = [
@@ -75,7 +78,9 @@ fn get_attrs<'ll>(this: &ArgAttributes, cx: &CodegenCx<'ll, '_>) -> SmallVec<[&'
     // Only apply remaining attributes when optimizing
     if cx.sess().opts.optimize != config::OptLevel::No {
         let deref = this.pointee_size.bytes();
-        if deref != 0 {
+        // dereferenceable in LLVM currently implies nofree, so only emit dereferenceable if nofree
+        // is also set.
+        if deref != 0 && regular.contains(ArgAttribute::NoFree) {
             if regular.contains(ArgAttribute::NonNull) {
                 attrs.push(llvm::CreateDereferenceableAttr(cx.llcx, deref));
             } else {

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -294,6 +294,7 @@ pub(crate) enum AttributeKind {
     SanitizeRealtimeNonblocking = 47,
     SanitizeRealtimeBlocking = 48,
     Convergent = 49,
+    NoFree = 50,
 }
 
 /// LLVMIntPredicate

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

@@ -383,6 +383,7 @@ enum class LLVMRustAttributeKind {
   SanitizeRealtimeNonblocking = 47,
   SanitizeRealtimeBlocking = 48,
   Convergent = 49,
+  NoFree = 50,
 };
 
 static Attribute::AttrKind fromRust(LLVMRustAttributeKind Kind) {
@@ -481,6 +482,8 @@ static Attribute::AttrKind fromRust(LLVMRustAttributeKind Kind) {
     return Attribute::SanitizeRealtimeBlocking;
   case LLVMRustAttributeKind::Convergent:
     return Attribute::Convergent;
+  case LLVMRustAttributeKind::NoFree:
+    return Attribute::NoFree;
   }
   report_fatal_error("bad LLVMRustAttributeKind");
 }

compiler/rustc_middle/src/ty/layout.rs

@@ -979,33 +979,19 @@ where
             }
             ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
                 tcx.layout_of(typing_env.as_query_input(ty)).ok().map(|layout| {
-                    let (size, kind);
-                    match mt {
+                    let kind = match mt {
                         hir::Mutability::Not => {
                             let frozen = optimize && ty.is_freeze(tcx, typing_env);
-
-                            // Non-frozen shared references are not necessarily dereferenceable for the entire duration of the function
-                            // (see <https://github.com/rust-lang/rust/pull/98017>)
-                            // (if we had "dereferenceable on entry", we could support this)
-                            size = if frozen { layout.size } else { Size::ZERO };
-
-                            kind = PointerKind::SharedRef { frozen };
+                            PointerKind::SharedRef { frozen }
                         }
                         hir::Mutability::Mut => {
                             let unpin = optimize
                                 && ty.is_unpin(tcx, typing_env)
                                 && ty.is_unsafe_unpin(tcx, typing_env);
-
-                            // Mutable references to potentially self-referential types are not
-                            // necessarily dereferenceable for the entire duration of the function
-                            // (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>)
-                            // (if we had "dereferenceable on entry", we could support this)
-                            size = if unpin { layout.size } else { Size::ZERO };
-
-                            kind = PointerKind::MutableRef { unpin };
+                            PointerKind::MutableRef { unpin }
                         }
                     };
-                    PointeeInfo { safe: Some(kind), size, align: layout.align.abi }
+                    PointeeInfo { safe: Some(kind), size: layout.size, align: layout.align.abi }
                 })
             }
 
@@ -1021,12 +1007,7 @@ where
                             && pointee.is_unsafe_unpin(tcx, typing_env),
                         global: this.ty.is_box_global(tcx),
                     }),
-
-                    // `Box` are not necessarily dereferenceable for the entire duration of the function as
-                    // they can be deallocated at any time.
-                    // (if we had "dereferenceable on entry", we could support this)
-                    size: Size::ZERO,
-
+                    size: layout.size,
                     align: layout.align.abi,
                 })
             }

compiler/rustc_target/src/callconv/mod.rs

@@ -122,6 +122,10 @@ mod attr_impl {
             const InReg    = 1 << 6;
             const NoUndef  = 1 << 7;
             const Writable = 1 << 8;
+            /// It is UB for the allocation that this pointer points to to be freed
+            /// while the function is executing. Only valid on arguments (including
+            /// return values that are passed indirectly as arguments).
+            const NoFree   = 1 << 9;
         }
     }
     rustc_data_structures::external_bitflags_debug! { ArgAttribute }
@@ -143,9 +147,8 @@ pub enum ArgExtension {
 pub struct ArgAttributes {
     pub regular: ArgAttribute,
     pub arg_ext: ArgExtension,
-    /// The minimum size of the pointee, guaranteed to be valid for the duration of the whole call
-    /// (corresponding to LLVM's dereferenceable_or_null attributes, i.e., it is okay for this to be
-    /// set on a null pointer, but all non-null pointers must be dereferenceable).
+    /// If the pointer is not null, the minimum dereferenceable size of the pointee, at the time of
+    /// function entry (for arguments) or function return (for return values).
     pub pointee_size: Size,
     /// The minimum alignment of the pointee, if any.
     pub pointee_align: Option<Align>,
@@ -408,7 +411,8 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
             .set(ArgAttribute::NoAlias)
             .set(ArgAttribute::CapturesAddress)
             .set(ArgAttribute::NonNull)
-            .set(ArgAttribute::NoUndef);
+            .set(ArgAttribute::NoUndef)
+            .set(ArgAttribute::NoFree);
         attrs.pointee_size = layout.size;
         attrs.pointee_align = Some(layout.align.abi);
 

compiler/rustc_ty_utils/src/abi.rs

@@ -356,13 +356,7 @@ fn arg_attrs_for_rust_scalar<'tcx>(
                 Some(pointee.align.min(cx.tcx().sess.target.max_reliable_alignment()));
         }
 
-        // LLVM dereferenceable attribute has unclear semantics on the return type,
-        // they seem to be "dereferenceable until the end of the program", which is
-        // generally, not valid for references. See
-        // <https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/LLVM.20dereferenceable.20on.20return.20type/with/563001493>
-        if !is_return {
-            attrs.pointee_size = pointee.size;
-        };
+        attrs.pointee_size = pointee.size;
 
         if let Some(kind) = pointee.safe {
             // The aliasing rules for `Box<T>` are still not decided, but currently we emit
@@ -407,6 +401,26 @@ fn arg_attrs_for_rust_scalar<'tcx>(
                 }
             }
 
+            // NoFree is not valid on return values. If it were, it would mean something like
+            // "will not be freed until the end of the program", which is generally not valid for
+            // references.
+            let no_free = !is_return
+                && match kind {
+                    // Non-frozen shared references are not necessarily dereferenceable for the
+                    // entire duration of the function
+                    // (see <https://github.com/rust-lang/rust/pull/98017>).
+                    PointerKind::SharedRef { frozen } => frozen,
+                    // Mutable references to potentially self-referential types are not necessarily
+                    // dereferenceable for the entire duration of the function
+                    // (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>).
+                    PointerKind::MutableRef { unpin } => unpin,
+                    // Box may be deallocated during execution of the function.
+                    PointerKind::Box { .. } => false,
+                };
+            if no_free {
+                attrs.set(ArgAttribute::NoFree);
+            }
+
             if matches!(kind, PointerKind::SharedRef { frozen: true }) && !is_return {
                 attrs.set(ArgAttribute::ReadOnly);
                 attrs.set(ArgAttribute::CapturesReadOnly);
@@ -423,11 +437,18 @@ fn fn_abi_sanity_check<'tcx>(
     fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
     spec_abi: ExternAbi,
 ) {
+    fn fn_arg_attrs_sanity_check(attrs: &ArgAttributes, is_ret: bool) {
+        if attrs.regular.contains(ArgAttribute::NoFree) {
+            assert!(!is_ret, "NoFree not valid on return values");
+        }
+    }
+
     fn fn_arg_sanity_check<'tcx>(
         cx: &LayoutCx<'tcx>,
         fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
         spec_abi: ExternAbi,
         arg: &ArgAbi<'tcx, Ty<'tcx>>,
+        is_ret: bool,
     ) {
         let tcx = cx.tcx();
 
@@ -452,7 +473,7 @@ fn fn_abi_sanity_check<'tcx>(
             PassMode::Ignore => {
                 assert!(arg.layout.is_zst());
             }
-            PassMode::Direct(_) => {
+            PassMode::Direct(attrs) => {
                 // Here the Rust type is used to determine the actual ABI, so we have to be very
                 // careful. Scalar/Vector is fine, since backends will generally use
                 // `layout.backend_repr` and ignore everything else. We should just reject
@@ -482,28 +503,33 @@ fn fn_abi_sanity_check<'tcx>(
                         );
                     }
                 }
+                fn_arg_attrs_sanity_check(attrs, is_ret);
             }
-            PassMode::Pair(_, _) => {
+            PassMode::Pair(attrs1, attrs2) => {
                 // Similar to `Direct`, we need to make sure that backends use `layout.backend_repr`
                 // and ignore the rest of the layout.
                 assert!(
                     matches!(arg.layout.backend_repr, BackendRepr::ScalarPair(..)),
                     "PassMode::Pair for type {}",
                     arg.layout.ty
                 );
+                fn_arg_attrs_sanity_check(attrs1, is_ret);
+                fn_arg_attrs_sanity_check(attrs2, is_ret);
             }
             PassMode::Cast { .. } => {
                 // `Cast` means "transmute to `CastType`"; that only makes sense for sized types.
                 assert!(arg.layout.is_sized());
             }
-            PassMode::Indirect { meta_attrs: None, .. } => {
+            PassMode::Indirect { meta_attrs: None, attrs, .. } => {
                 // No metadata, must be sized.
                 // Conceptually, unsized arguments must be copied around, which requires dynamically
                 // determining their size, which we cannot do without metadata. Consult
                 // t-opsem before removing this check.
                 assert!(arg.layout.is_sized());
+                // Indirect returns are arguments from an ABI perspective.
+                fn_arg_attrs_sanity_check(attrs, false);
             }
-            PassMode::Indirect { meta_attrs: Some(_), on_stack, .. } => {
+            PassMode::Indirect { meta_attrs: Some(meta_attrs), attrs, on_stack } => {
                 // With metadata. Must be unsized and not on the stack.
                 assert!(arg.layout.is_unsized() && !on_stack);
                 // Also, must not be `extern` type.
@@ -515,14 +541,17 @@ fn fn_abi_sanity_check<'tcx>(
                     // t-opsem before removing this check.
                     panic!("unsized arguments must not be `extern` types");
                 }
+                // Indirect returns are arguments from an ABI perspective.
+                fn_arg_attrs_sanity_check(attrs, false);
+                fn_arg_attrs_sanity_check(meta_attrs, false);
             }
         }
     }
 
     for arg in fn_abi.args.iter() {
-        fn_arg_sanity_check(cx, fn_abi, spec_abi, arg);
+        fn_arg_sanity_check(cx, fn_abi, spec_abi, arg, false);
     }
-    fn_arg_sanity_check(cx, fn_abi, spec_abi, &fn_abi.ret);
+    fn_arg_sanity_check(cx, fn_abi, spec_abi, &fn_abi.ret, true);
 }
 
 #[tracing::instrument(

tests/codegen-llvm/addr-of-mutate.rs

@@ -5,7 +5,7 @@
 // Test for the absence of `readonly` on the argument when it is mutated via `&raw const`.
 // See <https://github.com/rust-lang/rust/issues/111502>.
 
-// CHECK: i8 @foo(ptr{{( dead_on_return)?}} noalias noundef align 1{{( captures\(address\))?}}{{( dead_on_return)?}} dereferenceable(128) %x)
+// CHECK: i8 @foo(ptr{{( dead_on_return)?}} noalias nofree noundef align 1{{( captures\(address\))?}}{{( dead_on_return)?}} dereferenceable(128) %x)
 #[no_mangle]
 pub fn foo(x: [u8; 128]) -> u8 {
     let ptr = core::ptr::addr_of!(x).cast_mut();
@@ -15,7 +15,7 @@ pub fn foo(x: [u8; 128]) -> u8 {
     x[0]
 }
 
-// CHECK: i1 @second(ptr{{( dead_on_return)?}} noalias noundef align {{[0-9]+}}{{( captures\(address\))?}}{{( dead_on_return)?}} dereferenceable({{[0-9]+}}) %a_ptr_and_b)
+// CHECK: i1 @second(ptr{{( dead_on_return)?}} noalias nofree noundef align {{[0-9]+}}{{( captures\(address\))?}}{{( dead_on_return)?}} dereferenceable({{[0-9]+}}) %a_ptr_and_b)
 #[no_mangle]
 pub unsafe fn second(a_ptr_and_b: (*mut (i32, bool), (i64, bool))) -> bool {
     let b_bool_ptr = core::ptr::addr_of!(a_ptr_and_b.1.1).cast_mut();
@@ -24,7 +24,7 @@ pub unsafe fn second(a_ptr_and_b: (*mut (i32, bool), (i64, bool))) -> bool {
 }
 
 // If going through a deref (and there are no other mutating accesses), then `readonly` is fine.
-// CHECK: i1 @third(ptr{{( dead_on_return)?}} noalias noundef readonly align {{[0-9]+}}{{( captures\(none\))?}}{{( dead_on_return)?}} dereferenceable({{[0-9]+}}) %a_ptr_and_b)
+// CHECK: i1 @third(ptr{{( dead_on_return)?}} noalias nofree noundef readonly align {{[0-9]+}}{{( captures\(none\))?}}{{( dead_on_return)?}} dereferenceable({{[0-9]+}}) %a_ptr_and_b)
 #[no_mangle]
 pub unsafe fn third(a_ptr_and_b: (*mut (i32, bool), (i64, bool))) -> bool {
     let b_bool_ptr = core::ptr::addr_of!((*a_ptr_and_b.0).1).cast_mut();

tests/codegen-llvm/drop-in-place-noalias.rs

@@ -7,7 +7,7 @@
 
 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{{.*}}StructUnpin{{.*}}(ptr noalias nofree noundef align 4 dereferenceable(12) %{{.+}})
 
 // CHECK: define internal void @{{.*}}core{{.*}}ptr{{.*}}drop_in_place{{.*}}StructNotUnpin{{.*}}(ptr noundef nonnull align 4 %{{.+}})