diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index d9393ffe534ac..854e3ccc21b4f 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -1,6 +1,7 @@ use crate::builder::Builder; use crate::context::CodegenCx; use crate::llvm::{self, AttributePlace}; +use crate::llvm_util; use crate::type_::Type; use crate::type_of::LayoutLlvmExt; use crate::value::Value; @@ -41,12 +42,29 @@ impl ArgAttributeExt for ArgAttribute { } pub trait ArgAttributesExt { - fn apply_attrs_to_llfn(&self, idx: AttributePlace, llfn: &Value); - fn apply_attrs_to_callsite(&self, idx: AttributePlace, callsite: &Value); + fn apply_attrs_to_llfn(&self, idx: AttributePlace, cx: &CodegenCx<'_, '_>, llfn: &Value); + fn apply_attrs_to_callsite( + &self, + idx: AttributePlace, + cx: &CodegenCx<'_, '_>, + callsite: &Value, + ); +} + +fn should_use_mutable_noalias(cx: &CodegenCx<'_, '_>) -> bool { + // LLVM prior to version 12 has known miscompiles in the presence of + // noalias attributes (see #54878). Only enable mutable noalias by + // default for versions we believe to be safe. + cx.tcx + .sess + .opts + .debugging_opts + .mutable_noalias + .unwrap_or_else(|| llvm_util::get_version() >= (12, 0, 0)) } impl ArgAttributesExt for ArgAttributes { - fn apply_attrs_to_llfn(&self, idx: AttributePlace, llfn: &Value) { + fn apply_attrs_to_llfn(&self, idx: AttributePlace, cx: &CodegenCx<'_, '_>, llfn: &Value) { let mut regular = self.regular; unsafe { let deref = self.pointee_size.bytes(); @@ -62,6 +80,9 @@ impl ArgAttributesExt for ArgAttributes { llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32); } regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn)); + if regular.contains(ArgAttribute::NoAliasMutRef) && should_use_mutable_noalias(cx) { + llvm::Attribute::NoAlias.apply_llfn(idx, llfn); + } match self.arg_ext { ArgExtension::None => {} ArgExtension::Zext => { @@ -74,7 +95,12 @@ impl ArgAttributesExt for ArgAttributes { } } - fn apply_attrs_to_callsite(&self, idx: AttributePlace, callsite: &Value) { + fn apply_attrs_to_callsite( + &self, + idx: AttributePlace, + cx: &CodegenCx<'_, '_>, + callsite: &Value, + ) { let mut regular = self.regular; unsafe { let deref = self.pointee_size.bytes(); @@ -98,6 +124,9 @@ impl ArgAttributesExt for ArgAttributes { ); } regular.for_each_kind(|attr| attr.apply_callsite(idx, callsite)); + if regular.contains(ArgAttribute::NoAliasMutRef) && should_use_mutable_noalias(cx) { + llvm::Attribute::NoAlias.apply_callsite(idx, callsite); + } match self.arg_ext { ArgExtension::None => {} ArgExtension::Zext => { @@ -419,13 +448,13 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { let mut i = 0; let mut apply = |attrs: &ArgAttributes| { - attrs.apply_attrs_to_llfn(llvm::AttributePlace::Argument(i), llfn); + attrs.apply_attrs_to_llfn(llvm::AttributePlace::Argument(i), cx, llfn); i += 1; i - 1 }; match self.ret.mode { PassMode::Direct(ref attrs) => { - attrs.apply_attrs_to_llfn(llvm::AttributePlace::ReturnValue, llfn); + attrs.apply_attrs_to_llfn(llvm::AttributePlace::ReturnValue, cx, llfn); } PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { assert!(!on_stack); @@ -480,18 +509,18 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { // FIXME(wesleywiser, eddyb): We should apply `nounwind` and `noreturn` as appropriate to this callsite. let mut i = 0; - let mut apply = |attrs: &ArgAttributes| { - attrs.apply_attrs_to_callsite(llvm::AttributePlace::Argument(i), callsite); + let mut apply = |cx: &CodegenCx<'_, '_>, attrs: &ArgAttributes| { + attrs.apply_attrs_to_callsite(llvm::AttributePlace::Argument(i), cx, callsite); i += 1; i - 1 }; match self.ret.mode { PassMode::Direct(ref attrs) => { - attrs.apply_attrs_to_callsite(llvm::AttributePlace::ReturnValue, callsite); + attrs.apply_attrs_to_callsite(llvm::AttributePlace::ReturnValue, &bx.cx, callsite); } PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { assert!(!on_stack); - let i = apply(attrs); + let i = apply(bx.cx, attrs); unsafe { llvm::LLVMRustAddStructRetCallSiteAttr( callsite, @@ -517,12 +546,12 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } for arg in &self.args { if arg.pad.is_some() { - apply(&ArgAttributes::new()); + apply(bx.cx, &ArgAttributes::new()); } match arg.mode { PassMode::Ignore => {} PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: true } => { - let i = apply(attrs); + let i = apply(bx.cx, attrs); unsafe { llvm::LLVMRustAddByValCallSiteAttr( callsite, @@ -533,22 +562,22 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } PassMode::Direct(ref attrs) | PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: false } => { - apply(attrs); + apply(bx.cx, attrs); } PassMode::Indirect { ref attrs, extra_attrs: Some(ref extra_attrs), on_stack: _, } => { - apply(attrs); - apply(extra_attrs); + apply(bx.cx, attrs); + apply(bx.cx, extra_attrs); } PassMode::Pair(ref a, ref b) => { - apply(a); - apply(b); + apply(bx.cx, a); + apply(bx.cx, b); } PassMode::Cast(_) => { - apply(&ArgAttributes::new()); + apply(bx.cx, &ArgAttributes::new()); } } } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 93ba2e6a4f1e9..b3f0c66de68c9 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -566,7 +566,7 @@ fn test_debugging_options_tracking_hash() { tracked!(merge_functions, Some(MergeFunctions::Disabled)); tracked!(mir_emit_retag, true); tracked!(mir_opt_level, Some(4)); - tracked!(mutable_noalias, true); + tracked!(mutable_noalias, Some(true)); tracked!(new_llvm_pass_manager, true); tracked!(no_codegen, true); tracked!(no_generate_arange_section, true); diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index ae367db019b59..53a7d8528d36e 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -993,6 +993,10 @@ rustc_queries! { query is_freeze_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { desc { "computing whether `{}` is freeze", env.value } } + /// Query backing `TyS::is_unpin`. + query is_unpin_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + desc { "computing whether `{}` is `Unpin`", env.value } + } /// Query backing `TyS::needs_drop`. query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { desc { "computing whether `{}` needs drop", env.value } diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 814581a6cf171..3a75a6d907d05 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -11,7 +11,7 @@ use rustc_hir as hir; use rustc_hir::lang_items::LangItem; use rustc_index::bit_set::BitSet; use rustc_index::vec::{Idx, IndexVec}; -use rustc_session::{DataTypeKind, FieldInfo, SizeKind, VariantInfo}; +use rustc_session::{config::OptLevel, DataTypeKind, FieldInfo, SizeKind, VariantInfo}; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::DUMMY_SP; use rustc_target::abi::call::{ @@ -2318,31 +2318,30 @@ where ty::Ref(_, ty, mt) if offset.bytes() == 0 => { let address_space = addr_space_of_ty(ty); let tcx = cx.tcx(); - let is_freeze = ty.is_freeze(tcx.at(DUMMY_SP), cx.param_env()); - let kind = match mt { - hir::Mutability::Not => { - if is_freeze { - PointerKind::Frozen - } else { - PointerKind::Shared + let kind = if tcx.sess.opts.optimize == OptLevel::No { + // Use conservative pointer kind if not optimizing. This saves us the + // Freeze/Unpin queries, and can save time in the codegen backend (noalias + // attributes in LLVM have compile-time cost even in unoptimized builds). + PointerKind::Shared + } else { + match mt { + hir::Mutability::Not => { + if ty.is_freeze(tcx.at(DUMMY_SP), cx.param_env()) { + PointerKind::Frozen + } else { + PointerKind::Shared + } } - } - hir::Mutability::Mut => { - // Previously we would only emit noalias annotations for LLVM >= 6 or in - // panic=abort mode. That was deemed right, as prior versions had many bugs - // in conjunction with unwinding, but later versions didn’t seem to have - // said issues. See issue #31681. - // - // Alas, later on we encountered a case where noalias would generate wrong - // code altogether even with recent versions of LLVM in *safe* code with no - // unwinding involved. See #54462. - // - // For now, do not enable mutable_noalias by default at all, while the - // issue is being figured out. - if tcx.sess.opts.debugging_opts.mutable_noalias { - PointerKind::UniqueBorrowed - } else { - PointerKind::Shared + hir::Mutability::Mut => { + // References to self-referential structures should not be considered + // noalias, as another pointer to the structure can be obtained, that + // is not based-on the original reference. We consider all !Unpin + // types to be potentially self-referential here. + if ty.is_unpin(tcx.at(DUMMY_SP), cx.param_env()) { + PointerKind::UniqueBorrowed + } else { + PointerKind::Shared + } } } }; @@ -2775,10 +2774,14 @@ where // and can be marked as both `readonly` and `noalias`, as // LLVM's definition of `noalias` is based solely on memory // dependencies rather than pointer equality + // + // Due to miscompiles in LLVM < 12, we apply a separate NoAliasMutRef attribute + // for UniqueBorrowed arguments, so that the codegen backend can decide + // whether or not to actually emit the attribute. let no_alias = match kind { - PointerKind::Shared => false, + PointerKind::Shared | PointerKind::UniqueBorrowed => false, PointerKind::UniqueOwned => true, - PointerKind::Frozen | PointerKind::UniqueBorrowed => !is_return, + PointerKind::Frozen => !is_return, }; if no_alias { attrs.set(ArgAttribute::NoAlias); @@ -2787,6 +2790,10 @@ where if kind == PointerKind::Frozen && !is_return { attrs.set(ArgAttribute::ReadOnly); } + + if kind == PointerKind::UniqueBorrowed && !is_return { + attrs.set(ArgAttribute::NoAliasMutRef); + } } } }; diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 8edde8794ed27..cff8166974a7a 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -741,6 +741,46 @@ impl<'tcx> ty::TyS<'tcx> { } } + /// Checks whether values of this type `T` implement the `Unpin` trait. + pub fn is_unpin(&'tcx self, tcx_at: TyCtxtAt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { + self.is_trivially_unpin() || tcx_at.is_unpin_raw(param_env.and(self)) + } + + /// Fast path helper for testing if a type is `Unpin`. + /// + /// Returning true means the type is known to be `Unpin`. Returning + /// `false` means nothing -- could be `Unpin`, might not be. + fn is_trivially_unpin(&self) -> bool { + match self.kind() { + ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Bool + | ty::Char + | ty::Str + | ty::Never + | ty::Ref(..) + | ty::RawPtr(_) + | ty::FnDef(..) + | ty::Error(_) + | ty::FnPtr(_) => true, + ty::Tuple(_) => self.tuple_fields().all(Self::is_trivially_unpin), + ty::Slice(elem_ty) | ty::Array(elem_ty, _) => elem_ty.is_trivially_unpin(), + ty::Adt(..) + | ty::Bound(..) + | ty::Closure(..) + | ty::Dynamic(..) + | ty::Foreign(_) + | ty::Generator(..) + | ty::GeneratorWitness(_) + | ty::Infer(_) + | ty::Opaque(..) + | ty::Param(_) + | ty::Placeholder(_) + | ty::Projection(_) => false, + } + } + /// If `ty.needs_drop(...)` returns `true`, then `ty` is definitely /// non-copy and *might* have a destructor attached; if it returns /// `false`, then `ty` definitely has no destructor (i.e., no drop glue). diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index d9e5a186073b6..517051b200b51 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -997,8 +997,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, (default: no)"), mir_opt_level: Option = (None, parse_opt_uint, [TRACKED], "MIR optimization level (0-4; default: 1 in non optimized builds and 2 in optimized builds)"), - mutable_noalias: bool = (false, parse_bool, [TRACKED], - "emit noalias metadata for mutable references (default: no)"), + mutable_noalias: Option = (None, parse_opt_bool, [TRACKED], + "emit noalias metadata for mutable references (default: yes for LLVM >= 12, otherwise no)"), new_llvm_pass_manager: bool = (false, parse_bool, [TRACKED], "use new LLVM pass manager (default: no)"), nll_facts: bool = (false, parse_bool, [UNTRACKED], diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 0deb1186b0fd6..2c3f7762759bf 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -65,7 +65,10 @@ mod attr_impl { const NoCapture = 1 << 2; const NonNull = 1 << 3; const ReadOnly = 1 << 4; - const InReg = 1 << 8; + const InReg = 1 << 5; + // NoAlias on &mut arguments can only be used with LLVM >= 12 due to miscompiles + // in earlier versions. FIXME: Remove this distinction once possible. + const NoAliasMutRef = 1 << 6; } } } diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index b14b1ef00db91..e2618da749fb3 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -1112,7 +1112,7 @@ pub enum PointerKind { /// `&T` where `T` contains no `UnsafeCell`, is `noalias` and `readonly`. Frozen, - /// `&mut T`, when we know `noalias` is safe for LLVM. + /// `&mut T` which is `noalias` but not `readonly`. UniqueBorrowed, /// `Box`, unlike `UniqueBorrowed`, it also has `noalias` on returns. diff --git a/compiler/rustc_ty_utils/src/common_traits.rs b/compiler/rustc_ty_utils/src/common_traits.rs index 24ba071786607..cedc84d97c2d9 100644 --- a/compiler/rustc_ty_utils/src/common_traits.rs +++ b/compiler/rustc_ty_utils/src/common_traits.rs @@ -18,6 +18,10 @@ fn is_freeze_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>> is_item_raw(tcx, query, LangItem::Freeze) } +fn is_unpin_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + is_item_raw(tcx, query, LangItem::Unpin) +} + fn is_item_raw<'tcx>( tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, @@ -37,5 +41,11 @@ fn is_item_raw<'tcx>( } pub(crate) fn provide(providers: &mut ty::query::Providers) { - *providers = ty::query::Providers { is_copy_raw, is_sized_raw, is_freeze_raw, ..*providers }; + *providers = ty::query::Providers { + is_copy_raw, + is_sized_raw, + is_freeze_raw, + is_unpin_raw, + ..*providers + }; } diff --git a/src/doc/reference b/src/doc/reference index e32a2f928f8b7..d10a0af8dca25 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit e32a2f928f8b78d534bca2b9e7736413314dc556 +Subproject commit d10a0af8dca25d9d548ca6a369fd66ad06acb3c9 diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index a1da4faf5d85a..0c34bf1b914b0 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -1,4 +1,4 @@ -// compile-flags: -C no-prepopulate-passes +// compile-flags: -O -C no-prepopulate-passes // ignore-tidy-linelength // min-system-llvm-version: 12.0 @@ -43,13 +43,13 @@ pub fn named_borrow<'r>(_: &'r i32) { pub fn unsafe_borrow(_: &UnsafeInner) { } -// CHECK: @mutable_unsafe_borrow(i16* align 2 dereferenceable(2) %_1) +// CHECK: @mutable_unsafe_borrow(i16* noalias align 2 dereferenceable(2) %_1) // ... unless this is a mutable borrow, those never alias #[no_mangle] pub fn mutable_unsafe_borrow(_: &mut UnsafeInner) { } -// CHECK: @mutable_borrow(i32* align 4 dereferenceable(4) %_1) +// CHECK: @mutable_borrow(i32* noalias align 4 dereferenceable(4) %_1) // FIXME #25759 This should also have `nocapture` #[no_mangle] pub fn mutable_borrow(_: &mut i32) { @@ -94,7 +94,7 @@ pub fn helper(_: usize) { pub fn slice(_: &[u8]) { } -// CHECK: @mutable_slice([0 x i8]* nonnull align 1 %_1.0, [[USIZE]] %_1.1) +// CHECK: @mutable_slice([0 x i8]* noalias nonnull align 1 %_1.0, [[USIZE]] %_1.1) // FIXME #25759 This should also have `nocapture` #[no_mangle] pub fn mutable_slice(_: &mut [u8]) { diff --git a/src/test/codegen/noalias-unpin.rs b/src/test/codegen/noalias-unpin.rs new file mode 100644 index 0000000000000..8ca9b98eee2f9 --- /dev/null +++ b/src/test/codegen/noalias-unpin.rs @@ -0,0 +1,15 @@ +// compile-flags: -O -Z mutable-noalias=yes + +#![crate_type = "lib"] + +pub struct SelfRef { + self_ref: *mut SelfRef, + _pin: std::marker::PhantomPinned +} + +// CHECK-LABEL: @test_self_ref( +// CHECK-NOT: noalias +#[no_mangle] +pub unsafe fn test_self_ref(s: &mut SelfRef) { + (*s.self_ref).self_ref = std::ptr::null_mut(); +} diff --git a/src/test/codegen/packed.rs b/src/test/codegen/packed.rs index c31e8457dcded..6ab28e87cb661 100644 --- a/src/test/codegen/packed.rs +++ b/src/test/codegen/packed.rs @@ -1,5 +1,5 @@ // ignore-tidy-linelength -// compile-flags: -C no-prepopulate-passes +// compile-flags: -O -C no-prepopulate-passes #![crate_type = "lib"]