Skip to content

Commit

Permalink
Auto merge of #122053 - erikdesjardins:alloca, r=nikic
Browse files Browse the repository at this point in the history
Stop using LLVM struct types for alloca

The alloca type has no semantic meaning, only the size (and alignment, but we specify it explicitly) matter. Using `[N x i8]` is a more direct way to specify that we want `N` bytes, and avoids relying on LLVM's struct layout. It is likely that a future LLVM version will change to an untyped alloca representation.

Split out from #121577.

r? `@ghost`
  • Loading branch information
bors committed Apr 24, 2024
2 parents c1feb3e + 6df27ef commit 29a56a3
Show file tree
Hide file tree
Showing 36 changed files with 234 additions and 226 deletions.
14 changes: 4 additions & 10 deletions compiler/rustc_codegen_gcc/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,26 +898,20 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
self.gcc_checked_binop(oop, typ, lhs, rhs)
}

fn alloca(&mut self, ty: Type<'gcc>, align: Align) -> RValue<'gcc> {
// FIXME(antoyo): this check that we don't call get_aligned() a second time on a type.
// Ideally, we shouldn't need to do this check.
let aligned_type = if ty == self.cx.u128_type || ty == self.cx.i128_type {
ty
} else {
ty.get_aligned(align.bytes())
};
fn alloca(&mut self, size: Size, align: Align) -> RValue<'gcc> {
let ty = self.cx.type_array(self.cx.type_i8(), size.bytes()).get_aligned(align.bytes());
// TODO(antoyo): It might be better to return a LValue, but fixing the rustc API is non-trivial.
self.stack_var_count.set(self.stack_var_count.get() + 1);
self.current_func()
.new_local(
self.location,
aligned_type,
ty,
&format!("stack_var_{}", self.stack_var_count.get()),
)
.get_address(self.location)
}

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

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ impl<'gcc, 'tcx> ArgAbiExt<'gcc, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
// We instead thus allocate some scratch space...
let scratch_size = cast.size(bx);
let scratch_align = cast.align(bx);
let llscratch = bx.alloca(cast.gcc_type(bx), scratch_align);
let llscratch = bx.alloca(scratch_size, scratch_align);
bx.lifetime_start(llscratch, scratch_size);

// ... where we first store the value...
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_gcc/src/intrinsic/simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_middle::span_bug;
use rustc_middle::ty::layout::HasTyCtxt;
use rustc_middle::ty::{self, Ty};
use rustc_span::{sym, Span, Symbol};
use rustc_target::abi::Align;
use rustc_target::abi::{Align, Size};

use crate::builder::Builder;
#[cfg(not(feature = "master"))]
Expand Down Expand Up @@ -558,7 +558,7 @@ pub fn generic_simd_intrinsic<'a, 'gcc, 'tcx>(
let ze = bx.zext(result, bx.type_ix(expected_bytes * 8));

// Convert the integer to a byte array
let ptr = bx.alloca(bx.type_ix(expected_bytes * 8), Align::ONE);
let ptr = bx.alloca(Size::from_bytes(expected_bytes), Align::ONE);
bx.store(ze, ptr, Align::ONE);
let array_ty = bx.type_array(bx.type_i8(), expected_bytes);
let ptr = bx.pointercast(ptr, bx.cx.type_ptr_to(array_ty));
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl<'ll, 'tcx> ArgAbiExt<'ll, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
// when passed by value, making it larger.
let copy_bytes = cmp::min(scratch_size.bytes(), self.layout.size.bytes());
// Allocate some scratch space...
let llscratch = bx.alloca(cast.llvm_type(bx), scratch_align);
let llscratch = bx.alloca(scratch_size, scratch_align);
bx.lifetime_start(llscratch, scratch_size);
// ...store the value...
bx.store(val, llscratch, scratch_align);
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,20 +468,21 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
val
}

fn alloca(&mut self, ty: &'ll Type, align: Align) -> &'ll Value {
fn alloca(&mut self, size: Size, align: Align) -> &'ll Value {
let mut bx = Builder::with_cx(self.cx);
bx.position_at_start(unsafe { llvm::LLVMGetFirstBasicBlock(self.llfn()) });
let ty = self.cx().type_array(self.cx().type_i8(), size.bytes());
unsafe {
let alloca = llvm::LLVMBuildAlloca(bx.llbuilder, ty, UNNAMED);
llvm::LLVMSetAlignment(alloca, align.bytes() as c_uint);
alloca
}
}

fn byte_array_alloca(&mut self, len: &'ll Value, align: Align) -> &'ll Value {
fn dynamic_alloca(&mut self, size: &'ll Value, align: Align) -> &'ll Value {
unsafe {
let alloca =
llvm::LLVMBuildArrayAlloca(self.llbuilder, self.cx().type_i8(), len, UNNAMED);
llvm::LLVMBuildArrayAlloca(self.llbuilder, self.cx().type_i8(), size, UNNAMED);
llvm::LLVMSetAlignment(alloca, align.bytes() as c_uint);
alloca
}
Expand Down
20 changes: 10 additions & 10 deletions compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt, LayoutOf};
use rustc_middle::ty::{self, GenericArgsRef, Ty};
use rustc_middle::{bug, span_bug};
use rustc_span::{sym, Span, Symbol};
use rustc_target::abi::{self, Align, HasDataLayout, Primitive};
use rustc_target::abi::{self, Align, HasDataLayout, Primitive, Size};
use rustc_target::spec::{HasTargetSpec, PanicStrategy};

use std::cmp::Ordering;
Expand Down Expand Up @@ -649,8 +649,9 @@ fn codegen_msvc_try<'ll>(
// }
//
// More information can be found in libstd's seh.rs implementation.
let ptr_size = bx.tcx().data_layout.pointer_size;
let ptr_align = bx.tcx().data_layout.pointer_align.abi;
let slot = bx.alloca(bx.type_ptr(), ptr_align);
let slot = bx.alloca(ptr_size, ptr_align);
let try_func_ty = bx.type_func(&[bx.type_ptr()], bx.type_void());
bx.invoke(try_func_ty, None, None, try_func, &[data], normal, catchswitch, None, None);

Expand Down Expand Up @@ -920,15 +921,14 @@ fn codegen_emcc_try<'ll>(

// We need to pass two values to catch_func (ptr and is_rust_panic), so
// create an alloca and pass a pointer to that.
let ptr_size = bx.tcx().data_layout.pointer_size;
let ptr_align = bx.tcx().data_layout.pointer_align.abi;
let i8_align = bx.tcx().data_layout.i8_align.abi;
let catch_data_type = bx.type_struct(&[bx.type_ptr(), bx.type_bool()], false);
let catch_data = bx.alloca(catch_data_type, ptr_align);
let catch_data_0 =
bx.inbounds_gep(catch_data_type, catch_data, &[bx.const_usize(0), bx.const_usize(0)]);
bx.store(ptr, catch_data_0, ptr_align);
let catch_data_1 =
bx.inbounds_gep(catch_data_type, catch_data, &[bx.const_usize(0), bx.const_usize(1)]);
// Required in order for there to be no padding between the fields.
assert!(i8_align <= ptr_align);
let catch_data = bx.alloca(2 * ptr_size, ptr_align);
bx.store(ptr, catch_data, ptr_align);
let catch_data_1 = bx.inbounds_ptradd(catch_data, bx.const_usize(ptr_size.bytes()));
bx.store(is_rust_panic, catch_data_1, i8_align);

let catch_ty = bx.type_func(&[bx.type_ptr(), bx.type_ptr()], bx.type_void());
Expand Down Expand Up @@ -1374,7 +1374,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
let ze = bx.zext(i_, bx.type_ix(expected_bytes * 8));

// Convert the integer to a byte array
let ptr = bx.alloca(bx.type_ix(expected_bytes * 8), Align::ONE);
let ptr = bx.alloca(Size::from_bytes(expected_bytes), Align::ONE);
bx.store(ze, ptr, Align::ONE);
let array_ty = bx.type_array(bx.type_i8(), expected_bytes);
return Ok(bx.load(array_ty, ptr, Align::ONE));
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ fn get_argc_argv<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
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), ptr_align);
let arg_argv = bx.alloca(2 * ptr_size, 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);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1517,7 +1517,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// when passed by value, making it larger.
let copy_bytes = cmp::min(scratch_size.bytes(), arg.layout.size.bytes());
// Allocate some scratch space...
let llscratch = bx.alloca(bx.cast_backend_type(cast), scratch_align);
let llscratch = bx.alloca(scratch_size, scratch_align);
bx.lifetime_start(llscratch, scratch_size);
// ...memcpy the value...
bx.memcpy(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
let llfield_ty = bx.cx().backend_type(field);

// Can't bitcast an aggregate, so round trip through memory.
let llptr = bx.alloca(llfield_ty, field.align.abi);
let llptr = bx.alloca(field.size, field.align.abi);
bx.store(*llval, llptr, field.align.abi);
*llval = bx.load(llfield_ty, llptr, field.align.abi);
}
Expand Down Expand Up @@ -470,7 +470,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
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.byte_array_alloca(size_extra, min_align);
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);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
align: Align,
) -> Self {
assert!(layout.is_sized(), "tried to statically allocate unsized place");
let tmp = bx.alloca(bx.cx().backend_type(layout), align);
let tmp = bx.alloca(layout.size, align);
Self::new_sized_aligned(tmp, layout, align)
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/traits/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ pub trait BuilderMethods<'a, 'tcx>:
}
fn to_immediate_scalar(&mut self, val: Self::Value, scalar: Scalar) -> Self::Value;

fn alloca(&mut self, ty: Self::Type, align: Align) -> Self::Value;
fn byte_array_alloca(&mut self, len: Self::Value, align: Align) -> 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
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,9 @@ pub fn array_char(f: fn(*const char)) {
f(&b as *const _);
f(&c as *const _);

// Any type of local array variable leads to stack protection with the
// "strong" heuristic. The 'basic' heuristic only adds stack protection to
// functions with local array variables of a byte-sized type, however. Since
// 'char' is 4 bytes in Rust, this function is not protected by the 'basic'
// heuristic
//
// (This test *also* takes the address of the local stack variables. We
// cannot know that this isn't what triggers the `strong` heuristic.
// However, the test strategy of passing the address of a stack array to an
// external function is sufficient to trigger the `basic` heuristic (see
// test `array_u8_large()`). Since the `basic` heuristic only checks for the
// presence of stack-local array variables, we can be confident that this
// test also captures this part of the `strong` heuristic specification.)

// all: __security_check_cookie
// strong: __security_check_cookie
// basic-NOT: __security_check_cookie
// basic: __security_check_cookie
// none-NOT: __security_check_cookie
// missing-NOT: __security_check_cookie
}
Expand Down Expand Up @@ -231,8 +217,8 @@ pub fn local_large_var_moved(f: fn(Gigastruct)) {
// Even though the local variable conceptually doesn't have its address
// taken, it's so large that the "move" is implemented with a reference to a
// stack-local variable in the ABI. Consequently, this function *is*
// protected by the `strong` heuristic. This is also the case for
// rvalue-references in C++, regardless of struct size:
// protected. This is also the case for rvalue-references in C++,
// regardless of struct size:
// ```
// cat <<EOF | clang++ -O2 -fstack-protector-strong -S -x c++ - -o - | grep stack_chk
// #include <cstdint>
Expand All @@ -246,7 +232,7 @@ pub fn local_large_var_moved(f: fn(Gigastruct)) {

// all: __security_check_cookie
// strong: __security_check_cookie
// basic-NOT: __security_check_cookie
// basic: __security_check_cookie
// none-NOT: __security_check_cookie
// missing-NOT: __security_check_cookie
}
Expand All @@ -259,9 +245,9 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) {
// A new instance of `Gigastruct` is passed to `f()`, without any apparent
// connection to this stack frame. Still, since instances of `Gigastruct`
// are sufficiently large, it is allocated in the caller stack frame and
// passed as a pointer. As such, this function is *also* protected by the
// `strong` heuristic, just like `local_large_var_moved`. This is also the
// case for pass-by-value of sufficiently large structs in C++:
// passed as a pointer. As such, this function is *also* protected, just
// like `local_large_var_moved`. This is also the case for pass-by-value
// of sufficiently large structs in C++:
// ```
// cat <<EOF | clang++ -O2 -fstack-protector-strong -S -x c++ - -o - | grep stack_chk
// #include <cstdint>
Expand All @@ -276,7 +262,7 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) {

// all: __security_check_cookie
// strong: __security_check_cookie
// basic-NOT: __security_check_cookie
// basic: __security_check_cookie
// none-NOT: __security_check_cookie
// missing-NOT: __security_check_cookie
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,9 @@ pub fn array_char(f: fn(*const char)) {
f(&b as *const _);
f(&c as *const _);

// Any type of local array variable leads to stack protection with the
// "strong" heuristic. The 'basic' heuristic only adds stack protection to
// functions with local array variables of a byte-sized type, however. Since
// 'char' is 4 bytes in Rust, this function is not protected by the 'basic'
// heuristic
//
// (This test *also* takes the address of the local stack variables. We
// cannot know that this isn't what triggers the `strong` heuristic.
// However, the test strategy of passing the address of a stack array to an
// external function is sufficient to trigger the `basic` heuristic (see
// test `array_u8_large()`). Since the `basic` heuristic only checks for the
// presence of stack-local array variables, we can be confident that this
// test also captures this part of the `strong` heuristic specification.)

// all: __security_check_cookie
// strong: __security_check_cookie
// basic-NOT: __security_check_cookie
// basic: __security_check_cookie
// none-NOT: __security_check_cookie
// missing-NOT: __security_check_cookie
}
Expand Down Expand Up @@ -239,8 +225,8 @@ pub fn local_large_var_moved(f: fn(Gigastruct)) {
// Even though the local variable conceptually doesn't have its address
// taken, it's so large that the "move" is implemented with a reference to a
// stack-local variable in the ABI. Consequently, this function *is*
// protected by the `strong` heuristic. This is also the case for
// rvalue-references in C++, regardless of struct size:
// protected. This is also the case for rvalue-references in C++,
// regardless of struct size:
// ```
// cat <<EOF | clang++ -O2 -fstack-protector-strong -S -x c++ - -o - | grep stack_chk
// #include <cstdint>
Expand All @@ -254,7 +240,7 @@ pub fn local_large_var_moved(f: fn(Gigastruct)) {

// all: __security_check_cookie
// strong: __security_check_cookie
// basic-NOT: __security_check_cookie
// basic: __security_check_cookie
// none-NOT: __security_check_cookie
// missing-NOT: __security_check_cookie
}
Expand All @@ -267,9 +253,9 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) {
// A new instance of `Gigastruct` is passed to `f()`, without any apparent
// connection to this stack frame. Still, since instances of `Gigastruct`
// are sufficiently large, it is allocated in the caller stack frame and
// passed as a pointer. As such, this function is *also* protected by the
// `strong` heuristic, just like `local_large_var_moved`. This is also the
// case for pass-by-value of sufficiently large structs in C++:
// passed as a pointer. As such, this function is *also* protected, just
// like `local_large_var_moved`. This is also the case for pass-by-value
// of sufficiently large structs in C++:
// ```
// cat <<EOF | clang++ -O2 -fstack-protector-strong -S -x c++ - -o - | grep stack_chk
// #include <cstdint>
Expand All @@ -284,7 +270,7 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) {

// all: __security_check_cookie
// strong: __security_check_cookie
// basic-NOT: __security_check_cookie
// basic: __security_check_cookie
// none-NOT: __security_check_cookie
// missing-NOT: __security_check_cookie
}
Expand Down
Loading

0 comments on commit 29a56a3

Please sign in to comment.