Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't UB on dangling ptr deref, instead check inbounds on projections #114330

Merged
merged 8 commits into from
Oct 16, 2023
1 change: 1 addition & 0 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,7 @@ impl fmt::Display for AlignFromBytesError {

impl Align {
pub const ONE: Align = Align { pow2: 0 };
// LLVM has a maximal supported alignment of 2^29, we inherit that.
pub const MAX: Align = Align { pow2: 29 };

#[inline]
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
const_eval_address_space_full =
there are no more free addresses in the address space
const_eval_align_check_failed = accessing memory with alignment {$has}, but alignment {$required} is required

const_eval_align_offset_invalid_align =
`align_offset` called with non-power-of-two align: {$target_align}

const_eval_alignment_check_failed =
accessing memory with alignment {$has}, but alignment {$required} is required
{$msg ->
[AccessedPtr] accessing memory
*[other] accessing memory based on pointer
} with alignment {$has}, but alignment {$required} is required

const_eval_already_reported =
an error has already been reported elsewhere (this should not usually be printed)
const_eval_assume_false =
Expand Down Expand Up @@ -61,7 +65,6 @@ const_eval_deref_coercion_non_const =
.target_note = deref defined here
const_eval_deref_function_pointer =
accessing {$allocation} which contains a function
const_eval_deref_test = dereferencing pointer failed
const_eval_deref_vtable_pointer =
accessing {$allocation} which contains a vtable
const_eval_different_allocations =
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::mem;

use either::{Left, Right};

use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -73,9 +75,9 @@ fn eval_body_using_ecx<'mir, 'tcx>(
None => InternKind::Constant,
}
};
ecx.machine.check_alignment = CheckAlignment::No; // interning doesn't need to respect alignment
let check_alignment = mem::replace(&mut ecx.machine.check_alignment, CheckAlignment::No); // interning doesn't need to respect alignment
intern_const_alloc_recursive(ecx, intern_kind, &ret)?;
// we leave alignment checks off, since this `ecx` will not be used for further evaluation anyway
ecx.machine.check_alignment = check_alignment;

debug!("eval_body_using_ecx done: {:?}", ret);
Ok(ret)
Expand Down
18 changes: 5 additions & 13 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use rustc_errors::{
use rustc_hir::ConstContext;
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
use rustc_middle::mir::interpret::{
CheckInAllocMsg, ExpectedKind, InterpError, InvalidMetaKind, InvalidProgramInfo, PointerKind,
ResourceExhaustionInfo, UndefinedBehaviorInfo, UnsupportedOpInfo, ValidationErrorInfo,
CheckInAllocMsg, ExpectedKind, InterpError, InvalidMetaKind, InvalidProgramInfo, Misalignment,
PointerKind, ResourceExhaustionInfo, UndefinedBehaviorInfo, UnsupportedOpInfo,
ValidationErrorInfo,
};
use rustc_middle::ty::{self, Ty};
use rustc_span::Span;
Expand Down Expand Up @@ -389,15 +390,6 @@ pub struct LiveDrop<'tcx> {
pub dropped_at: Option<Span>,
}

#[derive(LintDiagnostic)]
#[diag(const_eval_align_check_failed)]
pub struct AlignmentCheckFailed {
pub has: u64,
pub required: u64,
#[subdiagnostic]
pub frames: Vec<FrameNote>,
}

#[derive(Diagnostic)]
#[diag(const_eval_error, code = "E0080")]
pub struct ConstEvalError {
Expand Down Expand Up @@ -459,7 +451,6 @@ fn bad_pointer_message(msg: CheckInAllocMsg, handler: &Handler) -> String {
use crate::fluent_generated::*;

let msg = match msg {
CheckInAllocMsg::DerefTest => const_eval_deref_test,
CheckInAllocMsg::MemoryAccessTest => const_eval_memory_access_test,
CheckInAllocMsg::PointerArithmeticTest => const_eval_pointer_arithmetic_test,
CheckInAllocMsg::OffsetFromTest => const_eval_offset_from_test,
Expand Down Expand Up @@ -568,9 +559,10 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {

builder.set_arg("bad_pointer_message", bad_pointer_message(msg, handler));
}
AlignmentCheckFailed { required, has } => {
AlignmentCheckFailed(Misalignment { required, has }, msg) => {
builder.set_arg("required", required.bytes());
builder.set_arg("has", has.bytes());
builder.set_arg("msg", format!("{msg:?}"));
}
WriteToReadOnly(alloc) | DerefFunctionPointer(alloc) | DerefVTablePointer(alloc) => {
builder.set_arg("allocation", alloc);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory

#[inline(always)]
fn ecx(&self) -> &InterpCx<'mir, 'tcx, M> {
&self.ecx
self.ecx
}

fn visit_value(&mut self, mplace: &MPlaceTy<'tcx>) -> InterpResult<'tcx> {
Expand Down Expand Up @@ -259,15 +259,15 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
// to avoid could be expensive: on the potentially larger types, arrays and slices,
// rather than on all aggregates unconditionally.
if matches!(mplace.layout.ty.kind(), ty::Array(..) | ty::Slice(..)) {
let Some((size, align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
let Some((size, _align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
// We do the walk if we can't determine the size of the mplace: we may be
// dealing with extern types here in the future.
return Ok(true);
};

// If there is no provenance in this allocation, it does not contain references
// that point to another allocation, and we can avoid the interning walk.
if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr(), size, align)? {
if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr(), size)? {
if !alloc.has_provenance() {
return Ok(false);
}
Expand Down
25 changes: 9 additions & 16 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_middle::ty::layout::{LayoutOf as _, ValidityRequirement};
use rustc_middle::ty::GenericArgsRef;
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::{Abi, Align, Primitive, Size};
use rustc_target::abi::{Abi, Primitive, Size};

use super::{
util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
Expand Down Expand Up @@ -349,10 +349,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Check that the range between them is dereferenceable ("in-bounds or one past the
// end of the same allocation"). This is like the check in ptr_offset_inbounds.
let min_ptr = if dist >= 0 { b } else { a };
self.check_ptr_access_align(
self.check_ptr_access(
min_ptr,
Size::from_bytes(dist.unsigned_abs()),
Align::ONE,
CheckInAllocMsg::OffsetFromTest,
)?;

Expand Down Expand Up @@ -571,16 +570,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn ptr_offset_inbounds(
&self,
ptr: Pointer<Option<M::Provenance>>,
pointee_ty: Ty<'tcx>,
offset_count: i64,
offset_bytes: i64,
) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
// We cannot overflow i64 as a type's size must be <= isize::MAX.
let pointee_size = i64::try_from(self.layout_of(pointee_ty)?.size.bytes()).unwrap();
// The computed offset, in bytes, must not overflow an isize.
// `checked_mul` enforces a too small bound, but no actual allocation can be big enough for
// the difference to be noticeable.
let offset_bytes =
offset_count.checked_mul(pointee_size).ok_or(err_ub!(PointerArithOverflow))?;
// The offset being in bounds cannot rely on "wrapping around" the address space.
// So, first rule out overflows in the pointer arithmetic.
let offset_ptr = ptr.signed_offset(offset_bytes, self)?;
Expand All @@ -589,10 +580,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// pointers to be properly aligned (unlike a read/write operation).
let min_ptr = if offset_bytes >= 0 { ptr } else { offset_ptr };
// This call handles checking for integer/null pointers.
self.check_ptr_access_align(
self.check_ptr_access(
min_ptr,
Size::from_bytes(offset_bytes.unsigned_abs()),
Align::ONE,
CheckInAllocMsg::PointerArithmeticTest,
)?;
Ok(offset_ptr)
Expand Down Expand Up @@ -621,7 +611,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let src = self.read_pointer(src)?;
let dst = self.read_pointer(dst)?;

self.mem_copy(src, align, dst, align, size, nonoverlapping)
self.check_ptr_align(src, align)?;
self.check_ptr_align(dst, align)?;

self.mem_copy(src, dst, size, nonoverlapping)
}

pub(crate) fn write_bytes_intrinsic(
Expand Down Expand Up @@ -677,7 +670,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
size|
-> InterpResult<'tcx, &[u8]> {
let ptr = this.read_pointer(op)?;
let Some(alloc_ref) = self.get_ptr_alloc(ptr, size, Align::ONE)? else {
let Some(alloc_ref) = self.get_ptr_alloc(ptr, size)? else {
// zero-sized access
return Ok(&[]);
};
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
place: &PlaceTy<'tcx, Self::Provenance>,
) -> InterpResult<'tcx> {
// Without an aliasing model, all we can do is put `Uninit` into the place.
// Conveniently this also ensures that the place actually points to suitable memory.
ecx.write_uninit(place)
}

Expand Down
Loading
Loading