Skip to content

Commit 2fdd9ed

Browse files
committed
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.
2 parents f651b43 + f813ccd commit 2fdd9ed

18 files changed

Lines changed: 211 additions & 162 deletions

‎compiler/rustc_codegen_cranelift/build_system/tests.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,6 @@ const BASE_SYSROOT_SUITE: &[TestCase] = &[
7575
"example/arbitrary_self_types_pointers_and_wrappers.rs",
7676
&[],
7777
),
78-
TestCase::build_bin_and_run(
79-
"aot.issue_91827_extern_types",
80-
"example/issue-91827-extern-types.rs",
81-
&[],
82-
),
8378
TestCase::build_lib("build.alloc_system","example/alloc_system.rs","lib"),
8479
TestCase::build_bin_and_run("aot.alloc_example","example/alloc_example.rs",&[]),
8580
TestCase::jit_bin("jit.std_example","example/std_example.rs",""),

‎compiler/rustc_codegen_cranelift/example/issue-91827-extern-types.rs‎

Lines changed: 0 additions & 55 deletions
This file was deleted.

‎compiler/rustc_codegen_ssa/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ pub mod codegen_attrs;
5151
pubmod common;
5252
pubmod debuginfo;
5353
pubmod errors;
54-
pubmod glue;
5554
pubmod meth;
5655
pubmod mir;
5756
pubmod mono_item;
57+
pubmod size_of_val;
5858
pubmod target_features;
5959
pubmod traits;
6060

‎compiler/rustc_codegen_ssa/src/mir/intrinsic.rs‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use super::FunctionCx;
44
usecrate::common::IntPredicate;
55
usecrate::errors;
66
usecrate::errors::InvalidMonomorphization;
7-
usecrate::glue;
87
usecrate::meth;
8+
usecrate::size_of_val;
99
usecrate::traits::*;
1010
usecrate::MemFlags;
1111

@@ -88,21 +88,17 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
8888
sym::va_end => bx.va_end(args[0].immediate()),
8989
sym::size_of_val => {
9090
let tp_ty = fn_args.type_at(0);
91-
ifletOperandValue::Pair(_, meta) = args[0].val{
92-
let(llsize, _) = glue::size_and_align_of_dst(bx, tp_ty,Some(meta));
93-
llsize
94-
}else{
95-
bx.const_usize(bx.layout_of(tp_ty).size.bytes())
96-
}
91+
let meta =
92+
ifletOperandValue::Pair(_, meta) = args[0].val{Some(meta)}else{None};
93+
let(llsize, _) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta);
94+
llsize
9795
}
9896
sym::min_align_of_val => {
9997
let tp_ty = fn_args.type_at(0);
100-
ifletOperandValue::Pair(_, meta) = args[0].val{
101-
let(_, llalign) = glue::size_and_align_of_dst(bx, tp_ty,Some(meta));
102-
llalign
103-
}else{
104-
bx.const_usize(bx.layout_of(tp_ty).align.abi.bytes())
105-
}
98+
let meta =
99+
ifletOperandValue::Pair(_, meta) = args[0].val{Some(meta)}else{None};
100+
let(_, llalign) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta);
101+
llalign
106102
}
107103
sym::vtable_size | sym::vtable_align => {
108104
let vtable = args[0].immediate();

‎compiler/rustc_codegen_ssa/src/mir/operand.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::place::PlaceRef;
22
usesuper::{FunctionCx,LocalRef};
33

44
usecrate::base;
5-
usecrate::glue;
5+
usecrate::size_of_val;
66
usecrate::traits::*;
77
usecrate::MemFlags;
88

@@ -466,13 +466,13 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
466466
.ty;
467467

468468
letOperandValue::Ref(llptr,Some(llextra), _) = selfelse{
469-
bug!("store_unsized called with a sized value")
469+
bug!("store_unsized called with a sized value (or with an extern type)")
470470
};
471471

472472
// Allocate an appropriate region on the stack, and copy the value into it. Since alloca
473473
// doesn't support dynamic alignment, we allocate an extra align - 1 bytes, and align the
474474
// pointer manually.
475-
let(size, align) = glue::size_and_align_of_dst(bx, unsized_ty,Some(llextra));
475+
let(size, align) = size_of_val::size_and_align_of_dst(bx, unsized_ty,Some(llextra));
476476
let one = bx.const_usize(1);
477477
let align_minus_1 = bx.sub(align, one);
478478
let size_extra = bx.add(size, align_minus_1);

‎compiler/rustc_codegen_ssa/src/mir/place.rs‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::operand::OperandValue;
22
usesuper::{FunctionCx,LocalRef};
33

44
usecrate::common::IntPredicate;
5-
usecrate::glue;
5+
usecrate::size_of_val;
66
usecrate::traits::*;
77

88
use rustc_middle::mir;
@@ -99,6 +99,8 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
9999
let offset = self.layout.fields.offset(ix);
100100
let effective_field_align = self.align.restrict_for_offset(offset);
101101

102+
// `simple` is called when we don't need to adjust the offset to
103+
// the dynamic alignment of the field.
102104
letmut simple = || {
103105
let llval = matchself.layout.abi{
104106
_ if offset.bytes() == 0 => {
@@ -141,28 +143,21 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
141143
};
142144

143145
// Simple cases, which don't need DST adjustment:
144-
// * no metadata available - just log the case
145-
// * known alignment - sized types, `[T]`, `str` or a foreign type
146+
// * known alignment - sized types, `[T]`, `str`
147+
// * offset 0 -- rounding up to alignment cannot change the offset
146148
// Note that looking at `field.align` is incorrect since that is not necessarily equal
147149
// to the dynamic alignment of the type.
148150
match field.ty.kind(){
149-
_ ifself.llextra.is_none() => {
150-
debug!(
151-
"unsized field `{}`, of `{:?}` has no metadata for adjustment",
152-
ix,self.llval
153-
);
154-
returnsimple();
155-
}
156151
_ if field.is_sized() => returnsimple(),
157-
ty::Slice(..) | ty::Str | ty::Foreign(..) => returnsimple(),
152+
ty::Slice(..) | ty::Str => returnsimple(),
153+
_ if offset.bytes() == 0 => returnsimple(),
158154
_ => {}
159155
}
160156

161157
// We need to get the pointer manually now.
162158
// We do this by casting to a `*i8`, then offsetting it by the appropriate amount.
163159
// We do this instead of, say, simply adjusting the pointer from the result of a GEP
164-
// because the field may have an arbitrary alignment in the LLVM representation
165-
// anyway.
160+
// because the field may have an arbitrary alignment in the LLVM representation.
166161
//
167162
// To demonstrate:
168163
//
@@ -179,7 +174,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
179174
let unaligned_offset = bx.cx().const_usize(offset.bytes());
180175

181176
// Get the alignment of the field
182-
let(_,mut unsized_align) = glue::size_and_align_of_dst(bx, field.ty, meta);
177+
let(_,mut unsized_align) = size_of_val::size_and_align_of_dst(bx, field.ty, meta);
183178

184179
// For packed types, we need to cap alignment.
185180
iflet ty::Adt(def, _) = self.layout.ty.kind()

compiler/rustc_codegen_ssa/src/glue.rs renamed to compiler/rustc_codegen_ssa/src/size_of_val.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
//!
2-
//
3-
// Code relating to drop glue.
1+
//! Computing the size and alignment of a value.
42
3+
usecrate::common;
54
usecrate::common::IntPredicate;
65
usecrate::meth;
76
usecrate::traits::*;
7+
use rustc_hir::LangItem;
8+
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
89
use rustc_middle::ty::{self,Ty};
910
use rustc_target::abi::WrappingRange;
1011

@@ -14,7 +15,7 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
1415
info:Option<Bx::Value>,
1516
) -> (Bx::Value,Bx::Value){
1617
let layout = bx.layout_of(t);
17-
debug!("size_and_align_of_dst(ty={}, info={:?}): layout: {:?}", t, info, layout);
18+
trace!("size_and_align_of_dst(ty={}, info={:?}): layout: {:?}", t, info, layout);
1819
if layout.is_sized(){
1920
let size = bx.const_usize(layout.size.bytes());
2021
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>>(
5152
bx.const_usize(unit.align.abi.bytes()),
5253
)
5354
}
54-
_ => {
55+
ty::Foreign(_) => {
56+
// `extern` type. We cannot compute the size, so panic.
57+
let msg_str = with_no_visible_paths!({
58+
with_no_trimmed_paths!({
59+
format!("attempted to compute the size or alignment of extern type `{t}`")
60+
})
61+
});
62+
let msg = bx.const_str(&msg_str);
63+
64+
// Obtain the panic entry point.
65+
let(fn_abi, llfn) = common::build_langcall(bx,None,LangItem::PanicNounwind);
66+
67+
// Generate the call.
68+
// Cannot use `do_call` since we don't have a MIR terminator so we can't create a `TerminationCodegenHelper`.
69+
// (But we are in good company, this code is duplicated plenty of times.)
70+
let fn_ty = bx.fn_decl_backend_type(fn_abi);
71+
72+
bx.call(fn_ty,/* fn_attrs */None,Some(fn_abi), llfn,&[msg.0, msg.1],None);
73+
74+
// This function does not return so we can now return whatever we want.
75+
let size = bx.const_usize(layout.size.bytes());
76+
let align = bx.const_usize(layout.align.abi.bytes());
77+
(size, align)
78+
}
79+
ty::Adt(..) | ty::Tuple(..) => {
5580
// First get the size of all statically known fields.
5681
// Don't use size_of because it also rounds up to alignment, which we
5782
// 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>>(
122147

123148
(size, align)
124149
}
150+
_ => bug!("size_and_align_of_dst: {t} not supported"),
125151
}
126152
}

‎compiler/rustc_const_eval/src/interpret/projection.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,15 @@ where
174174
};
175175
(base_meta, offset.align_to(align))
176176
}
177-
None => {
178-
// For unsized types with an extern type tail we perform no adjustments.
179-
// NOTE: keep this in sync with `PlaceRef::project_field` in the codegen backend.
180-
assert!(matches!(base_meta,MemPlaceMeta::None));
177+
Noneif offset == Size::ZERO => {
178+
// If the offset is 0, then rounding it up to alignment wouldn't change anything,
179+
// so we can do this even for types where we cannot determine the alignment.
181180
(base_meta, offset)
182181
}
182+
None => {
183+
// We don't know the alignment of this field, so we cannot adjust.
184+
throw_unsup_format!("`extern type` does not have a known offset")
185+
}
183186
}
184187
}else{
185188
// base_meta could be present; we might be accessing a sized field of an unsized
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#![feature(extern_types)]
2+
3+
extern"C"{
4+
typeOpaque;
5+
}
6+
7+
structNewtype(Opaque);
8+
9+
structS{
10+
i:i32,
11+
j:i32,
12+
a:Newtype,
13+
}
14+
15+
fnmain(){
16+
let buf = [0i32;4];
17+
18+
let x:&Newtype = unsafe{&*(&buf as*const_as*constNewtype)};
19+
// Projecting to the newtype works, because it is always at offset 0.
20+
let _field = &x.0;
21+
22+
let x:&S = unsafe{&*(&buf as*const_as*constS)};
23+
// Accessing sized fields is perfectly fine, even at non-zero offsets.
24+
let _field = &x.i;
25+
let _field = &x.j;
26+
// This needs to compute the field offset, but we don't know the type's alignment,
27+
// so this panics.
28+
let _field = &x.a;//~ERROR: does not have a known offset
29+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: unsupported operation: `extern type` does not have a known offset
2+
--> $DIR/extern-type-field-offset.rs:LL:CC
3+
|
4+
LL | let _field = &x.a;
5+
| ^^^^ `extern type` does not have a known offset
6+
|
7+
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
8+
= note: BACKTRACE:
9+
= note: inside `main` at $DIR/extern-type-field-offset.rs:LL:CC
10+
11+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
12+
13+
error: aborting due to 1 previous error
14+

0 commit comments

Comments
 (0)