Skip to content

Commit

Permalink
interpret: fix dealing with overflow during slice indexing
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Sep 14, 2024
1 parent 0307e40 commit 3b806d3
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 7 deletions.
7 changes: 3 additions & 4 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,9 +599,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let count = self.read_target_usize(count)?;
let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap())?;
let (size, align) = (layout.size, layout.align.abi);
// `checked_mul` enforces a too small bound (the correct one would probably be target_isize_max),
// but no actual allocation can be big enough for the difference to be noticeable.
let size = size.checked_mul(count, self).ok_or_else(|| {

let size = self.compute_size_in_bytes(size, count).ok_or_else(|| {
err_ub_custom!(
fluent::const_eval_size_overflow,
name = if nonoverlapping { "copy_nonoverlapping" } else { "copy" }
Expand Down Expand Up @@ -649,7 +648,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {

// `checked_mul` enforces a too small bound (the correct one would probably be target_isize_max),
// but no actual allocation can be big enough for the difference to be noticeable.
let len = layout.size.checked_mul(count, self).ok_or_else(|| {
let len = self.compute_size_in_bytes(layout.size, count).ok_or_else(|| {
err_ub_custom!(fluent::const_eval_size_overflow, name = "write_bytes")
})?;

Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_const_eval/src/interpret/operator.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use either::Either;
use rustc_apfloat::{Float, FloatConvert};
use rustc_middle::mir::interpret::{InterpResult, Scalar};
use rustc_middle::mir::interpret::{InterpResult, PointerArithmetic, Scalar};
use rustc_middle::mir::NullOp;
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, FloatTy, ScalarInt, Ty};
use rustc_middle::{bug, mir, span_bug};
use rustc_span::symbol::sym;
use rustc_target::abi::Size;
use tracing::trace;

use super::{throw_ub, ImmTy, InterpCx, Machine, MemPlaceMeta};
Expand Down Expand Up @@ -287,6 +288,20 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
})
}

/// Computes the total size of this access, `count * elem_size`,
/// checking for overflow beyond isize::MAX.
pub(super) fn compute_size_in_bytes(&self, elem_size: Size, count: u64) -> Option<Size> {
// `checked_mul` applies `u64` limits independent of the target pointer size... but the
// subsequent check for `max_size_of_val` means we also handle 32bit targets correctly.
// (We cannot use `Size::checked_mul` as that enforces `obj_size_bound` as the limit, which
// would be wrong here.)
elem_size
.bytes()
.checked_mul(count)
.map(Size::from_bytes)
.filter(|&total| total <= self.max_size_of_val())
}

fn binary_ptr_op(
&self,
bin_op: mir::BinOp,
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc_target::abi::{self, Size, VariantIdx};
use tracing::{debug, instrument};

use super::{
throw_ub, throw_unsup, InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy,
err_ub, throw_ub, throw_unsup, InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy,
Provenance, Scalar,
};

Expand Down Expand Up @@ -229,7 +229,11 @@ where
// This can only be reached in ConstProp and non-rustc-MIR.
throw_ub!(BoundsCheckFailed { len, index });
}
let offset = stride * index; // `Size` multiplication
// With raw slices, `len` can be so big that this *can* overflow.
let offset = self
.compute_size_in_bytes(stride, index)
.ok_or_else(|| err_ub!(PointerArithOverflow))?;

// All fields have the same layout.
let field_layout = base.layout().field(self, 0);
(offset, field_layout)
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/consts/slice-index-overflow-issue-130284.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const C: () = {
let value = [1, 2];
let ptr = value.as_ptr().wrapping_add(2);
let fat = std::ptr::slice_from_raw_parts(ptr, usize::MAX);
unsafe {
// This used to ICE, but it should just report UB.
let _ice = (*fat)[usize::MAX - 1];
//~^ERROR: constant value failed
//~| overflow
}
};

fn main() {}
9 changes: 9 additions & 0 deletions tests/ui/consts/slice-index-overflow-issue-130284.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0080]: evaluation of constant value failed
--> $DIR/slice-index-overflow-issue-130284.rs:7:20
|
LL | let _ice = (*fat)[usize::MAX - 1];
| ^^^^^^^^^^^^^^^^^^^^^^ overflowing pointer arithmetic: the total offset in bytes does not fit in an `isize`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0080`.

0 comments on commit 3b806d3

Please sign in to comment.