diff options
| author | bors <bors@rust-lang.org> | 2023-12-13 08:33:05 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-12-13 08:33:05 +0000 |
| commit | 2fdd9eda0ce4fa0ecbf3099783f4f505235ceb44 (patch) | |
| tree | 7b71f6689b6d293b08c6d48b0fb834f0da1f1dc6 | |
| parent | f651b436ce70a6f89ec1636fbedb128a473d809b (diff) | |
| parent | f813ccd7840e7aee205d39be06166e5cbdaf54e3 (diff) | |
| download | rust-2fdd9eda0ce4fa0ecbf3099783f4f505235ceb44.tar.gz rust-2fdd9eda0ce4fa0ecbf3099783f4f505235ceb44.zip | |
Auto merge of #118534 - RalfJung:extern-type-size-of-val, r=WaffleLapkin
codegen: panic when trying to compute size/align of extern type The alignment is also computed when accessing a field of extern type at non-zero offset, so we also panic in that case. Previously `size_of_val` worked because the code path there assumed that "thin pointer" means "sized". But that's not true any more with extern types. The returned size and align are just blatantly wrong, so it seems better to panic than returning wrong results. We use a non-unwinding panic since code probably does not expect size_of_val to panic.
18 files changed, 211 insertions, 162 deletions
diff --git a/compiler/rustc_codegen_cranelift/build_system/tests.rs b/compiler/rustc_codegen_cranelift/build_system/tests.rs index 3309a0a6abd..1a38d5967f4 100644 --- a/compiler/rustc_codegen_cranelift/build_system/tests.rs +++ b/compiler/rustc_codegen_cranelift/build_system/tests.rs @@ -75,11 +75,6 @@ const BASE_SYSROOT_SUITE: &[TestCase] = &[ "example/arbitrary_self_types_pointers_and_wrappers.rs", &[], ), - TestCase::build_bin_and_run( - "aot.issue_91827_extern_types", - "example/issue-91827-extern-types.rs", - &[], - ), TestCase::build_lib("build.alloc_system", "example/alloc_system.rs", "lib"), TestCase::build_bin_and_run("aot.alloc_example", "example/alloc_example.rs", &[]), TestCase::jit_bin("jit.std_example", "example/std_example.rs", ""), diff --git a/compiler/rustc_codegen_cranelift/example/issue-91827-extern-types.rs b/compiler/rustc_codegen_cranelift/example/issue-91827-extern-types.rs deleted file mode 100644 index 6f39c5edcad..00000000000 --- a/compiler/rustc_codegen_cranelift/example/issue-91827-extern-types.rs +++ /dev/null @@ -1,55 +0,0 @@ -// Copied from rustc ui test suite - -// run-pass -// -// Test that we can handle unsized types with an extern type tail part. -// Regression test for issue #91827. - -#![feature(extern_types)] - -use std::ptr::addr_of; - -extern "C" { - type Opaque; -} - -unsafe impl Sync for Opaque {} - -#[repr(C)] -pub struct List<T> { - len: usize, - data: [T; 0], - tail: Opaque, -} - -#[repr(C)] -pub struct ListImpl<T, const N: usize> { - len: usize, - data: [T; N], -} - -impl<T> List<T> { - const fn as_slice(&self) -> &[T] { - unsafe { std::slice::from_raw_parts(self.data.as_ptr(), self.len) } - } -} - -impl<T, const N: usize> ListImpl<T, N> { - const fn as_list(&self) -> &List<T> { - unsafe { std::mem::transmute(self) } - } -} - -pub static A: ListImpl<u128, 3> = ListImpl { len: 3, data: [5, 6, 7] }; -pub static A_REF: &'static List<u128> = A.as_list(); -pub static A_TAIL_OFFSET: isize = tail_offset(A.as_list()); - -const fn tail_offset<T>(list: &List<T>) -> isize { - unsafe { (addr_of!(list.tail) as *const u8).offset_from(list as *const List<T> as *const u8) } -} - -fn main() { - assert_eq!(A_REF.as_slice(), &[5, 6, 7]); - // Check that interpreter and code generation agree about the position of the tail field. - assert_eq!(A_TAIL_OFFSET, tail_offset(A_REF)); -} diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index cd5eb77e06e..9b60f0844a0 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -51,10 +51,10 @@ pub mod codegen_attrs; pub mod common; pub mod debuginfo; pub mod errors; -pub mod glue; pub mod meth; pub mod mir; pub mod mono_item; +pub mod size_of_val; pub mod target_features; pub mod traits; diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 136d06d561a..a5bffc33d39 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -4,8 +4,8 @@ use super::FunctionCx; use crate::common::IntPredicate; use crate::errors; use crate::errors::InvalidMonomorphization; -use crate::glue; use crate::meth; +use crate::size_of_val; use crate::traits::*; use crate::MemFlags; @@ -88,21 +88,17 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { sym::va_end => bx.va_end(args[0].immediate()), sym::size_of_val => { let tp_ty = fn_args.type_at(0); - if let OperandValue::Pair(_, meta) = args[0].val { - let (llsize, _) = glue::size_and_align_of_dst(bx, tp_ty, Some(meta)); - llsize - } else { - bx.const_usize(bx.layout_of(tp_ty).size.bytes()) - } + let meta = + if let OperandValue::Pair(_, meta) = args[0].val { Some(meta) } else { None }; + let (llsize, _) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta); + llsize } sym::min_align_of_val => { let tp_ty = fn_args.type_at(0); - if let OperandValue::Pair(_, meta) = args[0].val { - let (_, llalign) = glue::size_and_align_of_dst(bx, tp_ty, Some(meta)); - llalign - } else { - bx.const_usize(bx.layout_of(tp_ty).align.abi.bytes()) - } + let meta = + if let OperandValue::Pair(_, meta) = args[0].val { Some(meta) } else { None }; + let (_, llalign) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta); + llalign } sym::vtable_size | sym::vtable_align => { let vtable = args[0].immediate(); diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index feee3ac03d0..e8c58f6b6f8 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -2,7 +2,7 @@ use super::place::PlaceRef; use super::{FunctionCx, LocalRef}; use crate::base; -use crate::glue; +use crate::size_of_val; use crate::traits::*; use crate::MemFlags; @@ -466,13 +466,13 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> { .ty; let OperandValue::Ref(llptr, Some(llextra), _) = self else { - bug!("store_unsized called with a sized value") + 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) = glue::size_and_align_of_dst(bx, unsized_ty, Some(llextra)); + 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); diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs index 83425dee1a8..c0bb3ac5661 100644 --- a/compiler/rustc_codegen_ssa/src/mir/place.rs +++ b/compiler/rustc_codegen_ssa/src/mir/place.rs @@ -2,7 +2,7 @@ use super::operand::OperandValue; use super::{FunctionCx, LocalRef}; use crate::common::IntPredicate; -use crate::glue; +use crate::size_of_val; use crate::traits::*; use rustc_middle::mir; @@ -99,6 +99,8 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> { let offset = self.layout.fields.offset(ix); let effective_field_align = self.align.restrict_for_offset(offset); + // `simple` is called when we don't need to adjust the offset to + // the dynamic alignment of the field. let mut simple = || { let llval = match self.layout.abi { _ if offset.bytes() == 0 => { @@ -141,28 +143,21 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> { }; // Simple cases, which don't need DST adjustment: - // * no metadata available - just log the case - // * known alignment - sized types, `[T]`, `str` or a foreign type + // * known alignment - sized types, `[T]`, `str` + // * offset 0 -- rounding up to alignment cannot change the offset // Note that looking at `field.align` is incorrect since that is not necessarily equal // to the dynamic alignment of the type. match field.ty.kind() { - _ if self.llextra.is_none() => { - debug!( - "unsized field `{}`, of `{:?}` has no metadata for adjustment", - ix, self.llval - ); - return simple(); - } _ if field.is_sized() => return simple(), - ty::Slice(..) | ty::Str | ty::Foreign(..) => return simple(), + ty::Slice(..) | ty::Str => return simple(), + _ if offset.bytes() == 0 => return simple(), _ => {} } // We need to get the pointer manually now. // We do this by casting to a `*i8`, then offsetting it by the appropriate amount. // We do this instead of, say, simply adjusting the pointer from the result of a GEP - // because the field may have an arbitrary alignment in the LLVM representation - // anyway. + // because the field may have an arbitrary alignment in the LLVM representation. // // To demonstrate: // @@ -179,7 +174,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> { let unaligned_offset = bx.cx().const_usize(offset.bytes()); // Get the alignment of the field - let (_, mut unsized_align) = glue::size_and_align_of_dst(bx, field.ty, meta); + let (_, mut unsized_align) = size_of_val::size_and_align_of_dst(bx, field.ty, meta); // For packed types, we need to cap alignment. if let ty::Adt(def, _) = self.layout.ty.kind() diff --git a/compiler/rustc_codegen_ssa/src/glue.rs b/compiler/rustc_codegen_ssa/src/size_of_val.rs index c34f1dbf856..b8a4949d59f 100644 --- a/compiler/rustc_codegen_ssa/src/glue.rs +++ b/compiler/rustc_codegen_ssa/src/size_of_val.rs @@ -1,10 +1,11 @@ -//! -// -// Code relating to drop glue. +//! Computing the size and alignment of a value. +use crate::common; use crate::common::IntPredicate; use crate::meth; use crate::traits::*; +use rustc_hir::LangItem; +use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths}; use rustc_middle::ty::{self, Ty}; use rustc_target::abi::WrappingRange; @@ -14,7 +15,7 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( info: Option<Bx::Value>, ) -> (Bx::Value, Bx::Value) { let layout = bx.layout_of(t); - debug!("size_and_align_of_dst(ty={}, info={:?}): layout: {:?}", t, info, layout); + trace!("size_and_align_of_dst(ty={}, info={:?}): layout: {:?}", t, info, layout); if layout.is_sized() { let size = bx.const_usize(layout.size.bytes()); let align = bx.const_usize(layout.align.abi.bytes()); @@ -51,7 +52,31 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( bx.const_usize(unit.align.abi.bytes()), ) } - _ => { + ty::Foreign(_) => { + // `extern` type. We cannot compute the size, so panic. + let msg_str = with_no_visible_paths!({ + with_no_trimmed_paths!({ + format!("attempted to compute the size or alignment of extern type `{t}`") + }) + }); + let msg = bx.const_str(&msg_str); + + // Obtain the panic entry point. + let (fn_abi, llfn) = common::build_langcall(bx, None, LangItem::PanicNounwind); + + // Generate the call. + // Cannot use `do_call` since we don't have a MIR terminator so we can't create a `TerminationCodegenHelper`. + // (But we are in good company, this code is duplicated plenty of times.) + let fn_ty = bx.fn_decl_backend_type(fn_abi); + + bx.call(fn_ty, /* fn_attrs */ None, Some(fn_abi), llfn, &[msg.0, msg.1], None); + + // This function does not return so we can now return whatever we want. + let size = bx.const_usize(layout.size.bytes()); + let align = bx.const_usize(layout.align.abi.bytes()); + (size, align) + } + ty::Adt(..) | ty::Tuple(..) => { // First get the size of all statically known fields. // Don't use size_of because it also rounds up to alignment, which we // want to avoid, as the unsized field's alignment could be smaller. @@ -122,5 +147,6 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( (size, align) } + _ => bug!("size_and_align_of_dst: {t} not supported"), } } diff --git a/compiler/rustc_const_eval/src/interpret/projection.rs b/compiler/rustc_const_eval/src/interpret/projection.rs index 4d9e296d544..0f3b6b25c61 100644 --- a/compiler/rustc_const_eval/src/interpret/projection.rs +++ b/compiler/rustc_const_eval/src/interpret/projection.rs @@ -174,12 +174,15 @@ where }; (base_meta, offset.align_to(align)) } - None => { - // For unsized types with an extern type tail we perform no adjustments. - // NOTE: keep this in sync with `PlaceRef::project_field` in the codegen backend. - assert!(matches!(base_meta, MemPlaceMeta::None)); + None if offset == Size::ZERO => { + // If the offset is 0, then rounding it up to alignment wouldn't change anything, + // so we can do this even for types where we cannot determine the alignment. (base_meta, offset) } + None => { + // We don't know the alignment of this field, so we cannot adjust. + throw_unsup_format!("`extern type` does not have a known offset") + } } } else { // base_meta could be present; we might be accessing a sized field of an unsized diff --git a/src/tools/miri/tests/fail/extern-type-field-offset.rs b/src/tools/miri/tests/fail/extern-type-field-offset.rs new file mode 100644 index 00000000000..139405fc374 --- /dev/null +++ b/src/tools/miri/tests/fail/extern-type-field-offset.rs @@ -0,0 +1,29 @@ +#![feature(extern_types)] + +extern "C" { + type Opaque; +} + +struct Newtype(Opaque); + +struct S { + i: i32, + j: i32, + a: Newtype, +} + +fn main() { + let buf = [0i32; 4]; + + let x: &Newtype = unsafe { &*(&buf as *const _ as *const Newtype) }; + // Projecting to the newtype works, because it is always at offset 0. + let _field = &x.0; + + let x: &S = unsafe { &*(&buf as *const _ as *const S) }; + // Accessing sized fields is perfectly fine, even at non-zero offsets. + let _field = &x.i; + let _field = &x.j; + // This needs to compute the field offset, but we don't know the type's alignment, + // so this panics. + let _field = &x.a; //~ERROR: does not have a known offset +} diff --git a/src/tools/miri/tests/fail/extern-type-field-offset.stderr b/src/tools/miri/tests/fail/extern-type-field-offset.stderr new file mode 100644 index 00000000000..cbbf1b38361 --- /dev/null +++ b/src/tools/miri/tests/fail/extern-type-field-offset.stderr @@ -0,0 +1,14 @@ +error: unsupported operation: `extern type` does not have a known offset + --> $DIR/extern-type-field-offset.rs:LL:CC + | +LL | let _field = &x.a; + | ^^^^ `extern type` does not have a known offset + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/extern-type-field-offset.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/tests/ui/consts/const-eval/issue-91827-extern-types-field-offset.rs b/tests/ui/consts/const-eval/issue-91827-extern-types-field-offset.rs new file mode 100644 index 00000000000..c6960fa7259 --- /dev/null +++ b/tests/ui/consts/const-eval/issue-91827-extern-types-field-offset.rs @@ -0,0 +1,43 @@ +// Test that we can handle unsized types with an extern type tail part. +// Regression test for issue #91827. + +#![feature(extern_types)] + +use std::ptr::addr_of; + +extern "C" { + type Opaque; +} + +struct Newtype(Opaque); + +struct S { + i: i32, + j: i32, + a: Newtype, +} + +const NEWTYPE: () = unsafe { + let buf = [0i32; 4]; + let x: &Newtype = &*(&buf as *const _ as *const Newtype); + + // Projecting to the newtype works, because it is always at offset 0. + let field = &x.0; +}; + +const OFFSET: () = unsafe { + let buf = [0i32; 4]; + let x: &S = &*(&buf as *const _ as *const S); + + // Accessing sized fields is perfectly fine, even at non-zero offsets. + let field = &x.i; + let field = &x.j; + + // This needs to compute the field offset, but we don't know the type's alignment, so this + // fails. + let field = &x.a; + //~^ ERROR: evaluation of constant value failed + //~| does not have a known offset +}; + +fn main() {} diff --git a/tests/ui/consts/const-eval/issue-91827-extern-types-field-offset.stderr b/tests/ui/consts/const-eval/issue-91827-extern-types-field-offset.stderr new file mode 100644 index 00000000000..99f37fedd3d --- /dev/null +++ b/tests/ui/consts/const-eval/issue-91827-extern-types-field-offset.stderr @@ -0,0 +1,9 @@ +error[E0080]: evaluation of constant value failed + --> $DIR/issue-91827-extern-types-field-offset.rs:38:17 + | +LL | let field = &x.a; + | ^^^^ `extern type` does not have a known offset + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/const-eval/issue-91827-extern-types.rs b/tests/ui/consts/const-eval/issue-91827-extern-types.rs deleted file mode 100644 index c9aaa6e5587..00000000000 --- a/tests/ui/consts/const-eval/issue-91827-extern-types.rs +++ /dev/null @@ -1,59 +0,0 @@ -// run-pass -// -// Test that we can handle unsized types with an extern type tail part. -// Regression test for issue #91827. - -#![feature(extern_types)] - -use std::ptr::addr_of; - -extern "C" { - type Opaque; -} - -unsafe impl Sync for Opaque {} - -#[repr(C)] -pub struct List<T> { - len: usize, - data: [T; 0], - tail: Opaque, -} - -#[repr(C)] -pub struct ListImpl<T, const N: usize> { - len: usize, - data: [T; N], -} - -impl<T> List<T> { - const fn as_slice(&self) -> &[T] { - unsafe { - let ptr = addr_of!(self.tail) as *const T; - std::slice::from_raw_parts(ptr, self.len) - } - } -} - -impl<T, const N: usize> ListImpl<T, N> { - const fn as_list(&self) -> &List<T> { - unsafe { std::mem::transmute(self) } - } -} - -pub static A: ListImpl<u128, 3> = ListImpl { - len: 3, - data: [5, 6, 7], -}; -pub static A_REF: &'static List<u128> = A.as_list(); -pub static A_TAIL_OFFSET: isize = tail_offset(A.as_list()); - -const fn tail_offset<T>(list: &List<T>) -> isize { - unsafe { (addr_of!(list.tail) as *const u8).offset_from(list as *const List<T> as *const u8) } -} - -fn main() { - assert_eq!(A_REF.as_slice(), &[5, 6, 7]); - // Check that interpreter and code generation agree about the position of the tail field. - assert_eq!(A_TAIL_OFFSET, tail_offset(A_REF)); -} diff --git a/tests/ui/extern/extern-types-field-offset.rs b/tests/ui/extern/extern-types-field-offset.rs new file mode 100644 index 00000000000..bfbc1e9bffa --- /dev/null +++ b/tests/ui/extern/extern-types-field-offset.rs @@ -0,0 +1,33 @@ +// run-fail +// check-run-results +// exec-env:RUST_BACKTRACE=0 +// normalize-stderr-test: "(core/src/panicking\.rs):[0-9]+:[0-9]+" -> "$1:$$LINE:$$COL" +#![feature(extern_types)] + +extern "C" { + type Opaque; +} + +struct Newtype(Opaque); + +struct S { + i: i32, + j: i32, + a: Newtype, +} + +fn main() { + let buf = [0i32; 4]; + + let x: &Newtype = unsafe { &*(&buf as *const _ as *const Newtype) }; + // Projecting to the newtype works, because it is always at offset 0. + let field = &x.0; + + let x: &S = unsafe { &*(&buf as *const _ as *const S) }; + // Accessing sized fields is perfectly fine, even at non-zero offsets. + let field = &x.i; + let field = &x.j; + // This needs to compute the field offset, but we don't know the type's alignment, + // so this panics. + let field = &x.a; +} diff --git a/tests/ui/extern/extern-types-field-offset.run.stderr b/tests/ui/extern/extern-types-field-offset.run.stderr new file mode 100644 index 00000000000..1b04b860db5 --- /dev/null +++ b/tests/ui/extern/extern-types-field-offset.run.stderr @@ -0,0 +1,4 @@ +thread 'main' panicked at library/core/src/panicking.rs:$LINE:$COL: +attempted to compute the size or alignment of extern type `Opaque` +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +thread caused non-unwinding panic. aborting. diff --git a/tests/ui/extern/extern-types-size_of_val.align.run.stderr b/tests/ui/extern/extern-types-size_of_val.align.run.stderr new file mode 100644 index 00000000000..20c4d8785e8 --- /dev/null +++ b/tests/ui/extern/extern-types-size_of_val.align.run.stderr @@ -0,0 +1,4 @@ +thread 'main' panicked at library/core/src/panicking.rs:$LINE:$COL: +attempted to compute the size or alignment of extern type `A` +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +thread caused non-unwinding panic. aborting. diff --git a/tests/ui/extern/extern-types-size_of_val.rs b/tests/ui/extern/extern-types-size_of_val.rs index 3b02ea28eaa..4c4de873b7f 100644 --- a/tests/ui/extern/extern-types-size_of_val.rs +++ b/tests/ui/extern/extern-types-size_of_val.rs @@ -1,4 +1,8 @@ -// run-pass +// run-fail +// check-run-results +// exec-env:RUST_BACKTRACE=0 +// normalize-stderr-test: "(core/src/panicking\.rs):[0-9]+:[0-9]+" -> "$1:$$LINE:$$COL" +// revisions: size align #![feature(extern_types)] use std::mem::{align_of_val, size_of_val}; @@ -10,6 +14,10 @@ extern "C" { fn main() { let x: &A = unsafe { &*(1usize as *const A) }; - assert_eq!(size_of_val(x), 0); - assert_eq!(align_of_val(x), 1); + // These don't have a dynamic size, so this should panic. + if cfg!(size) { + assert_eq!(size_of_val(x), 0); + } else { + assert_eq!(align_of_val(x), 1); + } } diff --git a/tests/ui/extern/extern-types-size_of_val.size.run.stderr b/tests/ui/extern/extern-types-size_of_val.size.run.stderr new file mode 100644 index 00000000000..20c4d8785e8 --- /dev/null +++ b/tests/ui/extern/extern-types-size_of_val.size.run.stderr @@ -0,0 +1,4 @@ +thread 'main' panicked at library/core/src/panicking.rs:$LINE:$COL: +attempted to compute the size or alignment of extern type `A` +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +thread caused non-unwinding panic. aborting. |
