Skip to content

Commit ed4b010

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 ed4b010

File tree

1 file changed

+32
-20
lines changed

1 file changed

+32
-20
lines changed

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

Lines changed: 32 additions & 20 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,19 @@ 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+
(
311+
u8::try_from(env.pointer_type().bytes()).unwrap(),
312+
env.pointer_type().bytes(),
313+
)
314+
} else {
315+
(
316+
u8::try_from(std::mem::align_of::<T>()).unwrap(),
317+
u32::try_from(std::mem::size_of::<T>()).unwrap(),
318+
)
319+
};
307320
let required_size = required_capacity * entry_size;
308321

309322
match existing_slot {
@@ -368,36 +381,33 @@ pub(crate) mod stack_switching_helpers {
368381
/// index 0. This expects the Vector object to be empty (i.e., current
369382
/// length is 0), and to be of sufficient capacity to store |`values`|
370383
/// 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.
374384
pub fn store_data_entries<'a>(
375385
&self,
376386
env: &mut crate::func_environ::FuncEnvironment<'a>,
377387
builder: &mut FunctionBuilder,
378388
values: &[ir::Value],
389+
entry_size: u32,
379390
) {
380391
let store_count = builder
381392
.ins()
382393
.iconst(I32, i64::try_from(values.len()).unwrap());
383394

384395
debug_assert!(values.iter().all(|val| {
385396
let ty = builder.func.dfg.value_type(*val);
386-
let size = usize::try_from(ty.bytes()).unwrap();
387-
size <= std::mem::size_of::<T>()
397+
let size = ty.bytes();
398+
size <= entry_size
388399
}));
389400

390401
let memflags = ir::MemFlags::trusted();
391402

392403
let data_start_pointer = self.get_data(env, builder);
393404

394-
let entry_size = i32::try_from(std::mem::size_of::<T>()).unwrap();
395405
let mut offset = 0;
396406
for value in values {
397407
builder
398408
.ins()
399409
.store(memflags, *value, data_start_pointer, offset);
400-
offset += entry_size;
410+
offset += i32::try_from(entry_size).unwrap();
401411
}
402412

403413
self.set_length(env, builder, store_count);
@@ -793,11 +803,13 @@ pub(crate) mod stack_switching_helpers {
793803

794804
fn load_top_of_stack<'a>(
795805
&self,
796-
_env: &mut crate::func_environ::FuncEnvironment<'a>,
806+
env: &mut crate::func_environ::FuncEnvironment<'a>,
797807
builder: &mut FunctionBuilder,
798808
) -> ir::Value {
799809
let mem_flags = ir::MemFlags::trusted();
800-
builder.ins().load(I64, mem_flags, self.tos_ptr, 0)
810+
builder
811+
.ins()
812+
.load(env.pointer_type(), mem_flags, self.tos_ptr, 0)
801813
}
802814

803815
/// Returns address of the control context stored in the stack memory,
@@ -1114,10 +1126,8 @@ fn search_handler<'a>(
11141126
builder.switch_to_block(compare_tags);
11151127

11161128
let base = handler_list_data_ptr;
1117-
let entry_size = std::mem::size_of::<*mut u8>();
1118-
let offset = builder
1119-
.ins()
1120-
.imul_imm(index, i64::try_from(entry_size).unwrap());
1129+
let entry_size = env.pointer_type().bytes();
1130+
let offset = builder.ins().imul_imm(index, i64::from(entry_size));
11211131
let offset = builder.ins().uextend(I64, offset);
11221132
let entry_address = builder.ins().iadd(base, offset);
11231133

@@ -1378,7 +1388,8 @@ pub(crate) fn translate_resume<'a>(
13781388
.collect();
13791389

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

13831394
// To enable distinguishing switch and suspend handlers when searching the handler list:
13841395
// Store at which index the switch handlers start.
@@ -1624,7 +1635,8 @@ pub(crate) fn translate_suspend<'a>(
16241635
}
16251636

16261637
if suspend_args.len() > 0 {
1627-
values.store_data_entries(env, builder, suspend_args)
1638+
let entry_size: u32 = std::mem::size_of::<u128>().try_into().unwrap();
1639+
values.store_data_entries(env, builder, suspend_args, entry_size)
16281640
}
16291641

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

0 commit comments

Comments
 (0)