Skip to content

Commit 7a29573

Browse files
committed
stack-switching: don't conflate host and target pointer sizes
Disas tests were failing on i686 targeting x86_64 as the size of the host pointer was leaking into what we were using to do codegen in a few paths. This patch is a bit of a hack as it seems like using a generic <T> for T: *mut u8 (as an example) is a bit questionable. To keep things small, I do a hacky typecheck to map pointers to the target pointer size here.
1 parent 5e24337 commit 7a29573

File tree

1 file changed

+28
-17
lines changed

1 file changed

+28
-17
lines changed

crates/cranelift/src/func_environ/stack_switching/instructions.rs

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ pub(crate) mod stack_switching_helpers {
201201
}
202202
}
203203

204-
impl<T> VMHostArrayRef<T> {
204+
impl<T: 'static> VMHostArrayRef<T> {
205205
pub(crate) fn new(address: ir::Value) -> Self {
206206
Self {
207207
address,
@@ -270,8 +270,10 @@ pub(crate) mod stack_switching_helpers {
270270
builder: &mut FunctionBuilder,
271271
data: ir::Value,
272272
) {
273-
let offset = env.offsets.ptr.vmhostarray_data().into();
274-
self.set::<*mut T>(builder, offset, data);
273+
debug_assert_eq!(builder.func.dfg.value_type(data), env.pointer_type());
274+
let offset: i32 = env.offsets.ptr.vmhostarray_data().into();
275+
let mem_flags = ir::MemFlags::trusted();
276+
builder.ins().store(mem_flags, data, self.address, offset);
275277
}
276278

277279
/// Returns pointer to next empty slot in data buffer and marks the
@@ -302,8 +304,16 @@ pub(crate) mod stack_switching_helpers {
302304
required_capacity: u32,
303305
existing_slot: Option<StackSlot>,
304306
) -> StackSlot {
305-
let align = u8::try_from(std::mem::align_of::<T>()).unwrap();
306-
let entry_size = u32::try_from(std::mem::size_of::<T>()).unwrap();
307+
// TODO: hack around host pointer size being mixed up with target
308+
let (align, entry_size) =
309+
if core::any::TypeId::of::<T>() == core::any::TypeId::of::<*mut u8>() {
310+
(env.pointer_type().bytes() as u8, env.pointer_type().bytes())
311+
} else {
312+
(
313+
u8::try_from(std::mem::align_of::<T>()).unwrap(),
314+
u32::try_from(std::mem::size_of::<T>()).unwrap(),
315+
)
316+
};
307317
let required_size = required_capacity * entry_size;
308318

309319
match existing_slot {
@@ -368,36 +378,33 @@ pub(crate) mod stack_switching_helpers {
368378
/// index 0. This expects the Vector object to be empty (i.e., current
369379
/// length is 0), and to be of sufficient capacity to store |`values`|
370380
/// entries.
371-
/// If `allow_smaller` is true, we allow storing values whose type has a
372-
/// smaller size than T's. In that case, such values will be stored at
373-
/// the beginning of a `T`-sized slot.
374381
pub fn store_data_entries<'a>(
375382
&self,
376383
env: &mut crate::func_environ::FuncEnvironment<'a>,
377384
builder: &mut FunctionBuilder,
378385
values: &[ir::Value],
386+
entry_size: u32,
379387
) {
380388
let store_count = builder
381389
.ins()
382390
.iconst(I32, i64::try_from(values.len()).unwrap());
383391

384392
debug_assert!(values.iter().all(|val| {
385393
let ty = builder.func.dfg.value_type(*val);
386-
let size = usize::try_from(ty.bytes()).unwrap();
387-
size <= std::mem::size_of::<T>()
394+
let size = ty.bytes();
395+
size <= entry_size
388396
}));
389397

390398
let memflags = ir::MemFlags::trusted();
391399

392400
let data_start_pointer = self.get_data(env, builder);
393401

394-
let entry_size = i32::try_from(std::mem::size_of::<T>()).unwrap();
395402
let mut offset = 0;
396403
for value in values {
397404
builder
398405
.ins()
399406
.store(memflags, *value, data_start_pointer, offset);
400-
offset += entry_size;
407+
offset += i32::try_from(entry_size).unwrap();
401408
}
402409

403410
self.set_length(env, builder, store_count);
@@ -793,11 +800,13 @@ pub(crate) mod stack_switching_helpers {
793800

794801
fn load_top_of_stack<'a>(
795802
&self,
796-
_env: &mut crate::func_environ::FuncEnvironment<'a>,
803+
env: &mut crate::func_environ::FuncEnvironment<'a>,
797804
builder: &mut FunctionBuilder,
798805
) -> ir::Value {
799806
let mem_flags = ir::MemFlags::trusted();
800-
builder.ins().load(I64, mem_flags, self.tos_ptr, 0)
807+
builder
808+
.ins()
809+
.load(env.pointer_type(), mem_flags, self.tos_ptr, 0)
801810
}
802811

803812
/// Returns address of the control context stored in the stack memory,
@@ -1114,7 +1123,7 @@ fn search_handler<'a>(
11141123
builder.switch_to_block(compare_tags);
11151124

11161125
let base = handler_list_data_ptr;
1117-
let entry_size = std::mem::size_of::<*mut u8>();
1126+
let entry_size = env.pointer_type().bytes();
11181127
let offset = builder
11191128
.ins()
11201129
.imul_imm(index, i64::try_from(entry_size).unwrap());
@@ -1378,7 +1387,8 @@ pub(crate) fn translate_resume<'a>(
13781387
.collect();
13791388

13801389
// Store all tag addresess in the handler list.
1381-
handler_list.store_data_entries(env, builder, &all_tag_addresses);
1390+
let entry_size = env.pointer_type().bytes();
1391+
handler_list.store_data_entries(env, builder, &all_tag_addresses, entry_size.into());
13821392

13831393
// To enable distinguishing switch and suspend handlers when searching the handler list:
13841394
// Store at which index the switch handlers start.
@@ -1624,7 +1634,8 @@ pub(crate) fn translate_suspend<'a>(
16241634
}
16251635

16261636
if suspend_args.len() > 0 {
1627-
values.store_data_entries(env, builder, suspend_args)
1637+
let entry_size = std::mem::size_of::<u128>() as u32;
1638+
values.store_data_entries(env, builder, suspend_args, entry_size)
16281639
}
16291640

16301641
// Set current continuation to suspended and break up handler chain.

0 commit comments

Comments
 (0)