Skip to content

Commit fc5a5e2

Browse files
authored
Rollup merge of #144187 - RalfJung:type-id-base-addr, r=oli-obk
fix handling of base address for TypeId allocations This fixes the problems discovered by ````@theemathas```` in rust-lang/rust#142789: - const-eval would sometimes consider TypeId pointers to be null - the type ID is different in Miri than in regular executions Both boil down to the same issue: the TypeId "allocation" has a guaranteed 0 base address, but const-eval assumes it was non-zero (like normal allocations) and Miri randomized it (like normal allocations). r? ````@oli-obk````
2 parents 5f79b81 + 309b0c5 commit fc5a5e2

File tree

3 files changed

+37
-27
lines changed

3 files changed

+37
-27
lines changed

src/alloc_addresses/mod.rs

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -116,21 +116,26 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
116116
let this = self.eval_context_ref();
117117
let info = this.get_alloc_info(alloc_id);
118118

119-
// Miri's address assignment leaks state across thread boundaries, which is incompatible
120-
// with GenMC execution. So we instead let GenMC assign addresses to allocations.
121-
if let Some(genmc_ctx) = this.machine.data_race.as_genmc_ref() {
122-
let addr = genmc_ctx.handle_alloc(&this.machine, info.size, info.align, memory_kind)?;
123-
return interp_ok(addr);
124-
}
125-
126-
let mut rng = this.machine.rng.borrow_mut();
127119
// This is either called immediately after allocation (and then cached), or when
128120
// adjusting `tcx` pointers (which never get freed). So assert that we are looking
129121
// at a live allocation. This also ensures that we never re-assign an address to an
130122
// allocation that previously had an address, but then was freed and the address
131123
// information was removed.
132124
assert!(!matches!(info.kind, AllocKind::Dead));
133125

126+
// TypeId allocations always have a "base address" of 0 (i.e., the relative offset is the
127+
// hash fragment and therefore equal to the actual integer value).
128+
if matches!(info.kind, AllocKind::TypeId) {
129+
return interp_ok(0);
130+
}
131+
132+
// Miri's address assignment leaks state across thread boundaries, which is incompatible
133+
// with GenMC execution. So we instead let GenMC assign addresses to allocations.
134+
if let Some(genmc_ctx) = this.machine.data_race.as_genmc_ref() {
135+
let addr = genmc_ctx.handle_alloc(&this.machine, info.size, info.align, memory_kind)?;
136+
return interp_ok(addr);
137+
}
138+
134139
// This allocation does not have a base address yet, pick or reuse one.
135140
if !this.machine.native_lib.is_empty() {
136141
// In native lib mode, we use the "real" address of the bytes for this allocation.
@@ -157,7 +162,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
157162
this.get_alloc_bytes_unchecked_raw(alloc_id)?
158163
}
159164
}
160-
AllocKind::Function | AllocKind::Virtual => {
165+
AllocKind::Function | AllocKind::VTable => {
161166
// Allocate some dummy memory to get a unique address for this function/vtable.
162167
let alloc_bytes = MiriAllocBytes::from_bytes(
163168
&[0u8; 1],
@@ -169,12 +174,13 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
169174
std::mem::forget(alloc_bytes);
170175
ptr
171176
}
172-
AllocKind::Dead => unreachable!(),
177+
AllocKind::TypeId | AllocKind::Dead => unreachable!(),
173178
};
174179
// We don't have to expose this pointer yet, we do that in `prepare_for_native_call`.
175180
return interp_ok(base_ptr.addr().to_u64());
176181
}
177182
// We are not in native lib mode, so we control the addresses ourselves.
183+
let mut rng = this.machine.rng.borrow_mut();
178184
if let Some((reuse_addr, clock)) = global_state.reuse.take_addr(
179185
&mut *rng,
180186
info.size,
@@ -295,21 +301,25 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
295301
// Store address in cache.
296302
global_state.base_addr.try_insert(alloc_id, base_addr).unwrap();
297303

298-
// Also maintain the opposite mapping in `int_to_ptr_map`, ensuring we keep it sorted.
299-
// We have a fast-path for the common case that this address is bigger than all previous ones.
300-
let pos = if global_state
301-
.int_to_ptr_map
302-
.last()
303-
.is_some_and(|(last_addr, _)| *last_addr < base_addr)
304-
{
305-
global_state.int_to_ptr_map.len()
306-
} else {
307-
global_state
304+
// Also maintain the opposite mapping in `int_to_ptr_map`, ensuring we keep it
305+
// sorted. We have a fast-path for the common case that this address is bigger than
306+
// all previous ones. We skip this for allocations at address 0; those can't be
307+
// real, they must be TypeId "fake allocations".
308+
if base_addr != 0 {
309+
let pos = if global_state
308310
.int_to_ptr_map
309-
.binary_search_by_key(&base_addr, |(addr, _)| *addr)
310-
.unwrap_err()
311-
};
312-
global_state.int_to_ptr_map.insert(pos, (base_addr, alloc_id));
311+
.last()
312+
.is_some_and(|(last_addr, _)| *last_addr < base_addr)
313+
{
314+
global_state.int_to_ptr_map.len()
315+
} else {
316+
global_state
317+
.int_to_ptr_map
318+
.binary_search_by_key(&base_addr, |(addr, _)| *addr)
319+
.unwrap_err()
320+
};
321+
global_state.int_to_ptr_map.insert(pos, (base_addr, alloc_id));
322+
}
313323

314324
interp_ok(base_addr)
315325
}

src/borrow_tracker/stacked_borrows/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ trait EvalContextPrivExt<'tcx, 'ecx>: crate::MiriInterpCxExt<'tcx> {
650650
dcx.log_protector();
651651
}
652652
},
653-
AllocKind::Function | AllocKind::Virtual | AllocKind::Dead => {
653+
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
654654
// No stacked borrows on these allocations.
655655
}
656656
}
@@ -1021,7 +1021,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
10211021
trace!("Stacked Borrows tag {tag:?} exposed in {alloc_id:?}");
10221022
alloc_extra.borrow_tracker_sb().borrow_mut().exposed_tags.insert(tag);
10231023
}
1024-
AllocKind::Function | AllocKind::Virtual | AllocKind::Dead => {
1024+
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
10251025
// No stacked borrows on these allocations.
10261026
}
10271027
}

src/borrow_tracker/tree_borrows/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -673,7 +673,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
673673
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
674674
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
675675
}
676-
AllocKind::Function | AllocKind::Virtual | AllocKind::Dead => {
676+
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
677677
// No tree borrows on these allocations.
678678
}
679679
}

0 commit comments

Comments
 (0)