Skip to content

Commit 0f15f05

Browse files
RalfJungRalfJung
committed
interpret: simplify pointer arithmetic logic
1 parent a7a342b commit 0f15f05

27 files changed

+73
-187
lines changed

‎compiler/rustc_abi/src/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ impl Size {
516516
/// Truncates `value` to `self` bits and then sign-extends it to 128 bits
517517
/// (i.e., if it is negative, fill with 1's on the left).
518518
#[inline]
519-
pub fn sign_extend(self, value: u128) -> u128 {
519+
pub fn sign_extend(self, value: u128) -> i128 {
520520
let size = self.bits();
521521
if size == 0 {
522522
// Truncated until nothing is left.
@@ -526,7 +526,7 @@ impl Size {
526526
let shift = 128 - size;
527527
// Shift the unsigned value to the left, then shift back to the right as signed
528528
// (essentially fills with sign bit on the left).
529-
(((value << shift) as i128) >> shift) as u128
529+
((value << shift) as i128) >> shift
530530
}
531531

532532
/// Truncates `value` to `self` bits.
@@ -544,7 +544,7 @@ impl Size {
544544

545545
#[inline]
546546
pub fn signed_int_min(&self) -> i128 {
547-
self.sign_extend(1_u128 << (self.bits() - 1)) as i128
547+
self.sign_extend(1_u128 << (self.bits() - 1))
548548
}
549549

550550
#[inline]

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

-11
Original file line numberDiff line numberDiff line change
@@ -560,17 +560,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
560560
self.frame().body
561561
}
562562

563-
#[inline(always)]
564-
pub fn sign_extend(&self, value: u128, ty: TyAndLayout<'_>) -> u128 {
565-
assert!(ty.abi.is_signed());
566-
ty.size.sign_extend(value)
567-
}
568-
569-
#[inline(always)]
570-
pub fn truncate(&self, value: u128, ty: TyAndLayout<'_>) -> u128 {
571-
ty.size.truncate(value)
572-
}
573-
574563
#[inline]
575564
pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool {
576565
ty.is_freeze(*self.tcx, self.param_env)

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

+3-6
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
206206
} else {
207207
(val_bits >> shift_bits) | (val_bits << inv_shift_bits)
208208
};
209-
let truncated_bits = self.truncate(result_bits, layout_val);
209+
let truncated_bits = layout_val.size.truncate(result_bits);
210210
let result = Scalar::from_uint(truncated_bits, layout_val.size);
211211
self.write_scalar(result, dest)?;
212212
}
@@ -580,13 +580,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
580580
ptr: Pointer<Option<M::Provenance>>,
581581
offset_bytes: i64,
582582
) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
583-
// We first compute the pointer with overflow checks, to get a specific error for when it
584-
// overflows (though technically this is redundant with the following inbounds check).
585-
let result = ptr.signed_offset(offset_bytes, self)?;
586583
// The offset must be in bounds starting from `ptr`.
587584
self.check_ptr_access_signed(ptr, offset_bytes, CheckInAllocMsg::PointerArithmeticTest)?;
588-
// Done.
589-
Ok(result)
585+
// This also implies that there is no overflow, so we are done.
586+
Ok(ptr.wrapping_signed_offset(offset_bytes, self))
590587
}
591588

592589
/// Copy `count*size_of::<T>()` many bytes from `*src` to `*dst`.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
473473
throw_ub!(PointerOutOfBounds {
474474
alloc_id,
475475
alloc_size,
476-
ptr_offset: self.target_usize_to_isize(offset),
476+
ptr_offset: self.sign_extend_to_target_isize(offset),
477477
inbounds_size: size,
478478
msg,
479479
})

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

+2-4
Original file line numberDiff line numberDiff line change
@@ -289,10 +289,8 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for PlaceTy<'tcx, Prov> {
289289
// projections are type-checked and bounds-checked.
290290
assert!(offset + layout.size <= self.layout.size);
291291

292-
let new_offset = Size::from_bytes(
293-
ecx.data_layout()
294-
.offset(old_offset.unwrap_or(Size::ZERO).bytes(), offset.bytes())?,
295-
);
292+
// Size `+`, ensures no overflow.
293+
let new_offset = old_offset.unwrap_or(Size::ZERO) + offset;
296294

297295
PlaceTy {
298296
place: Place::Local { local, offset: Some(new_offset), locals_addr },

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
362362
// of the first element.
363363
let elem_size = first.layout.size;
364364
let first_ptr = first.ptr();
365-
let rest_ptr = first_ptr.offset(elem_size, self)?;
365+
let rest_ptr = first_ptr.wrapping_offset(elem_size, self);
366366
// No alignment requirement since `copy_op` above already checked it.
367367
self.mem_copy_repeatedly(
368368
first_ptr,

‎compiler/rustc_lint/src/types.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -309,11 +309,7 @@ fn report_bin_hex_error(
309309
) {
310310
let (t, actually) = match ty {
311311
attr::IntType::SignedInt(t) => {
312-
let actually = if negative {
313-
-(size.sign_extend(val) as i128)
314-
} else {
315-
size.sign_extend(val) as i128
316-
};
312+
let actually = if negative { -(size.sign_extend(val)) } else { size.sign_extend(val) };
317313
(t.name_str(), actually.to_string())
318314
}
319315
attr::IntType::UnsignedInt(t) => {

‎compiler/rustc_middle/src/mir/interpret/pointer.rs

+11-87
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc_data_structures::static_assert_size;
55
use rustc_macros::{HashStable, TyDecodable, TyEncodable};
66
use rustc_target::abi::{HasDataLayout, Size};
77

8-
use super::{AllocId, InterpResult};
8+
use super::AllocId;
99

1010
////////////////////////////////////////////////////////////////////////////////
1111
// Pointer arithmetic
@@ -40,62 +40,13 @@ pub trait PointerArithmetic: HasDataLayout {
4040
}
4141

4242
#[inline]
43-
fn target_usize_to_isize(&self, val: u64) -> i64 {
44-
let val = val as i64;
45-
// Now wrap-around into the machine_isize range.
46-
if val > self.target_isize_max() {
47-
// This can only happen if the ptr size is < 64, so we know max_usize_plus_1 fits into
48-
// i64.
49-
debug_assert!(self.pointer_size().bits() < 64);
50-
let max_usize_plus_1 = 1u128 << self.pointer_size().bits();
51-
val - i64::try_from(max_usize_plus_1).unwrap()
52-
} else {
53-
val
54-
}
55-
}
56-
57-
/// Helper function: truncate given value-"overflowed flag" pair to pointer size and
58-
/// update "overflowed flag" if there was an overflow.
59-
/// This should be called by all the other methods before returning!
60-
#[inline]
61-
fn truncate_to_ptr(&self, (val, over): (u64, bool)) -> (u64, bool) {
62-
let val = u128::from(val);
63-
let max_ptr_plus_1 = 1u128 << self.pointer_size().bits();
64-
(u64::try_from(val % max_ptr_plus_1).unwrap(), over || val >= max_ptr_plus_1)
65-
}
66-
67-
#[inline]
68-
fn overflowing_offset(&self, val: u64, i: u64) -> (u64, bool) {
69-
// We do not need to check if i fits in a machine usize. If it doesn't,
70-
// either the wrapping_add will wrap or res will not fit in a pointer.
71-
let res = val.overflowing_add(i);
72-
self.truncate_to_ptr(res)
73-
}
74-
75-
#[inline]
76-
fn overflowing_signed_offset(&self, val: u64, i: i64) -> (u64, bool) {
77-
// We need to make sure that i fits in a machine isize.
78-
let n = i.unsigned_abs();
79-
if i >= 0 {
80-
let (val, over) = self.overflowing_offset(val, n);
81-
(val, over || i > self.target_isize_max())
82-
} else {
83-
let res = val.overflowing_sub(n);
84-
let (val, over) = self.truncate_to_ptr(res);
85-
(val, over || i < self.target_isize_min())
86-
}
87-
}
88-
89-
#[inline]
90-
fn offset<'tcx>(&self, val: u64, i: u64) -> InterpResult<'tcx, u64> {
91-
let (res, over) = self.overflowing_offset(val, i);
92-
if over { throw_ub!(PointerArithOverflow) } else { Ok(res) }
43+
fn truncate_to_target_usize(&self, val: u64) -> u64 {
44+
self.pointer_size().truncate(val.into()).try_into().unwrap()
9345
}
9446

9547
#[inline]
96-
fn signed_offset<'tcx>(&self, val: u64, i: i64) -> InterpResult<'tcx, u64> {
97-
let (res, over) = self.overflowing_signed_offset(val, i);
98-
if over { throw_ub!(PointerArithOverflow) } else { Ok(res) }
48+
fn sign_extend_to_target_isize(&self, val: u64) -> i64 {
49+
self.pointer_size().sign_extend(val.into()).try_into().unwrap()
9950
}
10051
}
10152

@@ -331,7 +282,7 @@ impl<Prov> Pointer<Option<Prov>> {
331282
}
332283
}
333284

334-
impl<'tcx, Prov> Pointer<Prov> {
285+
impl<Prov> Pointer<Prov> {
335286
#[inline(always)]
336287
pub fn new(provenance: Prov, offset: Size) -> Self {
337288
Pointer { provenance, offset }
@@ -349,43 +300,16 @@ impl<'tcx, Prov> Pointer<Prov> {
349300
Pointer { provenance: f(self.provenance), ..self }
350301
}
351302

352-
#[inline]
353-
pub fn offset(self, i: Size, cx: &impl HasDataLayout) -> InterpResult<'tcx, Self> {
354-
Ok(Pointer {
355-
offset: Size::from_bytes(cx.data_layout().offset(self.offset.bytes(), i.bytes())?),
356-
..self
357-
})
358-
}
359-
360-
#[inline]
361-
pub fn overflowing_offset(self, i: Size, cx: &impl HasDataLayout) -> (Self, bool) {
362-
let (res, over) = cx.data_layout().overflowing_offset(self.offset.bytes(), i.bytes());
363-
let ptr = Pointer { offset: Size::from_bytes(res), ..self };
364-
(ptr, over)
365-
}
366-
367303
#[inline(always)]
368304
pub fn wrapping_offset(self, i: Size, cx: &impl HasDataLayout) -> Self {
369-
self.overflowing_offset(i, cx).0
370-
}
371-
372-
#[inline]
373-
pub fn signed_offset(self, i: i64, cx: &impl HasDataLayout) -> InterpResult<'tcx, Self> {
374-
Ok(Pointer {
375-
offset: Size::from_bytes(cx.data_layout().signed_offset(self.offset.bytes(), i)?),
376-
..self
377-
})
378-
}
379-
380-
#[inline]
381-
pub fn overflowing_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> (Self, bool) {
382-
let (res, over) = cx.data_layout().overflowing_signed_offset(self.offset.bytes(), i);
383-
let ptr = Pointer { offset: Size::from_bytes(res), ..self };
384-
(ptr, over)
305+
let res =
306+
cx.data_layout().truncate_to_target_usize(self.offset.bytes().wrapping_add(i.bytes()));
307+
Pointer { offset: Size::from_bytes(res), ..self }
385308
}
386309

387310
#[inline(always)]
388311
pub fn wrapping_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> Self {
389-
self.overflowing_signed_offset(i, cx).0
312+
// It's wrapping anyway, so we can just cast to `u64`.
313+
self.wrapping_offset(Size::from_bytes(i as u64), cx)
390314
}
391315
}

‎compiler/rustc_middle/src/mir/interpret/value.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ impl<'tcx, Prov: Provenance> Scalar<Prov> {
393393
#[inline]
394394
pub fn to_int(self, size: Size) -> InterpResult<'tcx, i128> {
395395
let b = self.to_bits(size)?;
396-
Ok(size.sign_extend(b) as i128)
396+
Ok(size.sign_extend(b))
397397
}
398398

399399
/// Converts the scalar to produce an `i8`. Fails if the scalar is a pointer.

‎compiler/rustc_middle/src/ty/consts/int.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ impl ScalarInt {
234234
let data = i.into();
235235
// `into` performed sign extension, we have to truncate
236236
let r = Self::raw(size.truncate(data as u128), size);
237-
(r, size.sign_extend(r.data) as i128 != data)
237+
(r, size.sign_extend(r.data) != data)
238238
}
239239

240240
#[inline]
@@ -335,7 +335,7 @@ impl ScalarInt {
335335
#[inline]
336336
pub fn to_int(self, size: Size) -> i128 {
337337
let b = self.to_bits(size);
338-
size.sign_extend(b) as i128
338+
size.sign_extend(b)
339339
}
340340

341341
/// Converts the `ScalarInt` to i8.

‎compiler/rustc_middle/src/ty/util.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ impl<'tcx> Discr<'tcx> {
7979
let (val, oflo) = if signed {
8080
let min = size.signed_int_min();
8181
let max = size.signed_int_max();
82-
let val = size.sign_extend(self.val) as i128;
82+
let val = size.sign_extend(self.val);
8383
assert!(n < (i128::MAX as u128));
8484
let n = n as i128;
8585
let oflo = val > max - n;

‎src/tools/miri/src/alloc_addresses/mod.rs

+10-14
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rand::Rng;
1111

1212
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1313
use rustc_span::Span;
14-
use rustc_target::abi::{Align, HasDataLayout, Size};
14+
use rustc_target::abi::{Align, Size};
1515

1616
use crate::{concurrency::VClock, *};
1717

@@ -307,15 +307,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
307307

308308
let (prov, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
309309
let alloc_id = prov.alloc_id();
310-
let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?;
311310

312-
// Add offset with the right kind of pointer-overflowing arithmetic.
313-
let dl = ecx.data_layout();
314-
let absolute_addr = dl.overflowing_offset(base_addr, offset.bytes()).0;
315-
Ok(interpret::Pointer::new(
311+
// Get a pointer to the beginning of this allocation.
312+
let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?;
313+
let base_ptr = interpret::Pointer::new(
316314
Provenance::Concrete { alloc_id, tag },
317-
Size::from_bytes(absolute_addr),
318-
))
315+
Size::from_bytes(base_addr),
316+
);
317+
// Add offset with the right kind of pointer-overflowing arithmetic.
318+
Ok(base_ptr.wrapping_offset(offset, ecx))
319319
}
320320

321321
/// When a pointer is used for a memory access, this computes where in which allocation the
@@ -341,12 +341,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
341341
let base_addr = *ecx.machine.alloc_addresses.borrow().base_addr.get(&alloc_id).unwrap();
342342

343343
// Wrapping "addr - base_addr"
344-
#[allow(clippy::cast_possible_wrap)] // we want to wrap here
345-
let neg_base_addr = (base_addr as i64).wrapping_neg();
346-
Some((
347-
alloc_id,
348-
Size::from_bytes(ecx.overflowing_signed_offset(addr.bytes(), neg_base_addr).0),
349-
))
344+
let rel_offset = ecx.truncate_to_target_usize(addr.bytes().wrapping_sub(base_addr));
345+
Some((alloc_id, Size::from_bytes(rel_offset)))
350346
}
351347
}
352348

‎src/tools/miri/src/helpers.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
606606
}
607607
// The part between the end_ptr and the end of the place is also frozen.
608608
// So pretend there is a 0-sized `UnsafeCell` at the end.
609-
unsafe_cell_action(&place.ptr().offset(size, this)?, Size::ZERO)?;
609+
unsafe_cell_action(&place.ptr().wrapping_offset(size, this), Size::ZERO)?;
610610
// Done!
611611
return Ok(());
612612

@@ -975,7 +975,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
975975
loop {
976976
// FIXME: We are re-getting the allocation each time around the loop.
977977
// Would be nice if we could somehow "extend" an existing AllocRange.
978-
let alloc = this.get_ptr_alloc(ptr.offset(len, this)?, size1)?.unwrap(); // not a ZST, so we will get a result
978+
let alloc = this.get_ptr_alloc(ptr.wrapping_offset(len, this), size1)?.unwrap(); // not a ZST, so we will get a result
979979
let byte = alloc.read_integer(alloc_range(Size::ZERO, size1))?.to_u8()?;
980980
if byte == 0 {
981981
break;
@@ -1039,7 +1039,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
10391039
break;
10401040
} else {
10411041
wchars.push(wchar_int.try_into().unwrap());
1042-
ptr = ptr.offset(size, this)?;
1042+
ptr = ptr.wrapping_offset(size, this);
10431043
}
10441044
}
10451045

‎src/tools/miri/src/shims/foreign_items.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
622622
{
623623
let idx = u64::try_from(idx).unwrap();
624624
#[allow(clippy::arithmetic_side_effects)] // idx < num, so this never wraps
625-
let new_ptr = ptr.offset(Size::from_bytes(num - idx - 1), this)?;
625+
let new_ptr = ptr.wrapping_offset(Size::from_bytes(num - idx - 1), this);
626626
this.write_pointer(new_ptr, dest)?;
627627
} else {
628628
this.write_null(dest)?;
@@ -646,7 +646,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
646646
.iter()
647647
.position(|&c| c == val);
648648
if let Some(idx) = idx {
649-
let new_ptr = ptr.offset(Size::from_bytes(idx as u64), this)?;
649+
let new_ptr = ptr.wrapping_offset(Size::from_bytes(idx as u64), this);
650650
this.write_pointer(new_ptr, dest)?;
651651
} else {
652652
this.write_null(dest)?;

‎src/tools/miri/src/shims/unix/env.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ impl<'tcx> UnixEnvVars<'tcx> {
8282
};
8383
// The offset is used to strip the "{name}=" part of the string.
8484
let var_ptr = var_ptr
85-
.offset(Size::from_bytes(u64::try_from(name.len()).unwrap().strict_add(1)), ecx)?;
85+
.wrapping_offset(Size::from_bytes(u64::try_from(name.len()).unwrap().strict_add(1)), ecx);
8686
Ok(Some(var_ptr))
8787
}
8888

‎src/tools/miri/src/shims/unix/fs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -996,7 +996,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
996996
&this.ptr_to_mplace(entry, dirent64_layout),
997997
)?;
998998

999-
let name_ptr = entry.offset(Size::from_bytes(d_name_offset), this)?;
999+
let name_ptr = entry.wrapping_offset(Size::from_bytes(d_name_offset), this);
10001000
this.write_bytes_ptr(name_ptr, name_bytes.iter().copied())?;
10011001

10021002
Some(entry)

0 commit comments

Comments
 (0)