Skip to content

Commit f25c190

Browse files
RalfJungRalfJung
committed
compile-time evaluation: emit a lint when a write through an immutable pointer occurs
1 parent a3234b4 commit f25c190

File tree

16 files changed

+380
-186
lines changed

16 files changed

+380
-186
lines changed

‎compiler/rustc_const_eval/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,9 @@ const_eval_validation_uninhabited_val = {$front_matter}: encountered a value of
461461
const_eval_validation_uninit = {$front_matter}: encountered uninitialized memory, but {$expected}
462462
const_eval_validation_unsafe_cell = {$front_matter}: encountered `UnsafeCell` in a `const`
463463
464+
const_eval_write_through_immutable_pointer =
465+
writing through a pointer that was derived from a shared (immutable) reference
466+
464467
const_eval_write_to_read_only =
465468
writing to {$allocation} which is read-only
466469
const_eval_zst_pointer_out_of_bounds =

‎compiler/rustc_const_eval/src/const_eval/error.rs

+36-8
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
use std::mem;
22

33
use rustc_errors::{DiagnosticArgValue, DiagnosticMessage, IntoDiagnostic, IntoDiagnosticArg};
4+
use rustc_hir::CRATE_HIR_ID;
45
use rustc_middle::mir::AssertKind;
6+
use rustc_middle::query::TyCtxtAt;
57
use rustc_middle::ty::TyCtxt;
68
use rustc_middle::ty::{layout::LayoutError, ConstInt};
79
use rustc_span::{ErrorGuaranteed, Span, Symbol, DUMMY_SP};
810

9-
use super::InterpCx;
11+
use super::{CompileTimeInterpreter, InterpCx};
1012
use crate::errors::{self, FrameNote, ReportErrorExt};
11-
use crate::interpret::{ErrorHandled, InterpError, InterpErrorInfo, Machine, MachineStopType};
13+
use crate::interpret::{ErrorHandled, InterpError, InterpErrorInfo, MachineStopType};
1214

1315
/// The CTFE machine has some custom error kinds.
1416
#[derive(Clone, Debug)]
@@ -57,16 +59,20 @@ impl<'tcx> Into<InterpErrorInfo<'tcx>> for ConstEvalErrKind {
5759
}
5860
}
5961

60-
pub fn get_span_and_frames<'tcx, 'mir, M: Machine<'mir, 'tcx>>(
61-
ecx: &InterpCx<'mir, 'tcx, M>,
62+
pub fn get_span_and_frames<'tcx, 'mir>(
63+
tcx: TyCtxtAt<'tcx>,
64+
machine: &CompileTimeInterpreter<'mir, 'tcx>,
6265
) -> (Span, Vec<errors::FrameNote>)
6366
where
6467
'tcx: 'mir,
6568
{
66-
let mut stacktrace = ecx.generate_stacktrace();
69+
let mut stacktrace =
70+
InterpCx::<CompileTimeInterpreter<'mir, 'tcx>>::generate_stacktrace_from_stack(
71+
&machine.stack,
72+
);
6773
// Filter out `requires_caller_location` frames.
68-
stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(*ecx.tcx));
69-
let span = stacktrace.first().map(|f| f.span).unwrap_or(ecx.tcx.span);
74+
stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(*tcx));
75+
let span = stacktrace.first().map(|f| f.span).unwrap_or(tcx.span);
7076

7177
let mut frames = Vec::new();
7278

@@ -87,7 +93,7 @@ where
8793

8894
let mut last_frame: Option<errors::FrameNote> = None;
8995
for frame_info in &stacktrace {
90-
let frame = frame_info.as_note(*ecx.tcx);
96+
let frame = frame_info.as_note(*tcx);
9197
match last_frame.as_mut() {
9298
Some(last_frame)
9399
if last_frame.span == frame.span
@@ -156,3 +162,25 @@ where
156162
}
157163
}
158164
}
165+
166+
/// Emit a lint from a const-eval situation.
167+
// Even if this is unused, please don't remove it -- chances are we will need to emit a lint during const-eval again in the future!
168+
pub(super) fn lint<'tcx, 'mir, L>(
169+
tcx: TyCtxtAt<'tcx>,
170+
machine: &CompileTimeInterpreter<'mir, 'tcx>,
171+
lint: &'static rustc_session::lint::Lint,
172+
decorator: impl FnOnce(Vec<errors::FrameNote>) -> L,
173+
) where
174+
L: for<'a> rustc_errors::DecorateLint<'a, ()>,
175+
{
176+
let (span, frames) = get_span_and_frames(tcx, machine);
177+
178+
tcx.emit_spanned_lint(
179+
lint,
180+
// We use the root frame for this so the crate that defines the const defines whether the
181+
// lint is emitted.
182+
machine.stack.first().and_then(|frame| frame.lint_root()).unwrap_or(CRATE_HIR_ID),
183+
span,
184+
decorator(frames),
185+
);
186+
}

‎compiler/rustc_const_eval/src/const_eval/eval_queries.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
338338
*ecx.tcx,
339339
error,
340340
None,
341-
|| super::get_span_and_frames(&ecx),
341+
|| super::get_span_and_frames(ecx.tcx, &ecx.machine),
342342
|span, frames| ConstEvalError {
343343
span,
344344
error_kind: kind,
@@ -419,7 +419,7 @@ pub fn const_report_error<'mir, 'tcx>(
419419
*ecx.tcx,
420420
error,
421421
None,
422-
|| crate::const_eval::get_span_and_frames(ecx),
422+
|| crate::const_eval::get_span_and_frames(ecx.tcx, &ecx.machine),
423423
move |span, frames| errors::UndefinedBehavior { span, ub_note, frames, raw_bytes },
424424
)
425425
}

‎compiler/rustc_const_eval/src/const_eval/machine.rs

+52-13
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,30 @@
1-
use rustc_hir::def::DefKind;
2-
use rustc_hir::LangItem;
3-
use rustc_middle::mir;
4-
use rustc_middle::mir::interpret::PointerArithmetic;
5-
use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
6-
use rustc_middle::ty::{self, TyCtxt};
7-
use rustc_span::Span;
81
use std::borrow::Borrow;
2+
use std::fmt;
93
use std::hash::Hash;
104
use std::ops::ControlFlow;
115

6+
use rustc_ast::Mutability;
127
use rustc_data_structures::fx::FxIndexMap;
138
use rustc_data_structures::fx::IndexEntry;
14-
use std::fmt;
15-
16-
use rustc_ast::Mutability;
9+
use rustc_hir::def::DefKind;
1710
use rustc_hir::def_id::DefId;
11+
use rustc_hir::LangItem;
12+
use rustc_middle::mir;
1813
use rustc_middle::mir::AssertMessage;
14+
use rustc_middle::query::TyCtxtAt;
15+
use rustc_middle::ty;
16+
use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
17+
use rustc_session::lint::builtin::WRITES_THROUGH_IMMUTABLE_POINTER;
1918
use rustc_span::symbol::{sym, Symbol};
19+
use rustc_span::Span;
2020
use rustc_target::abi::{Align, Size};
2121
use rustc_target::spec::abi::Abi as CallAbi;
2222

2323
use crate::errors::{LongRunning, LongRunningWarn};
2424
use crate::fluent_generated as fluent;
2525
use crate::interpret::{
26-
self, compile_time_machine, AllocId, ConstAllocation, FnArg, FnVal, Frame, ImmTy, InterpCx,
27-
InterpResult, OpTy, PlaceTy, Pointer, Scalar,
26+
self, compile_time_machine, AllocId, AllocRange, ConstAllocation, CtfeProvenance, FnArg, FnVal,
27+
Frame, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, PointerArithmetic, Scalar,
2828
};
2929

3030
use super::error::*;
@@ -671,7 +671,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
671671
}
672672

673673
fn before_access_global(
674-
_tcx: TyCtxt<'tcx>,
674+
_tcx: TyCtxtAt<'tcx>,
675675
machine: &Self,
676676
alloc_id: AllocId,
677677
alloc: ConstAllocation<'tcx>,
@@ -708,6 +708,45 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
708708
}
709709
}
710710
}
711+
712+
fn retag_ptr_value(
713+
ecx: &mut InterpCx<'mir, 'tcx, Self>,
714+
_kind: mir::RetagKind,
715+
val: &ImmTy<'tcx, CtfeProvenance>,
716+
) -> InterpResult<'tcx, ImmTy<'tcx, CtfeProvenance>> {
717+
if let ty::Ref(_, ty, mutbl) = val.layout.ty.kind()
718+
&& *mutbl == Mutability::Not
719+
&& ty.is_freeze(*ecx.tcx, ecx.param_env)
720+
{
721+
// This is a frozen shared reference, mark it immutable.
722+
let place = ecx.ref_to_mplace(val)?;
723+
let new_place = place.map_provenance(|p| p.map(CtfeProvenance::as_immutable));
724+
Ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout))
725+
} else {
726+
Ok(val.clone())
727+
}
728+
}
729+
730+
fn before_memory_write(
731+
tcx: TyCtxtAt<'tcx>,
732+
machine: &mut Self,
733+
_alloc_extra: &mut Self::AllocExtra,
734+
(_alloc_id, immutable): (AllocId, bool),
735+
range: AllocRange,
736+
) -> InterpResult<'tcx> {
737+
if range.size == Size::ZERO {
738+
// Nothing to check.
739+
return Ok(());
740+
}
741+
// Reject writes through immutable pointers.
742+
if immutable {
743+
super::lint(tcx, machine, WRITES_THROUGH_IMMUTABLE_POINTER, |frames| {
744+
crate::errors::WriteThroughImmutablePointer { frames }
745+
});
746+
}
747+
// Everything else is fine.
748+
Ok(())
749+
}
711750
}
712751

713752
// Please do not add any code below the above `Machine` trait impl. I (oli-obk) plan more cleanups

‎compiler/rustc_const_eval/src/errors.rs

+7
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,13 @@ pub struct ConstEvalError {
402402
pub frame_notes: Vec<FrameNote>,
403403
}
404404

405+
#[derive(LintDiagnostic)]
406+
#[diag(const_eval_write_through_immutable_pointer)]
407+
pub struct WriteThroughImmutablePointer {
408+
#[subdiagnostic]
409+
pub frames: Vec<FrameNote>,
410+
}
411+
405412
#[derive(Diagnostic)]
406413
#[diag(const_eval_nullary_intrinsic_fail)]
407414
pub struct NullaryIntrinsicError {

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

+8-7
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ use std::hash::Hash;
99
use rustc_apfloat::{Float, FloatConvert};
1010
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
1111
use rustc_middle::mir;
12+
use rustc_middle::query::TyCtxtAt;
13+
use rustc_middle::ty;
1214
use rustc_middle::ty::layout::TyAndLayout;
13-
use rustc_middle::ty::{self, TyCtxt};
1415
use rustc_span::def_id::DefId;
1516
use rustc_target::abi::Size;
1617
use rustc_target::spec::abi::Abi as CallAbi;
@@ -285,7 +286,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
285286
/// `def_id` is `Some` if this is the "lazy" allocation of a static.
286287
#[inline]
287288
fn before_access_global(
288-
_tcx: TyCtxt<'tcx>,
289+
_tcx: TyCtxtAt<'tcx>,
289290
_machine: &Self,
290291
_alloc_id: AllocId,
291292
_allocation: ConstAllocation<'tcx>,
@@ -380,7 +381,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
380381
/// need to mutate.
381382
#[inline(always)]
382383
fn before_memory_read(
383-
_tcx: TyCtxt<'tcx>,
384+
_tcx: TyCtxtAt<'tcx>,
384385
_machine: &Self,
385386
_alloc_extra: &Self::AllocExtra,
386387
_prov: (AllocId, Self::ProvenanceExtra),
@@ -392,7 +393,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
392393
/// Hook for performing extra checks on a memory write access.
393394
#[inline(always)]
394395
fn before_memory_write(
395-
_tcx: TyCtxt<'tcx>,
396+
_tcx: TyCtxtAt<'tcx>,
396397
_machine: &mut Self,
397398
_alloc_extra: &mut Self::AllocExtra,
398399
_prov: (AllocId, Self::ProvenanceExtra),
@@ -404,7 +405,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
404405
/// Hook for performing extra operations on a memory deallocation.
405406
#[inline(always)]
406407
fn before_memory_deallocation(
407-
_tcx: TyCtxt<'tcx>,
408+
_tcx: TyCtxtAt<'tcx>,
408409
_machine: &mut Self,
409410
_alloc_extra: &mut Self::AllocExtra,
410411
_prov: (AllocId, Self::ProvenanceExtra),
@@ -507,7 +508,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
507508
/// (CTFE and ConstProp) use the same instance. Here, we share that code.
508509
pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
509510
type Provenance = CtfeProvenance;
510-
type ProvenanceExtra = (); // FIXME extract the "immutable" bool?
511+
type ProvenanceExtra = bool; // the "immutable" flag
511512

512513
type ExtraFnVal = !;
513514

@@ -595,6 +596,6 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
595596
) -> Option<(AllocId, Size, Self::ProvenanceExtra)> {
596597
// We know `offset` is relative to the allocation, so we can use `into_parts`.
597598
let (prov, offset) = ptr.into_parts();
598-
Some((prov.alloc_id(), offset, ()))
599+
Some((prov.alloc_id(), offset, prov.immutable()))
599600
}
600601
}

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

+7-7
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
338338
// Let the machine take some extra action
339339
let size = alloc.size();
340340
M::before_memory_deallocation(
341-
*self.tcx,
341+
self.tcx,
342342
&mut self.machine,
343343
&mut alloc.extra,
344344
(alloc_id, prov),
@@ -545,7 +545,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
545545
(val, Some(def_id))
546546
}
547547
};
548-
M::before_access_global(*self.tcx, &self.machine, id, alloc, def_id, is_write)?;
548+
M::before_access_global(self.tcx, &self.machine, id, alloc, def_id, is_write)?;
549549
// We got tcx memory. Let the machine initialize its "extra" stuff.
550550
M::adjust_allocation(
551551
self,
@@ -610,7 +610,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
610610
)?;
611611
if let Some((alloc_id, offset, prov, alloc)) = ptr_and_alloc {
612612
let range = alloc_range(offset, size);
613-
M::before_memory_read(*self.tcx, &self.machine, &alloc.extra, (alloc_id, prov), range)?;
613+
M::before_memory_read(self.tcx, &self.machine, &alloc.extra, (alloc_id, prov), range)?;
614614
Ok(Some(AllocRef { alloc, range, tcx: *self.tcx, alloc_id }))
615615
} else {
616616
// Even in this branch we have to be sure that we actually access the allocation, in
@@ -671,13 +671,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
671671
{
672672
let parts = self.get_ptr_access(ptr, size)?;
673673
if let Some((alloc_id, offset, prov)) = parts {
674-
let tcx = *self.tcx;
674+
let tcx = self.tcx;
675675
// FIXME: can we somehow avoid looking up the allocation twice here?
676676
// We cannot call `get_raw_mut` inside `check_and_deref_ptr` as that would duplicate `&mut self`.
677677
let (alloc, machine) = self.get_alloc_raw_mut(alloc_id)?;
678678
let range = alloc_range(offset, size);
679679
M::before_memory_write(tcx, machine, &mut alloc.extra, (alloc_id, prov), range)?;
680-
Ok(Some(AllocRefMut { alloc, range, tcx, alloc_id }))
680+
Ok(Some(AllocRefMut { alloc, range, tcx: *tcx, alloc_id }))
681681
} else {
682682
Ok(None)
683683
}
@@ -1117,7 +1117,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
11171117
let src_alloc = self.get_alloc_raw(src_alloc_id)?;
11181118
let src_range = alloc_range(src_offset, size);
11191119
M::before_memory_read(
1120-
*tcx,
1120+
tcx,
11211121
&self.machine,
11221122
&src_alloc.extra,
11231123
(src_alloc_id, src_prov),
@@ -1147,7 +1147,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
11471147
let (dest_alloc, extra) = self.get_alloc_raw_mut(dest_alloc_id)?;
11481148
let dest_range = alloc_range(dest_offset, size * num_copies);
11491149
M::before_memory_write(
1150-
*tcx,
1150+
tcx,
11511151
extra,
11521152
&mut dest_alloc.extra,
11531153
(dest_alloc_id, dest_prov),

0 commit comments

Comments
 (0)