Skip to content

Commit 365807f

Browse files
committed
Make Box<dyn FnOnce> respect self alignment
1 parent 5179ebe commit 365807f

File tree

6 files changed

+160
-15
lines changed

6 files changed

+160
-15
lines changed

src/librustc_mir_build/build/expr/as_operand.rs

+121
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,64 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
2121
self.as_operand(block, local_scope, expr)
2222
}
2323

24+
/// Returns an operand suitable for use until the end of the current scope expression and
25+
/// suitable also to be passed as function arguments.
26+
///
27+
/// The operand returned from this function will *not be valid* after an ExprKind::Scope is
28+
/// passed, so please do *not* return it from functions to avoid bad miscompiles. Returns an
29+
/// operand suitable for use as a call argument. This is almost always equivalent to
30+
/// `as_operand`, except for the particular case of passing values of (potentially) unsized
31+
/// types "by value" (see details below).
32+
///
33+
/// As with `as_operand`, the operand returned from this function will *not be valid* after an
34+
/// `ExprKind::Scope` is passed, so do not return it from functions.
35+
///
36+
/// # Parameters of unsized types
37+
///
38+
/// We tweak the handling of parameters of unsized type slightly to avoid the need to create a
39+
/// local variable of unsized type. For example, consider this program:
40+
///
41+
/// ```rust
42+
/// fn foo(p: dyn Debug) { ... }
43+
///
44+
/// fn bar(box_p: Box<dyn Debug>) { foo(*p); }
45+
/// ```
46+
///
47+
/// Ordinarily, for sized types, we would compile the call `foo(*p)` like so:
48+
///
49+
/// ```rust
50+
/// let tmp0 = *box_p; // tmp0 would be the operand returned by this function call
51+
/// foo(tmp0)
52+
/// ```
53+
///
54+
/// But because the parameter to `foo` is of the unsized type `dyn Debug`, and because it is
55+
/// being moved the deref of a box, we compile it slightly differently. The temporary `tmp0`
56+
/// that we create *stores the entire box*, and the parameter to the call itself will be
57+
/// `*tmp0`:
58+
///
59+
/// ```rust
60+
/// let tmp0 = box_p; call foo(*tmp0)
61+
/// ```
62+
///
63+
/// This way, the temporary `tmp0` that we create has type `Box<dyn Debug>`, which is sized.
64+
/// The value passed to the call (`*tmp0`) still has the `dyn Debug` type -- but the way that
65+
/// calls are compiled means that this parameter will be passed "by reference", meaning that we
66+
/// will actually provide a pointer to the interior of the box, and not move the `dyn Debug`
67+
/// value to the stack.
68+
///
69+
/// See #68034 for more details.
70+
crate fn as_local_call_operand<M>(
71+
&mut self,
72+
block: BasicBlock,
73+
expr: M,
74+
) -> BlockAnd<Operand<'tcx>>
75+
where
76+
M: Mirror<'tcx, Output = Expr<'tcx>>,
77+
{
78+
let local_scope = self.local_scope();
79+
self.as_call_operand(block, local_scope, expr)
80+
}
81+
2482
/// Compile `expr` into a value that can be used as an operand.
2583
/// If `expr` is a place like `x`, this will introduce a
2684
/// temporary `tmp = x`, so that we capture the value of `x` at
@@ -40,6 +98,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
4098
self.expr_as_operand(block, scope, expr)
4199
}
42100

101+
fn as_call_operand<M>(
102+
&mut self,
103+
block: BasicBlock,
104+
scope: Option<region::Scope>,
105+
expr: M,
106+
) -> BlockAnd<Operand<'tcx>>
107+
where
108+
M: Mirror<'tcx, Output = Expr<'tcx>>,
109+
{
110+
let expr = self.hir.mirror(expr);
111+
self.expr_as_call_operand(block, scope, expr)
112+
}
113+
43114
fn expr_as_operand(
44115
&mut self,
45116
mut block: BasicBlock,
@@ -69,4 +140,54 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
69140
}
70141
}
71142
}
143+
144+
fn expr_as_call_operand(
145+
&mut self,
146+
mut block: BasicBlock,
147+
scope: Option<region::Scope>,
148+
expr: Expr<'tcx>,
149+
) -> BlockAnd<Operand<'tcx>> {
150+
debug!("expr_as_call_operand(block={:?}, expr={:?})", block, expr);
151+
let this = self;
152+
153+
if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
154+
let source_info = this.source_info(expr.span);
155+
let region_scope = (region_scope, source_info);
156+
return this.in_scope(region_scope, lint_level, |this| {
157+
this.as_call_operand(block, scope, value)
158+
});
159+
}
160+
161+
let tcx = this.hir.tcx();
162+
163+
if tcx.features().unsized_locals {
164+
let ty = expr.ty;
165+
let span = expr.span;
166+
let param_env = this.hir.param_env;
167+
168+
if !ty.is_sized(tcx.at(span), param_env) {
169+
// !sized means !copy, so this is an unsized move
170+
assert!(!ty.is_copy_modulo_regions(tcx, param_env, span));
171+
172+
// As described above, detect the case where we are passing a value of unsized
173+
// type, and that value is coming from the deref of a box.
174+
if let ExprKind::Deref { ref arg } = expr.kind {
175+
let arg = this.hir.mirror(arg.clone());
176+
177+
// Generate let tmp0 = arg0
178+
let operand = unpack!(block = this.as_temp(block, scope, arg, Mutability::Mut));
179+
180+
// Return the operand *tmp0 to be used as the call argument
181+
let place = Place {
182+
local: operand,
183+
projection: tcx.intern_place_elems(&[PlaceElem::Deref]),
184+
};
185+
186+
return block.and(Operand::Move(place));
187+
}
188+
}
189+
}
190+
191+
this.expr_as_operand(block, scope, expr)
192+
}
72193
}

src/librustc_mir_build/build/expr/into.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
use crate::build::expr::category::{Category, RvalueFunc};
44
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
55
use crate::hair::*;
6-
use rustc_middle::mir::*;
7-
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation};
86
use rustc_data_structures::fx::FxHashMap;
97
use rustc_hir as hir;
8+
use rustc_middle::mir::*;
9+
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation};
1010
use rustc_span::symbol::sym;
1111

1212
use rustc_target::spec::abi::Abi;
@@ -207,7 +207,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
207207
} else {
208208
let args: Vec<_> = args
209209
.into_iter()
210-
.map(|arg| unpack!(block = this.as_local_operand(block, arg)))
210+
.map(|arg| unpack!(block = this.as_local_call_operand(block, arg)))
211211
.collect();
212212

213213
let success = this.cfg.start_new_block();

src/test/ui/fn/dyn-fn-alignment.rs

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// run-pass
2+
3+
#![feature(unsized_locals)]
4+
#![allow(dead_code)]
5+
#[repr(align(256))]
6+
struct A {
7+
v: u8,
8+
}
9+
10+
impl A {
11+
fn f(&self) -> *const A {
12+
self
13+
}
14+
}
15+
16+
fn f2(v: u8) -> Box<dyn FnOnce() -> *const A> {
17+
let a = A { v };
18+
Box::new(move || a.f())
19+
}
20+
21+
fn main() {
22+
let addr = f2(0)();
23+
assert_eq!(addr as usize % 256, 0, "addr: {:?}", addr);
24+
}

src/test/ui/unsized-locals/borrow-after-move.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ LL | println!("{}", &y);
4545
error[E0382]: borrow of moved value: `x`
4646
--> $DIR/borrow-after-move.rs:39:24
4747
|
48+
LL | let x = "hello".to_owned().into_boxed_str();
49+
| - move occurs because `x` has type `std::boxed::Box<str>`, which does not implement the `Copy` trait
4850
LL | x.foo();
4951
| - value moved here
5052
LL | println!("{}", &x);
51-
| ^^ value borrowed here after partial move
52-
|
53-
= note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait
53+
| ^^ value borrowed here after move
5454

5555
error: aborting due to 5 previous errors
5656

src/test/ui/unsized-locals/double-move.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -38,25 +38,25 @@ LL | y.foo();
3838
LL | y.foo();
3939
| ^ value used here after move
4040

41-
error[E0382]: use of moved value: `*x`
41+
error[E0382]: use of moved value: `x`
4242
--> $DIR/double-move.rs:45:9
4343
|
4444
LL | let _y = *x;
4545
| -- value moved here
4646
LL | x.foo();
47-
| ^ value used here after move
47+
| ^ value used here after partial move
4848
|
4949
= note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait
5050

5151
error[E0382]: use of moved value: `*x`
5252
--> $DIR/double-move.rs:51:18
5353
|
54+
LL | let x = "hello".to_owned().into_boxed_str();
55+
| - move occurs because `x` has type `std::boxed::Box<str>`, which does not implement the `Copy` trait
5456
LL | x.foo();
5557
| - value moved here
5658
LL | let _y = *x;
5759
| ^^ value used here after move
58-
|
59-
= note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait
6060

6161
error: aborting due to 6 previous errors
6262

src/test/ui/unsized-locals/unsized-exprs2.stderr

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
error[E0508]: cannot move out of type `[u8]`, a non-copy slice
2-
--> $DIR/unsized-exprs2.rs:22:19
2+
--> $DIR/unsized-exprs2.rs:22:5
33
|
44
LL | udrop::<[u8]>(foo()[..]);
5-
| ^^^^^^^^^
6-
| |
7-
| cannot move out of here
8-
| move occurs because value has type `[u8]`, which does not implement the `Copy` trait
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^
6+
| |
7+
| cannot move out of here
8+
| move occurs because value has type `[u8]`, which does not implement the `Copy` trait
99

1010
error: aborting due to previous error
1111

0 commit comments

Comments
 (0)