Skip to content

Commit

Permalink
Auto merge of rust-lang#122206 - matthiaskrgr:rollup-4txx9wx, r=matth…
Browse files Browse the repository at this point in the history
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#121201 (align_offset, align_to: no longer allow implementations to spuriously fail to align)
 - rust-lang#122076 (Tweak the way we protect in-place function arguments in interpreters)
 - rust-lang#122100 (Better comment for implicit captures in RPITIT)
 - rust-lang#122157 (Add the new description field to Target::to_json, and add descriptions for some MSVC targets)
 - rust-lang#122164 (Fix misaligned loads when loading UEFI arg pointers)
 - rust-lang#122171 (Add some new solver tests)
 - rust-lang#122172 (Don't ICE if we collect no RPITITs unless there are no unification errors)
 - rust-lang#122197 (inspect formatter: add braces)
 - rust-lang#122198 (Remove handling for previously dropped LLVM version)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Mar 8, 2024
2 parents a655e64 + b61edb9 commit 46b180e
Show file tree
Hide file tree
Showing 47 changed files with 418 additions and 214 deletions.
14 changes: 4 additions & 10 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,35 +260,29 @@ pub unsafe fn create_module<'ll>(
}

if let Some(BranchProtection { bti, pac_ret }) = sess.opts.unstable_opts.branch_protection {
let behavior = if llvm_version >= (15, 0, 0) {
llvm::LLVMModFlagBehavior::Min
} else {
llvm::LLVMModFlagBehavior::Error
};

if sess.target.arch == "aarch64" {
llvm::LLVMRustAddModuleFlag(
llmod,
behavior,
llvm::LLVMModFlagBehavior::Min,
c"branch-target-enforcement".as_ptr().cast(),
bti.into(),
);
llvm::LLVMRustAddModuleFlag(
llmod,
behavior,
llvm::LLVMModFlagBehavior::Min,
c"sign-return-address".as_ptr().cast(),
pac_ret.is_some().into(),
);
let pac_opts = pac_ret.unwrap_or(PacRet { leaf: false, key: PAuthKey::A });
llvm::LLVMRustAddModuleFlag(
llmod,
behavior,
llvm::LLVMModFlagBehavior::Min,
c"sign-return-address-all".as_ptr().cast(),
pac_opts.leaf.into(),
);
llvm::LLVMRustAddModuleFlag(
llmod,
behavior,
llvm::LLVMModFlagBehavior::Min,
c"sign-return-address-with-bkey".as_ptr().cast(),
u32::from(pac_opts.key == PAuthKey::B),
);
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,11 +510,13 @@ fn get_argc_argv<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// Params for UEFI
let param_handle = bx.get_param(0);
let param_system_table = bx.get_param(1);
let ptr_size = bx.tcx().data_layout.pointer_size;
let ptr_align = bx.tcx().data_layout.pointer_align.abi;
let arg_argc = bx.const_int(cx.type_isize(), 2);
let arg_argv = bx.alloca(cx.type_array(cx.type_ptr(), 2), Align::ONE);
bx.store(param_handle, arg_argv, Align::ONE);
let arg_argv_el1 = bx.gep(cx.type_ptr(), arg_argv, &[bx.const_int(cx.type_int(), 1)]);
bx.store(param_system_table, arg_argv_el1, Align::ONE);
let arg_argv = bx.alloca(cx.type_array(cx.type_ptr(), 2), ptr_align);
bx.store(param_handle, arg_argv, ptr_align);
let arg_argv_el1 = bx.inbounds_ptradd(arg_argv, bx.const_usize(ptr_size.bytes()));
bx.store(param_system_table, arg_argv_el1, ptr_align);
(arg_argc, arg_argv)
} else if cx.sess().target.main_needs_argc_argv {
// Params from native `main()` used as args for rust start function
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
if self.tcx.has_attr(def_id, sym::rustc_const_panic_str)
|| Some(def_id) == self.tcx.lang_items().begin_panic_fn()
{
let args = self.copy_fn_args(args)?;
let args = self.copy_fn_args(args);
// &str or &&str
assert!(args.len() == 1);

Expand All @@ -254,7 +254,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {

return Ok(Some(new_instance));
} else if Some(def_id) == self.tcx.lang_items().align_offset_fn() {
let args = self.copy_fn_args(args)?;
let args = self.copy_fn_args(args);
// For align_offset, we replace the function call if the pointer has no address.
match self.align_offset(instance, &args, dest, ret)? {
ControlFlow::Continue(()) => return Ok(Some(instance)),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,11 +472,11 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
/// argument/return value was actually copied or passed in-place..
fn protect_in_place_function_argument(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
place: &PlaceTy<'tcx, Self::Provenance>,
mplace: &MPlaceTy<'tcx, Self::Provenance>,
) -> InterpResult<'tcx> {
// Without an aliasing model, all we can do is put `Uninit` into the place.
// Conveniently this also ensures that the place actually points to suitable memory.
ecx.write_uninit(place)
ecx.write_uninit(mplace)
}

/// Called immediately before a new stack frame gets pushed.
Expand Down
70 changes: 48 additions & 22 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::borrow::Cow;

use either::Either;

use rustc_middle::{
mir,
ty::{
Expand Down Expand Up @@ -29,28 +31,25 @@ pub enum FnArg<'tcx, Prov: Provenance = CtfeProvenance> {
Copy(OpTy<'tcx, Prov>),
/// Allow for the argument to be passed in-place: destroy the value originally stored at that place and
/// make the place inaccessible for the duration of the function call.
InPlace(PlaceTy<'tcx, Prov>),
InPlace(MPlaceTy<'tcx, Prov>),
}

impl<'tcx, Prov: Provenance> FnArg<'tcx, Prov> {
pub fn layout(&self) -> &TyAndLayout<'tcx> {
match self {
FnArg::Copy(op) => &op.layout,
FnArg::InPlace(place) => &place.layout,
FnArg::InPlace(mplace) => &mplace.layout,
}
}
}

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Make a copy of the given fn_arg. Any `InPlace` are degenerated to copies, no protection of the
/// original memory occurs.
pub fn copy_fn_arg(
&self,
arg: &FnArg<'tcx, M::Provenance>,
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
pub fn copy_fn_arg(&self, arg: &FnArg<'tcx, M::Provenance>) -> OpTy<'tcx, M::Provenance> {
match arg {
FnArg::Copy(op) => Ok(op.clone()),
FnArg::InPlace(place) => self.place_to_op(place),
FnArg::Copy(op) => op.clone(),
FnArg::InPlace(mplace) => mplace.clone().into(),
}
}

Expand All @@ -59,7 +58,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn copy_fn_args(
&self,
args: &[FnArg<'tcx, M::Provenance>],
) -> InterpResult<'tcx, Vec<OpTy<'tcx, M::Provenance>>> {
) -> Vec<OpTy<'tcx, M::Provenance>> {
args.iter().map(|fn_arg| self.copy_fn_arg(fn_arg)).collect()
}

Expand All @@ -70,7 +69,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, FnArg<'tcx, M::Provenance>> {
Ok(match arg {
FnArg::Copy(op) => FnArg::Copy(self.project_field(op, field)?),
FnArg::InPlace(place) => FnArg::InPlace(self.project_field(place, field)?),
FnArg::InPlace(mplace) => FnArg::InPlace(self.project_field(mplace, field)?),
})
}

Expand Down Expand Up @@ -238,10 +237,36 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, Vec<FnArg<'tcx, M::Provenance>>> {
ops.iter()
.map(|op| {
Ok(match &op.node {
mir::Operand::Move(place) => FnArg::InPlace(self.eval_place(*place)?),
_ => FnArg::Copy(self.eval_operand(&op.node, None)?),
})
let arg = match &op.node {
mir::Operand::Copy(_) | mir::Operand::Constant(_) => {
// Make a regular copy.
let op = self.eval_operand(&op.node, None)?;
FnArg::Copy(op)
}
mir::Operand::Move(place) => {
// If this place lives in memory, preserve its location.
// We call `place_to_op` which will be an `MPlaceTy` whenever there exists
// an mplace for this place. (This is in contrast to `PlaceTy::as_mplace_or_local`
// which can return a local even if that has an mplace.)
let place = self.eval_place(*place)?;
let op = self.place_to_op(&place)?;

match op.as_mplace_or_imm() {
Either::Left(mplace) => FnArg::InPlace(mplace),
Either::Right(_imm) => {
// This argument doesn't live in memory, so there's no place
// to make inaccessible during the call.
// We rely on there not being any stray `PlaceTy` that would let the
// caller directly access this local!
// This is also crucial for tail calls, where we want the `FnArg` to
// stay valid when the old stack frame gets popped.
FnArg::Copy(op)
}
}
}
};

Ok(arg)
})
.collect()
}
Expand Down Expand Up @@ -451,7 +476,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// We work with a copy of the argument for now; if this is in-place argument passing, we
// will later protect the source it comes from. This means the callee cannot observe if we
// did in-place of by-copy argument passing, except for pointer equality tests.
let caller_arg_copy = self.copy_fn_arg(caller_arg)?;
let caller_arg_copy = self.copy_fn_arg(caller_arg);
if !already_live {
let local = callee_arg.as_local().unwrap();
let meta = caller_arg_copy.meta();
Expand All @@ -469,8 +494,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// specifically.)
self.copy_op_allow_transmute(&caller_arg_copy, &callee_arg)?;
// If this was an in-place pass, protect the place it comes from for the duration of the call.
if let FnArg::InPlace(place) = caller_arg {
M::protect_in_place_function_argument(self, place)?;
if let FnArg::InPlace(mplace) = caller_arg {
M::protect_in_place_function_argument(self, mplace)?;
}
Ok(())
}
Expand Down Expand Up @@ -517,7 +542,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
M::call_intrinsic(
self,
instance,
&self.copy_fn_args(args)?,
&self.copy_fn_args(args),
destination,
target,
unwind,
Expand Down Expand Up @@ -594,8 +619,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.map(|arg| (
arg.layout().ty,
match arg {
FnArg::Copy(op) => format!("copy({:?})", *op),
FnArg::InPlace(place) => format!("in-place({:?})", *place),
FnArg::Copy(op) => format!("copy({op:?})"),
FnArg::InPlace(mplace) => format!("in-place({mplace:?})"),
}
))
.collect::<Vec<_>>()
Expand Down Expand Up @@ -717,8 +742,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
callee_ty: callee_fn_abi.ret.layout.ty
});
}

// Protect return place for in-place return value passing.
M::protect_in_place_function_argument(self, &destination.clone().into())?;
M::protect_in_place_function_argument(self, &destination)?;

// Don't forget to mark "initially live" locals as live.
self.storage_live_for_always_live_locals()?;
Expand All @@ -741,7 +767,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// An `InPlace` does nothing here, we keep the original receiver intact. We can't
// really pass the argument in-place anyway, and we are constructing a new
// `Immediate` receiver.
let mut receiver = self.copy_fn_arg(&args[0])?;
let mut receiver = self.copy_fn_arg(&args[0]);
let receiver_place = loop {
match receiver.layout.ty.kind() {
ty::Ref(..) | ty::RawPtr(..) => {
Expand Down
59 changes: 14 additions & 45 deletions compiler/rustc_error_codes/src/error_codes/E0657.md
Original file line number Diff line number Diff line change
@@ -1,57 +1,26 @@
A lifetime bound on a trait implementation was captured at an incorrect place.
An `impl Trait` captured a higher-ranked lifetime, which is not supported.

Currently, `impl Trait` types are only allowed to capture lifetimes from
their parent items, and not from any `for<'a>` binders in scope.

Erroneous code example:

```compile_fail,E0657
trait Id<T> {}
trait Lt<'a> {}
impl<'a> Lt<'a> for () {}
impl<T> Id<T> for T {}
fn free_fn_capture_hrtb_in_impl_trait()
-> Box<for<'a> Id<impl Lt<'a>>> // error!
{
Box::new(())
}
trait BorrowInto<'a> {
type Target;
struct Foo;
impl Foo {
fn impl_fn_capture_hrtb_in_impl_trait()
-> Box<for<'a> Id<impl Lt<'a>>> // error!
{
Box::new(())
}
fn borrow_into(&'a self) -> Self::Target;
}
```
Here, you have used the inappropriate lifetime in the `impl Trait`,
The `impl Trait` can only capture lifetimes bound at the fn or impl
level.
impl<'a> BorrowInto<'a> for () {
type Target = &'a ();
To fix this we have to define the lifetime at the function or impl
level and use that lifetime in the `impl Trait`. For example you can
define the lifetime at the function:

```
trait Id<T> {}
trait Lt<'a> {}
impl<'a> Lt<'a> for () {}
impl<T> Id<T> for T {}
fn free_fn_capture_hrtb_in_impl_trait<'b>()
-> Box<for<'a> Id<impl Lt<'b>>> // ok!
{
Box::new(())
fn borrow_into(&'a self) -> Self::Target {
self
}
}
struct Foo;
impl Foo {
fn impl_fn_capture_hrtb_in_impl_trait<'b>()
-> Box<for<'a> Id<impl Lt<'b>>> // ok!
{
Box::new(())
}
fn opaque() -> impl for<'a> BorrowInto<'a, Target = impl Sized + 'a> {
()
}
```
4 changes: 4 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,10 @@ hir_analysis_only_current_traits_primitive = only traits defined in the current
hir_analysis_only_current_traits_ty = `{$ty}` is not defined in the current crate
hir_analysis_opaque_captures_higher_ranked_lifetime = `impl Trait` cannot capture {$bad_place}
.label = `impl Trait` implicitly captures all lifetimes in scope
.note = lifetime declared here
hir_analysis_paren_sugar_attribute = the `#[rustc_paren_sugar]` attribute is a temporary means of controlling which traits can use parenthetical notation
.help = add `#![feature(unboxed_closures)]` to the crate attributes to use it
Expand Down
15 changes: 7 additions & 8 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,14 +521,6 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
)
.fold_with(&mut collector);

if !unnormalized_trait_sig.output().references_error() {
debug_assert_ne!(
collector.types.len(),
0,
"expect >1 RPITITs in call to `collect_return_position_impl_trait_in_trait_tys`"
);
}

let trait_sig = ocx.normalize(&misc_cause, param_env, unnormalized_trait_sig);
trait_sig.error_reported()?;
let trait_return_ty = trait_sig.output();
Expand Down Expand Up @@ -647,6 +639,13 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
}
}

if !unnormalized_trait_sig.output().references_error() {
debug_assert!(
!collector.types.is_empty(),
"expect >0 RPITITs in call to `collect_return_position_impl_trait_in_trait_tys`"
);
}

// FIXME: This has the same issue as #108544, but since this isn't breaking
// existing code, I'm not particularly inclined to do the same hack as above
// where we process wf obligations manually. This can be fixed in a forward-
Expand Down
Loading

0 comments on commit 46b180e

Please sign in to comment.