Skip to content

Commit ea9a24e

Browse files
committed
avoid re-checking the offset while iterating an array/slice
1 parent b1ebf00 commit ea9a24e

File tree

5 files changed

+57
-15
lines changed

5 files changed

+57
-15
lines changed

compiler/rustc_const_eval/src/interpret/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackP
2626
pub use self::memory::{AllocKind, AllocRef, AllocRefMut, FnVal, Memory, MemoryKind};
2727
pub use self::operand::{ImmTy, Immediate, OpTy, Readable};
2828
pub use self::place::{MPlaceTy, MemPlaceMeta, PlaceTy, Writeable};
29-
pub use self::projection::Projectable;
29+
pub use self::projection::{OffsetMode, Projectable};
3030
pub use self::terminator::FnArg;
3131
pub use self::validity::{CtfeValidationMode, RefTracking};
3232
pub use self::visitor::ValueVisitor;

compiler/rustc_const_eval/src/interpret/operand.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ use rustc_target::abi::{self, Abi, Align, HasDataLayout, Size};
1414

1515
use super::{
1616
alloc_range, from_known_layout, mir_assign_valid_types, AllocId, Frame, InterpCx, InterpResult,
17-
MPlaceTy, Machine, MemPlace, MemPlaceMeta, PlaceTy, Pointer, Projectable, Provenance, Scalar,
17+
MPlaceTy, Machine, MemPlace, MemPlaceMeta, OffsetMode, PlaceTy, Pointer, Projectable,
18+
Provenance, Scalar,
1819
};
1920

2021
/// An `Immediate` represents a single immediate self-contained Rust value.
@@ -297,6 +298,7 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for ImmTy<'tcx, Prov> {
297298
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
298299
&self,
299300
offset: Size,
301+
_mode: OffsetMode,
300302
meta: MemPlaceMeta<Prov>,
301303
layout: TyAndLayout<'tcx>,
302304
ecx: &InterpCx<'mir, 'tcx, M>,
@@ -391,12 +393,13 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for OpTy<'tcx, Prov> {
391393
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
392394
&self,
393395
offset: Size,
396+
mode: OffsetMode,
394397
meta: MemPlaceMeta<Prov>,
395398
layout: TyAndLayout<'tcx>,
396399
ecx: &InterpCx<'mir, 'tcx, M>,
397400
) -> InterpResult<'tcx, Self> {
398401
match self.as_mplace_or_imm() {
399-
Left(mplace) => Ok(mplace.offset_with_meta(offset, meta, layout, ecx)?.into()),
402+
Left(mplace) => Ok(mplace.offset_with_meta(offset, mode, meta, layout, ecx)?.into()),
400403
Right(imm) => {
401404
assert_matches!(meta, MemPlaceMeta::None); // no place to store metadata here
402405
// Every part of an uninit is uninit.

compiler/rustc_const_eval/src/interpret/place.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use rustc_target::abi::{Abi, Align, FieldIdx, HasDataLayout, Size, FIRST_VARIANT
1616

1717
use super::{
1818
alloc_range, mir_assign_valid_types, AllocId, AllocRef, AllocRefMut, ImmTy, Immediate,
19-
InterpCx, InterpResult, Machine, MemoryKind, OpTy, Operand, Pointer, PointerArithmetic,
20-
Projectable, Provenance, Readable, Scalar,
19+
InterpCx, InterpResult, Machine, MemoryKind, OffsetMode, OpTy, Operand, Pointer,
20+
PointerArithmetic, Projectable, Provenance, Readable, Scalar,
2121
};
2222

2323
#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
@@ -91,6 +91,7 @@ impl<Prov: Provenance> MemPlace<Prov> {
9191
fn offset_with_meta_<'mir, 'tcx, M: Machine<'mir, 'tcx, Provenance = Prov>>(
9292
self,
9393
offset: Size,
94+
mode: OffsetMode,
9495
meta: MemPlaceMeta<Prov>,
9596
ecx: &InterpCx<'mir, 'tcx, M>,
9697
) -> InterpResult<'tcx, Self> {
@@ -101,8 +102,12 @@ impl<Prov: Provenance> MemPlace<Prov> {
101102
if offset > ecx.data_layout().max_size_of_val() {
102103
throw_ub!(PointerArithOverflow);
103104
}
104-
let offset: i64 = offset.bytes().try_into().unwrap();
105-
let ptr = ecx.ptr_offset_inbounds(self.ptr, offset)?;
105+
let ptr = match mode {
106+
OffsetMode::Inbounds => {
107+
ecx.ptr_offset_inbounds(self.ptr, offset.bytes().try_into().unwrap())?
108+
}
109+
OffsetMode::Wrapping => self.ptr.wrapping_offset(offset, ecx),
110+
};
106111
Ok(MemPlace { ptr, meta })
107112
}
108113
}
@@ -194,12 +199,13 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for MPlaceTy<'tcx, Prov> {
194199
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
195200
&self,
196201
offset: Size,
202+
mode: OffsetMode,
197203
meta: MemPlaceMeta<Prov>,
198204
layout: TyAndLayout<'tcx>,
199205
ecx: &InterpCx<'mir, 'tcx, M>,
200206
) -> InterpResult<'tcx, Self> {
201207
Ok(MPlaceTy {
202-
mplace: self.mplace.offset_with_meta_(offset, meta, ecx)?,
208+
mplace: self.mplace.offset_with_meta_(offset, mode, meta, ecx)?,
203209
align: self.align.restrict_for_offset(offset),
204210
layout,
205211
})
@@ -306,12 +312,13 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for PlaceTy<'tcx, Prov> {
306312
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
307313
&self,
308314
offset: Size,
315+
mode: OffsetMode,
309316
meta: MemPlaceMeta<Prov>,
310317
layout: TyAndLayout<'tcx>,
311318
ecx: &InterpCx<'mir, 'tcx, M>,
312319
) -> InterpResult<'tcx, Self> {
313320
Ok(match self.as_mplace_or_local() {
314-
Left(mplace) => mplace.offset_with_meta(offset, meta, layout, ecx)?.into(),
321+
Left(mplace) => mplace.offset_with_meta(offset, mode, meta, layout, ecx)?.into(),
315322
Right((frame, local, old_offset)) => {
316323
debug_assert!(layout.is_sized(), "unsized locals should live in memory");
317324
assert_matches!(meta, MemPlaceMeta::None); // we couldn't store it anyway...
@@ -952,7 +959,13 @@ where
952959
&mut Operand::Indirect(mplace) => mplace, // this already was an indirect local
953960
};
954961
if let Some(offset) = offset {
955-
whole_local.offset_with_meta_(offset, MemPlaceMeta::None, self)?
962+
// This offset is always inbounds, no need to check it again.
963+
whole_local.offset_with_meta_(
964+
offset,
965+
OffsetMode::Wrapping,
966+
MemPlaceMeta::None,
967+
self,
968+
)?
956969
} else {
957970
// Preserve wide place metadata, do not call `offset`.
958971
whole_local

compiler/rustc_const_eval/src/interpret/projection.rs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ use rustc_target::abi::{self, VariantIdx};
1919

2020
use super::{InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy, Provenance, Scalar};
2121

22+
/// Describes the constraints placed on offset-projections.
23+
#[derive(Copy, Clone, Debug)]
24+
pub enum OffsetMode {
25+
/// The offset has to be inbounds, like `ptr::offset`.
26+
Inbounds,
27+
/// No constraints, just wrap around the edge of the address space.
28+
Wrapping,
29+
}
30+
2231
/// A thing that we can project into, and that has a layout.
2332
pub trait Projectable<'tcx, Prov: Provenance>: Sized + std::fmt::Debug {
2433
/// Get the layout.
@@ -53,6 +62,7 @@ pub trait Projectable<'tcx, Prov: Provenance>: Sized + std::fmt::Debug {
5362
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
5463
&self,
5564
offset: Size,
65+
mode: OffsetMode,
5666
meta: MemPlaceMeta<Prov>,
5767
layout: TyAndLayout<'tcx>,
5868
ecx: &InterpCx<'mir, 'tcx, M>,
@@ -65,7 +75,7 @@ pub trait Projectable<'tcx, Prov: Provenance>: Sized + std::fmt::Debug {
6575
ecx: &InterpCx<'mir, 'tcx, M>,
6676
) -> InterpResult<'tcx, Self> {
6777
assert!(layout.is_sized());
68-
self.offset_with_meta(offset, MemPlaceMeta::None, layout, ecx)
78+
self.offset_with_meta(offset, OffsetMode::Inbounds, MemPlaceMeta::None, layout, ecx)
6979
}
7080

7181
fn transmute<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
@@ -75,7 +85,7 @@ pub trait Projectable<'tcx, Prov: Provenance>: Sized + std::fmt::Debug {
7585
) -> InterpResult<'tcx, Self> {
7686
assert!(self.layout().is_sized() && layout.is_sized());
7787
assert_eq!(self.layout().size, layout.size);
78-
self.offset_with_meta(Size::ZERO, MemPlaceMeta::None, layout, ecx)
88+
self.offset_with_meta(Size::ZERO, OffsetMode::Wrapping, MemPlaceMeta::None, layout, ecx)
7989
}
8090

8191
/// Convert this to an `OpTy`. This might be an irreversible transformation, but is useful for
@@ -102,7 +112,17 @@ impl<'tcx, 'a, Prov: Provenance, P: Projectable<'tcx, Prov>> ArrayIterator<'tcx,
102112
ecx: &InterpCx<'mir, 'tcx, M>,
103113
) -> InterpResult<'tcx, Option<(u64, P)>> {
104114
let Some(idx) = self.range.next() else { return Ok(None) };
105-
Ok(Some((idx, self.base.offset(self.stride * idx, self.field_layout, ecx)?)))
115+
// We use `Wrapping` here since the offset has already been checked when the iterator was created.
116+
Ok(Some((
117+
idx,
118+
self.base.offset_with_meta(
119+
self.stride * idx,
120+
OffsetMode::Wrapping,
121+
MemPlaceMeta::None,
122+
self.field_layout,
123+
ecx,
124+
)?,
125+
)))
106126
}
107127
}
108128

@@ -157,7 +177,7 @@ where
157177
(MemPlaceMeta::None, offset)
158178
};
159179

160-
base.offset_with_meta(offset, meta, field_layout, self)
180+
base.offset_with_meta(offset, OffsetMode::Inbounds, meta, field_layout, self)
161181
}
162182

163183
/// Downcasting to an enum variant.
@@ -246,6 +266,9 @@ where
246266
};
247267
let len = base.len(self)?;
248268
let field_layout = base.layout().field(self, 0);
269+
// Ensure that all the offsets are in-bounds once, up-front.
270+
base.offset(len * stride, self.layout_of(self.tcx.types.unit).unwrap(), self)?;
271+
// Create the iterator.
249272
Ok(ArrayIterator { base, range: 0..len, stride, field_layout, _phantom: PhantomData })
250273
}
251274

@@ -303,7 +326,7 @@ where
303326
};
304327
let layout = self.layout_of(ty)?;
305328

306-
base.offset_with_meta(from_offset, meta, layout, self)
329+
base.offset_with_meta(from_offset, OffsetMode::Inbounds, meta, layout, self)
307330
}
308331

309332
/// Applying a general projection

tests/ui/consts/extra-const-ub/detect-extra-ub.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,7 @@ const PARTIAL_POINTER: () = unsafe {
8888
const VALID_ENUM1: E = { let e = E::A; e };
8989
const VALID_ENUM2: Result<&'static [u8], ()> = { let e = Err(()); e };
9090

91+
// Htting the (non-integer) array code in validation with an immediate local.
92+
const VALID_ARRAY: [Option<i32>; 0] = { let e = [None; 0]; e };
93+
9194
fn main() {}

0 commit comments

Comments
 (0)