Make Box<dyn FnOnce> respect self alignment

This commit is contained in:
Santiago Pastorino 2020-04-17 09:27:35 -03:00
parent 5179ebe206
commit 365807f057
No known key found for this signature in database
GPG Key ID: 8131A24E0C79EFAF
6 changed files with 160 additions and 15 deletions

View File

@ -21,6 +21,64 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.as_operand(block, local_scope, expr)
}
/// Returns an operand suitable for use until the end of the current scope expression and
/// suitable also to be passed as function arguments.
///
/// The operand returned from this function will *not be valid* after an ExprKind::Scope is
/// passed, so please do *not* return it from functions to avoid bad miscompiles. Returns an
/// operand suitable for use as a call argument. This is almost always equivalent to
/// `as_operand`, except for the particular case of passing values of (potentially) unsized
/// types "by value" (see details below).
///
/// As with `as_operand`, the operand returned from this function will *not be valid* after an
/// `ExprKind::Scope` is passed, so do not return it from functions.
///
/// # Parameters of unsized types
///
/// We tweak the handling of parameters of unsized type slightly to avoid the need to create a
/// local variable of unsized type. For example, consider this program:
///
/// ```rust
/// fn foo(p: dyn Debug) { ... }
///
/// fn bar(box_p: Box<dyn Debug>) { foo(*p); }
/// ```
///
/// Ordinarily, for sized types, we would compile the call `foo(*p)` like so:
///
/// ```rust
/// let tmp0 = *box_p; // tmp0 would be the operand returned by this function call
/// foo(tmp0)
/// ```
///
/// But because the parameter to `foo` is of the unsized type `dyn Debug`, and because it is
/// being moved the deref of a box, we compile it slightly differently. The temporary `tmp0`
/// that we create *stores the entire box*, and the parameter to the call itself will be
/// `*tmp0`:
///
/// ```rust
/// let tmp0 = box_p; call foo(*tmp0)
/// ```
///
/// This way, the temporary `tmp0` that we create has type `Box<dyn Debug>`, which is sized.
/// The value passed to the call (`*tmp0`) still has the `dyn Debug` type -- but the way that
/// calls are compiled means that this parameter will be passed "by reference", meaning that we
/// will actually provide a pointer to the interior of the box, and not move the `dyn Debug`
/// value to the stack.
///
/// See #68034 for more details.
crate fn as_local_call_operand<M>(
&mut self,
block: BasicBlock,
expr: M,
) -> BlockAnd<Operand<'tcx>>
where
M: Mirror<'tcx, Output = Expr<'tcx>>,
{
let local_scope = self.local_scope();
self.as_call_operand(block, local_scope, expr)
}
/// Compile `expr` into a value that can be used as an operand.
/// If `expr` is a place like `x`, this will introduce a
/// temporary `tmp = x`, so that we capture the value of `x` at
@ -40,6 +98,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.expr_as_operand(block, scope, expr)
}
fn as_call_operand<M>(
&mut self,
block: BasicBlock,
scope: Option<region::Scope>,
expr: M,
) -> BlockAnd<Operand<'tcx>>
where
M: Mirror<'tcx, Output = Expr<'tcx>>,
{
let expr = self.hir.mirror(expr);
self.expr_as_call_operand(block, scope, expr)
}
fn expr_as_operand(
&mut self,
mut block: BasicBlock,
@ -69,4 +140,54 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}
}
fn expr_as_call_operand(
&mut self,
mut block: BasicBlock,
scope: Option<region::Scope>,
expr: Expr<'tcx>,
) -> BlockAnd<Operand<'tcx>> {
debug!("expr_as_call_operand(block={:?}, expr={:?})", block, expr);
let this = self;
if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
let source_info = this.source_info(expr.span);
let region_scope = (region_scope, source_info);
return this.in_scope(region_scope, lint_level, |this| {
this.as_call_operand(block, scope, value)
});
}
let tcx = this.hir.tcx();
if tcx.features().unsized_locals {
let ty = expr.ty;
let span = expr.span;
let param_env = this.hir.param_env;
if !ty.is_sized(tcx.at(span), param_env) {
// !sized means !copy, so this is an unsized move
assert!(!ty.is_copy_modulo_regions(tcx, param_env, span));
// As described above, detect the case where we are passing a value of unsized
// type, and that value is coming from the deref of a box.
if let ExprKind::Deref { ref arg } = expr.kind {
let arg = this.hir.mirror(arg.clone());
// Generate let tmp0 = arg0
let operand = unpack!(block = this.as_temp(block, scope, arg, Mutability::Mut));
// Return the operand *tmp0 to be used as the call argument
let place = Place {
local: operand,
projection: tcx.intern_place_elems(&[PlaceElem::Deref]),
};
return block.and(Operand::Move(place));
}
}
}
this.expr_as_operand(block, scope, expr)
}
}

View File

@ -3,10 +3,10 @@
use crate::build::expr::category::{Category, RvalueFunc};
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::hair::*;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir as hir;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation};
use rustc_span::symbol::sym;
use rustc_target::spec::abi::Abi;
@ -207,7 +207,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} else {
let args: Vec<_> = args
.into_iter()
.map(|arg| unpack!(block = this.as_local_operand(block, arg)))
.map(|arg| unpack!(block = this.as_local_call_operand(block, arg)))
.collect();
let success = this.cfg.start_new_block();

View File

@ -0,0 +1,24 @@
// run-pass
#![feature(unsized_locals)]
#![allow(dead_code)]
#[repr(align(256))]
struct A {
v: u8,
}
impl A {
fn f(&self) -> *const A {
self
}
}
fn f2(v: u8) -> Box<dyn FnOnce() -> *const A> {
let a = A { v };
Box::new(move || a.f())
}
fn main() {
let addr = f2(0)();
assert_eq!(addr as usize % 256, 0, "addr: {:?}", addr);
}

View File

@ -45,12 +45,12 @@ LL | println!("{}", &y);
error[E0382]: borrow of moved value: `x`
--> $DIR/borrow-after-move.rs:39:24
|
LL | let x = "hello".to_owned().into_boxed_str();
| - move occurs because `x` has type `std::boxed::Box<str>`, which does not implement the `Copy` trait
LL | x.foo();
| - value moved here
LL | println!("{}", &x);
| ^^ value borrowed here after partial move
|
= note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait
| ^^ value borrowed here after move
error: aborting due to 5 previous errors

View File

@ -38,25 +38,25 @@ LL | y.foo();
LL | y.foo();
| ^ value used here after move
error[E0382]: use of moved value: `*x`
error[E0382]: use of moved value: `x`
--> $DIR/double-move.rs:45:9
|
LL | let _y = *x;
| -- value moved here
LL | x.foo();
| ^ value used here after move
| ^ value used here after partial move
|
= note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait
error[E0382]: use of moved value: `*x`
--> $DIR/double-move.rs:51:18
|
LL | let x = "hello".to_owned().into_boxed_str();
| - move occurs because `x` has type `std::boxed::Box<str>`, which does not implement the `Copy` trait
LL | x.foo();
| - value moved here
LL | let _y = *x;
| ^^ value used here after move
|
= note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait
error: aborting due to 6 previous errors

View File

@ -1,11 +1,11 @@
error[E0508]: cannot move out of type `[u8]`, a non-copy slice
--> $DIR/unsized-exprs2.rs:22:19
--> $DIR/unsized-exprs2.rs:22:5
|
LL | udrop::<[u8]>(foo()[..]);
| ^^^^^^^^^
| |
| cannot move out of here
| move occurs because value has type `[u8]`, which does not implement the `Copy` trait
| ^^^^^^^^^^^^^^^^^^^^^^^^
| |
| cannot move out of here
| move occurs because value has type `[u8]`, which does not implement the `Copy` trait
error: aborting due to previous error