Skip to content

Remove support for dynamic allocas #142911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions compiler/rustc_codegen_gcc/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,10 +931,6 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
.get_address(self.location)
}

fn dynamic_alloca(&mut self, _len: RValue<'gcc>, _align: Align) -> RValue<'gcc> {
unimplemented!();
}

fn load(&mut self, pointee_ty: Type<'gcc>, ptr: RValue<'gcc>, align: Align) -> RValue<'gcc> {
let block = self.llbb();
let function = block.get_function();
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,16 +538,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
}
}

fn dynamic_alloca(&mut self, size: &'ll Value, align: Align) -> &'ll Value {
unsafe {
let alloca =
llvm::LLVMBuildArrayAlloca(self.llbuilder, self.cx().type_i8(), size, UNNAMED);
llvm::LLVMSetAlignment(alloca, align.bytes() as c_uint);
// Cast to default addrspace if necessary
llvm::LLVMBuildPointerCast(self.llbuilder, alloca, self.cx().type_ptr(), UNNAMED)
}
}

fn load(&mut self, ty: &'ll Type, ptr: &'ll Value, align: Align) -> &'ll Value {
unsafe {
let load = llvm::LLVMBuildLoad2(self.llbuilder, ty, ptr, UNNAMED);
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1492,12 +1492,6 @@ unsafe extern "C" {
Ty: &'a Type,
Name: *const c_char,
) -> &'a Value;
pub(crate) fn LLVMBuildArrayAlloca<'a>(
B: &Builder<'a>,
Ty: &'a Type,
Val: &'a Value,
Name: *const c_char,
) -> &'a Value;
pub(crate) fn LLVMBuildLoad2<'a>(
B: &Builder<'a>,
Ty: &'a Type,
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,11 @@ enum LocalRef<'tcx, V> {
Place(PlaceRef<'tcx, V>),
/// `UnsizedPlace(p)`: `p` itself is a thin pointer (indirect place).
/// `*p` is the wide pointer that references the actual unsized place.
/// Every time it is initialized, we have to reallocate the place
/// and update the wide pointer. That's the reason why it is indirect.
///
/// Rust has no alloca and thus no ability to move the unsized place.
Comment on lines 141 to +144
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be made a direct place in the future? If so maybe add a FIXME?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know anything about codegen, so I don't know about that. There are quite a few occasions of UnsizedPlace(_) => bug!() though, so it does look like something that could be refactored.

///
/// FIXME: Since unsized locals are gutted, can we maybe use `Place` here? Or refactor it in
/// another way? There are quite a few `UnsizedPlace => bug` branches now.
UnsizedPlace(PlaceRef<'tcx, V>),
/// The backend [`OperandValue`] has already been generated.
Operand(OperandRef<'tcx, V>),
Expand Down Expand Up @@ -498,7 +501,7 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
LocalRef::Place(PlaceRef::new_sized(llarg, arg.layout))
}
}
// Unsized indirect qrguments
// Unsized indirect arguments
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => {
// As the storage for the indirect argument lives during
// the whole function call, we just copy the wide pointer.
Expand Down
40 changes: 1 addition & 39 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ use tracing::{debug, instrument};
use super::place::{PlaceRef, PlaceValue};
use super::rvalue::transmute_immediate;
use super::{FunctionCx, LocalRef};
use crate::MemFlags;
use crate::common::IntPredicate;
use crate::traits::*;
use crate::{MemFlags, size_of_val};

/// The representation of a Rust value. The enum variant is in fact
/// uniquely determined by the value's type, but is kept as a
Expand Down Expand Up @@ -771,44 +771,6 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
}
}
}

pub fn store_unsized<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
self,
bx: &mut Bx,
indirect_dest: PlaceRef<'tcx, V>,
) {
debug!("OperandRef::store_unsized: operand={:?}, indirect_dest={:?}", self, indirect_dest);
// `indirect_dest` must have `*mut T` type. We extract `T` out of it.
let unsized_ty = indirect_dest
.layout
.ty
.builtin_deref(true)
.unwrap_or_else(|| bug!("indirect_dest has non-pointer type: {:?}", indirect_dest));

let OperandValue::Ref(PlaceValue { llval: llptr, llextra: Some(llextra), .. }) = self
else {
bug!("store_unsized called with a sized value (or with an extern type)")
};

// Allocate an appropriate region on the stack, and copy the value into it. Since alloca
// doesn't support dynamic alignment, we allocate an extra align - 1 bytes, and align the
// pointer manually.
let (size, align) = size_of_val::size_and_align_of_dst(bx, unsized_ty, Some(llextra));
let one = bx.const_usize(1);
let align_minus_1 = bx.sub(align, one);
let size_extra = bx.add(size, align_minus_1);
let min_align = Align::ONE;
let alloca = bx.dynamic_alloca(size_extra, min_align);
let address = bx.ptrtoint(alloca, bx.type_isize());
let neg_address = bx.neg(address);
let offset = bx.and(neg_address, align_minus_1);
let dst = bx.inbounds_ptradd(alloca, offset);
bx.memcpy(dst, min_align, llptr, min_align, size, MemFlags::empty());

// Store the allocated region and the extra to the indirect place.
let indirect_operand = OperandValue::Pair(dst, llextra);
indirect_operand.store(bx, indirect_dest);
}
}

impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Expand Down
21 changes: 0 additions & 21 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,27 +364,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Some(imm)
}

pub(crate) fn codegen_rvalue_unsized(
&mut self,
bx: &mut Bx,
indirect_dest: PlaceRef<'tcx, Bx::Value>,
rvalue: &mir::Rvalue<'tcx>,
) {
debug!(
"codegen_rvalue_unsized(indirect_dest.llval={:?}, rvalue={:?})",
indirect_dest.val.llval, rvalue
);

match *rvalue {
mir::Rvalue::Use(ref operand) => {
let cg_operand = self.codegen_operand(bx, operand);
cg_operand.val.store_unsized(bx, indirect_dest);
}

_ => bug!("unsized assignment other than `Rvalue::Use`"),
}
}

pub(crate) fn codegen_rvalue_operand(
&mut self,
bx: &mut Bx,
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
match self.locals[index] {
LocalRef::Place(cg_dest) => self.codegen_rvalue(bx, cg_dest, rvalue),
LocalRef::UnsizedPlace(cg_indirect_dest) => {
self.codegen_rvalue_unsized(bx, cg_indirect_dest, rvalue)
let ty = cg_indirect_dest.layout.ty;
span_bug!(
statement.source_info.span,
"cannot reallocate from `UnsizedPlace({ty})` \
into `{rvalue:?}`; dynamic alloca is not supported",
);
}
LocalRef::PendingOperand => {
let operand = self.codegen_rvalue_operand(bx, rvalue);
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_codegen_ssa/src/traits/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ pub trait BuilderMethods<'a, 'tcx>:
fn to_immediate_scalar(&mut self, val: Self::Value, scalar: Scalar) -> Self::Value;

fn alloca(&mut self, size: Size, align: Align) -> Self::Value;
fn dynamic_alloca(&mut self, size: Self::Value, align: Align) -> Self::Value;

fn load(&mut self, ty: Self::Type, ptr: Self::Value, align: Align) -> Self::Value;
fn volatile_load(&mut self, ty: Self::Type, ptr: Self::Value) -> Self::Value;
Expand Down
20 changes: 19 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
arg_expr.span,
ObligationCauseCode::WellFormed(None),
);

// Unsized function arguments must be place expressions, because we can't move them.
self.check_place_expr_if_unsized(fn_input_ty, arg_expr);
}

// First, let's unify the formal method signature with the expectation eagerly.
Expand Down Expand Up @@ -544,6 +547,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// If `unsized_fn_params` is active, check that unsized values are place
/// expressions. This is needed because we can't move unsized values.
///
/// If `unsized_fn_params` is inactive, this will be checked in borrowck instead.
fn check_place_expr_if_unsized(&self, ty: Ty<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if self.tcx.features().unsized_fn_params() && !expr.is_syntactic_place_expr() {
self.require_type_is_sized(
ty,
expr.span,
ObligationCauseCode::UnsizedNonPlaceExpr(expr.span),
);
}
}

fn report_arg_errors(
&self,
compatibility_diagonal: IndexVec<ProvidedIdx, Compatibility<'tcx>>,
Expand Down Expand Up @@ -1870,7 +1887,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
});
}
hir::StmtKind::Semi(expr) => {
self.check_expr(expr);
let ty = self.check_expr(expr);
self.check_place_expr_if_unsized(ty, expr);
}
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,10 @@ pub enum ObligationCauseCode<'tcx> {

/// Obligations emitted during the normalization of a free type alias.
TypeAlias(ObligationCauseCodeHandle<'tcx>, Span, DefId),

/// Only reachable if the `unsized_fn_params` feature is used. Unsized function arguments must
/// be place expressions, because we can't move them.
UnsizedNonPlaceExpr(Span),
}

/// Whether a value can be extracted into a const.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3607,6 +3607,12 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
);
suggest_remove_deref(err, &expr);
}
ObligationCauseCode::UnsizedNonPlaceExpr(span) => {
err.span_note(
span,
"unsized values must be place expressions and cannot be moved from",
);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion library/core/src/mem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub const fn forget<T>(t: T) {
///
/// While Rust does not permit unsized locals since its removal in [#111942] it is
/// still possible to call functions with unsized values from a function argument
/// or in-place construction.
/// or place expression.
///
/// ```rust
/// #![feature(unsized_fn_params, forget_unsized)]
Expand Down
5 changes: 0 additions & 5 deletions tests/ui/unsized-locals/unsized-exprs-rpass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ impl std::ops::Add<i32> for A<[u8]> {
}

fn main() {
udrop::<[u8]>(loop {
break *foo();
});
udrop::<[u8]>(if true { *foo() } else { *foo() });
udrop::<[u8]>({ *foo() });
udrop::<[u8]>((*foo()));
*afoo() + 42;
udrop as fn([u8]);
Expand Down
12 changes: 10 additions & 2 deletions tests/ui/unsized-locals/unsized-exprs.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ note: required because it appears within the type `A<[u8]>`
|
LL | struct A<X: ?Sized>(X);
| ^
= note: structs must have a statically known size to be initialized
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-exprs.rs:19:22
|
LL | udrop::<A<[u8]>>(A { 0: *foo() });
| ^^^^^^^^^^^^^^^

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/unsized-exprs.rs:21:22
Expand All @@ -24,7 +28,11 @@ note: required because it appears within the type `A<[u8]>`
|
LL | struct A<X: ?Sized>(X);
| ^
= note: the return type of a function must have a statically known size
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-exprs.rs:21:22
|
LL | udrop::<A<[u8]>>(A(*foo()));
| ^^^^^^^^^

error: aborting due to 2 previous errors

Expand Down
27 changes: 27 additions & 0 deletions tests/ui/unsized-locals/unsized-non-place-exprs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//! `#![feature(unsized_fn_params)]` lets you use unsized function parameters. In particular this
//! is load bearing for `Box<dyn FnOnce()>: FnOnce()`. To do that, borrowck relaxes the requirement
//! that certain places must be `Sized`. But in #142911 we removed alloca support, so these
//! arguments can't move (or ICE at codegen) That means when `unsized_fn_params` is enabled, we must
//! explicitly check that unsized function arguments are place expressions.
//!
//! Also see tests/ui/unsized_locals/unsized-exprs-rpass.rs

#![feature(unsized_fn_params)]

fn foo() -> Box<[u8]> {
Box::new(*b"foo")
}

fn udrop<T: ?Sized>(_x: T) {}

fn main(){
// NB The ordering of the following operations matters, otherwise errors get swallowed somehow.

udrop::<[u8]>(if true { *foo() } else { *foo() }); //~ERROR the size for values of type `[u8]` cannot be known at compilation time
udrop::<[u8]>({ *foo() }); //~ERROR the size for values of type `[u8]` cannot be known at compilation time
udrop(match foo() { x => *x }); //~ERROR the size for values of type `[u8]` cannot be known at compilation time
udrop::<[u8]>({ loop { break *foo(); } }); //~ERROR the size for values of type `[u8]` cannot be known at compilation time

{ *foo() }; //~ERROR the size for values of type `[u8]` cannot be known at compilation time
{ loop { break *foo(); } }; //~ERROR the size for values of type `[u8]` cannot be known at compilation time
}
81 changes: 81 additions & 0 deletions tests/ui/unsized-locals/unsized-non-place-exprs.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/unsized-non-place-exprs.rs:20:19
|
LL | udrop::<[u8]>(if true { *foo() } else { *foo() });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-non-place-exprs.rs:20:19
|
LL | udrop::<[u8]>(if true { *foo() } else { *foo() });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/unsized-non-place-exprs.rs:21:19
|
LL | udrop::<[u8]>({ *foo() });
| ^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-non-place-exprs.rs:21:19
|
LL | udrop::<[u8]>({ *foo() });
| ^^^^^^^^^^

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/unsized-non-place-exprs.rs:22:11
|
LL | udrop(match foo() { x => *x });
| ^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-non-place-exprs.rs:22:11
|
LL | udrop(match foo() { x => *x });
| ^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/unsized-non-place-exprs.rs:23:19
|
LL | udrop::<[u8]>({ loop { break *foo(); } });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-non-place-exprs.rs:23:19
|
LL | udrop::<[u8]>({ loop { break *foo(); } });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/unsized-non-place-exprs.rs:25:5
|
LL | { *foo() };
| ^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-non-place-exprs.rs:25:5
|
LL | { *foo() };
| ^^^^^^^^^^

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/unsized-non-place-exprs.rs:26:5
|
LL | { loop { break *foo(); } };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
note: unsized values must be place expressions and cannot be moved from
--> $DIR/unsized-non-place-exprs.rs:26:5
|
LL | { loop { break *foo(); } };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0277`.
Loading