Skip to content

Commit dfb4194

Browse files
authored
Rollup merge of #95785 - RalfJung:interpret-size-mismatch, r=oli-obk
interpret: err instead of ICE on size mismatches in to_bits_or_ptr_internal We did this a while ago already for `to_i32()` and friends, but missed this one. That became quite annoying when I was debugging an ICE caused by `read_pointer` in a Miri shim where the code was passing an argument at the wrong type. Having `scalar_to_ptr` be fallible is consistent with all the other `Scalar::to_*` methods being fallible. I added `unwrap` only in code outside the interpreter, which is no worse off than before now in terms of panics. r? ````@oli-obk````
2 parents 24fa80d + 38004b7 commit dfb4194

File tree

16 files changed

+107
-67
lines changed

16 files changed

+107
-67
lines changed

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -167,17 +167,18 @@ pub(super) fn op_to_const<'tcx>(
167167
},
168168
Immediate::ScalarPair(a, b) => {
169169
// We know `offset` is relative to the allocation, so we can use `into_parts`.
170-
let (data, start) = match ecx.scalar_to_ptr(a.check_init().unwrap()).into_parts() {
171-
(Some(alloc_id), offset) => {
172-
(ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
173-
}
174-
(None, _offset) => (
175-
ecx.tcx.intern_const_alloc(Allocation::from_bytes_byte_aligned_immutable(
176-
b"" as &[u8],
177-
)),
178-
0,
179-
),
180-
};
170+
let (data, start) =
171+
match ecx.scalar_to_ptr(a.check_init().unwrap()).unwrap().into_parts() {
172+
(Some(alloc_id), offset) => {
173+
(ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
174+
}
175+
(None, _offset) => (
176+
ecx.tcx.intern_const_alloc(
177+
Allocation::from_bytes_byte_aligned_immutable(b"" as &[u8]),
178+
),
179+
0,
180+
),
181+
};
181182
let len = b.to_machine_usize(ecx).unwrap();
182183
let start = start.try_into().unwrap();
183184
let len: usize = len.try_into().unwrap();

compiler/rustc_const_eval/src/const_eval/machine.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,8 @@ impl interpret::MayLeak for ! {
197197
}
198198

199199
impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
200-
fn guaranteed_eq(&mut self, a: Scalar, b: Scalar) -> bool {
201-
match (a, b) {
200+
fn guaranteed_eq(&mut self, a: Scalar, b: Scalar) -> InterpResult<'tcx, bool> {
201+
Ok(match (a, b) {
202202
// Comparisons between integers are always known.
203203
(Scalar::Int { .. }, Scalar::Int { .. }) => a == b,
204204
// Equality with integers can never be known for sure.
@@ -207,25 +207,25 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
207207
// some things (like functions and vtables) do not have stable addresses
208208
// so we need to be careful around them (see e.g. #73722).
209209
(Scalar::Ptr(..), Scalar::Ptr(..)) => false,
210-
}
210+
})
211211
}
212212

213-
fn guaranteed_ne(&mut self, a: Scalar, b: Scalar) -> bool {
214-
match (a, b) {
213+
fn guaranteed_ne(&mut self, a: Scalar, b: Scalar) -> InterpResult<'tcx, bool> {
214+
Ok(match (a, b) {
215215
// Comparisons between integers are always known.
216216
(Scalar::Int(_), Scalar::Int(_)) => a != b,
217217
// Comparisons of abstract pointers with null pointers are known if the pointer
218218
// is in bounds, because if they are in bounds, the pointer can't be null.
219219
// Inequality with integers other than null can never be known for sure.
220220
(Scalar::Int(int), ptr @ Scalar::Ptr(..))
221221
| (ptr @ Scalar::Ptr(..), Scalar::Int(int)) => {
222-
int.is_null() && !self.scalar_may_be_null(ptr)
222+
int.is_null() && !self.scalar_may_be_null(ptr)?
223223
}
224224
// FIXME: return `true` for at least some comparisons where we can reliably
225225
// determine the result of runtime inequality tests at compile-time.
226226
// Examples include comparison of addresses in different static items.
227227
(Scalar::Ptr(..), Scalar::Ptr(..)) => false,
228-
}
228+
})
229229
}
230230
}
231231

@@ -329,9 +329,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
329329
let a = ecx.read_immediate(&args[0])?.to_scalar()?;
330330
let b = ecx.read_immediate(&args[1])?.to_scalar()?;
331331
let cmp = if intrinsic_name == sym::ptr_guaranteed_eq {
332-
ecx.guaranteed_eq(a, b)
332+
ecx.guaranteed_eq(a, b)?
333333
} else {
334-
ecx.guaranteed_ne(a, b)
334+
ecx.guaranteed_ne(a, b)?
335335
};
336336
ecx.write_scalar(Scalar::from_bool(cmp), dest)?;
337337
}

compiler/rustc_const_eval/src/interpret/cast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
283283
if let Some(entry_idx) = vptr_entry_idx {
284284
let entry_idx = u64::try_from(entry_idx).unwrap();
285285
let (old_data, old_vptr) = val.to_scalar_pair()?;
286-
let old_vptr = self.scalar_to_ptr(old_vptr);
286+
let old_vptr = self.scalar_to_ptr(old_vptr)?;
287287
let new_vptr = self
288288
.read_new_vtable_after_trait_upcasting_from_vtable(old_vptr, entry_idx)?;
289289
self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest)

compiler/rustc_const_eval/src/interpret/eval_context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
640640
Ok(Some((size, align)))
641641
}
642642
ty::Dynamic(..) => {
643-
let vtable = self.scalar_to_ptr(metadata.unwrap_meta());
643+
let vtable = self.scalar_to_ptr(metadata.unwrap_meta())?;
644644
// Read size and align from vtable (already checks size).
645645
Ok(Some(self.read_size_and_align_from_vtable(vtable)?))
646646
}

compiler/rustc_const_eval/src/interpret/intern.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
202202
if let ty::Dynamic(..) =
203203
tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind()
204204
{
205-
let ptr = self.ecx.scalar_to_ptr(mplace.meta.unwrap_meta());
205+
let ptr = self.ecx.scalar_to_ptr(mplace.meta.unwrap_meta())?;
206206
if let Some(alloc_id) = ptr.provenance {
207207
// Explicitly choose const mode here, since vtables are immutable, even
208208
// if the reference of the fat pointer is mutable.

compiler/rustc_const_eval/src/interpret/memory.rs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,30 +1102,38 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
11021102

11031103
/// Machine pointer introspection.
11041104
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
1105-
pub fn scalar_to_ptr(&self, scalar: Scalar<M::PointerTag>) -> Pointer<Option<M::PointerTag>> {
1105+
pub fn scalar_to_ptr(
1106+
&self,
1107+
scalar: Scalar<M::PointerTag>,
1108+
) -> InterpResult<'tcx, Pointer<Option<M::PointerTag>>> {
11061109
// We use `to_bits_or_ptr_internal` since we are just implementing the method people need to
11071110
// call to force getting out a pointer.
1108-
match scalar.to_bits_or_ptr_internal(self.pointer_size()) {
1109-
Err(ptr) => ptr.into(),
1110-
Ok(bits) => {
1111-
let addr = u64::try_from(bits).unwrap();
1112-
let ptr = M::ptr_from_addr(&self, addr);
1113-
if addr == 0 {
1114-
assert!(ptr.provenance.is_none(), "null pointer can never have an AllocId");
1111+
Ok(
1112+
match scalar
1113+
.to_bits_or_ptr_internal(self.pointer_size())
1114+
.map_err(|s| err_ub!(ScalarSizeMismatch(s)))?
1115+
{
1116+
Err(ptr) => ptr.into(),
1117+
Ok(bits) => {
1118+
let addr = u64::try_from(bits).unwrap();
1119+
let ptr = M::ptr_from_addr(&self, addr);
1120+
if addr == 0 {
1121+
assert!(ptr.provenance.is_none(), "null pointer can never have an AllocId");
1122+
}
1123+
ptr
11151124
}
1116-
ptr
1117-
}
1118-
}
1125+
},
1126+
)
11191127
}
11201128

11211129
/// Test if this value might be null.
11221130
/// If the machine does not support ptr-to-int casts, this is conservative.
1123-
pub fn scalar_may_be_null(&self, scalar: Scalar<M::PointerTag>) -> bool {
1124-
match scalar.try_to_int() {
1131+
pub fn scalar_may_be_null(&self, scalar: Scalar<M::PointerTag>) -> InterpResult<'tcx, bool> {
1132+
Ok(match scalar.try_to_int() {
11251133
Ok(int) => int.is_null(),
11261134
Err(_) => {
11271135
// Can only happen during CTFE.
1128-
let ptr = self.scalar_to_ptr(scalar);
1136+
let ptr = self.scalar_to_ptr(scalar)?;
11291137
match self.ptr_try_get_alloc_id(ptr) {
11301138
Ok((alloc_id, offset, _)) => {
11311139
let (size, _align) = self
@@ -1138,7 +1146,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
11381146
Err(_offset) => bug!("a non-int scalar is always a pointer"),
11391147
}
11401148
}
1141-
}
1149+
})
11421150
}
11431151

11441152
/// Turning a "maybe pointer" into a proper pointer (and some information

compiler/rustc_const_eval/src/interpret/operand.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
342342
&self,
343343
op: &OpTy<'tcx, M::PointerTag>,
344344
) -> InterpResult<'tcx, Pointer<Option<M::PointerTag>>> {
345-
Ok(self.scalar_to_ptr(self.read_scalar(op)?.check_init()?))
345+
self.scalar_to_ptr(self.read_scalar(op)?.check_init()?)
346346
}
347347

348348
// Turn the wide MPlace into a string (must already be dereferenced!)
@@ -738,7 +738,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
738738
// okay. Everything else, we conservatively reject.
739739
let ptr_valid = niche_start == 0
740740
&& variants_start == variants_end
741-
&& !self.scalar_may_be_null(tag_val);
741+
&& !self.scalar_may_be_null(tag_val)?;
742742
if !ptr_valid {
743743
throw_ub!(InvalidTag(dbg_val))
744744
}

compiler/rustc_const_eval/src/interpret/place.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ where
281281
};
282282

283283
let mplace = MemPlace {
284-
ptr: self.scalar_to_ptr(ptr.check_init()?),
284+
ptr: self.scalar_to_ptr(ptr.check_init()?)?,
285285
// We could use the run-time alignment here. For now, we do not, because
286286
// the point of tracking the alignment here is to make sure that the *static*
287287
// alignment information emitted with the loads is correct. The run-time
@@ -1104,7 +1104,7 @@ where
11041104
&self,
11051105
mplace: &MPlaceTy<'tcx, M::PointerTag>,
11061106
) -> InterpResult<'tcx, (ty::Instance<'tcx>, MPlaceTy<'tcx, M::PointerTag>)> {
1107-
let vtable = self.scalar_to_ptr(mplace.vtable()); // also sanity checks the type
1107+
let vtable = self.scalar_to_ptr(mplace.vtable())?; // also sanity checks the type
11081108
let (instance, ty) = self.read_drop_type_from_vtable(vtable)?;
11091109
let layout = self.layout_of(ty)?;
11101110

compiler/rustc_const_eval/src/interpret/terminator.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
519519
.kind(),
520520
ty::Dynamic(..)
521521
));
522-
let vtable = self.scalar_to_ptr(receiver_place.meta.unwrap_meta());
522+
let vtable = self.scalar_to_ptr(receiver_place.meta.unwrap_meta())?;
523523
let fn_val = self.get_vtable_slot(vtable, u64::try_from(idx).unwrap())?;
524524

525525
// `*mut receiver_place.layout.ty` is almost the layout that we

compiler/rustc_const_eval/src/interpret/traits.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
5050
let vtable_slot = self
5151
.get_ptr_alloc(vtable_slot, ptr_size, self.tcx.data_layout.pointer_align.abi)?
5252
.expect("cannot be a ZST");
53-
let fn_ptr = self.scalar_to_ptr(vtable_slot.read_ptr_sized(Size::ZERO)?.check_init()?);
53+
let fn_ptr = self.scalar_to_ptr(vtable_slot.read_ptr_sized(Size::ZERO)?.check_init()?)?;
5454
self.get_ptr_fn(fn_ptr)
5555
}
5656

@@ -75,7 +75,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
7575
.check_init()?;
7676
// We *need* an instance here, no other kind of function value, to be able
7777
// to determine the type.
78-
let drop_instance = self.get_ptr_fn(self.scalar_to_ptr(drop_fn))?.as_instance()?;
78+
let drop_instance = self.get_ptr_fn(self.scalar_to_ptr(drop_fn)?)?.as_instance()?;
7979
trace!("Found drop fn: {:?}", drop_instance);
8080
let fn_sig = drop_instance.ty(*self.tcx, self.param_env).fn_sig(*self.tcx);
8181
let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, fn_sig);
@@ -132,7 +132,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
132132
.get_ptr_alloc(vtable_slot, pointer_size, self.tcx.data_layout.pointer_align.abi)?
133133
.expect("cannot be a ZST");
134134

135-
let new_vtable = self.scalar_to_ptr(new_vtable.read_ptr_sized(Size::ZERO)?.check_init()?);
135+
let new_vtable =
136+
self.scalar_to_ptr(new_vtable.read_ptr_sized(Size::ZERO)?.check_init()?)?;
136137

137138
Ok(new_vtable)
138139
}

0 commit comments

Comments
 (0)