Skip to content

Commit 33c816b

Browse files
committed
Revert "Auto merge of #119627 - oli-obk:const_prop_lint_n̵o̵n̵sense, r=cjgillot"
This reverts commit 68411c9, reversing changes made to 7ffc697.
1 parent 5bd5d21 commit 33c816b

File tree

14 files changed

+437
-404
lines changed

14 files changed

+437
-404
lines changed

compiler/rustc_const_eval/src/errors.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,9 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
864864
InvalidProgramInfo::FnAbiAdjustForForeignAbi(_) => {
865865
rustc_middle::error::middle_adjust_for_foreign_abi_error
866866
}
867+
InvalidProgramInfo::ConstPropNonsense => {
868+
panic!("We had const-prop nonsense, this should never be printed")
869+
}
867870
}
868871
}
869872
fn add_args<G: EmissionGuarantee>(
@@ -872,7 +875,9 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
872875
builder: &mut DiagnosticBuilder<'_, G>,
873876
) {
874877
match self {
875-
InvalidProgramInfo::TooGeneric | InvalidProgramInfo::AlreadyReported(_) => {}
878+
InvalidProgramInfo::TooGeneric
879+
| InvalidProgramInfo::AlreadyReported(_)
880+
| InvalidProgramInfo::ConstPropNonsense => {}
876881
InvalidProgramInfo::Layout(e) => {
877882
// The level doesn't matter, `diag` is consumed without it being used.
878883
let dummy_level = Level::Bug;

compiler/rustc_const_eval/src/interpret/operand.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
643643
let layout = self.layout_of_local(frame, local, layout)?;
644644
let op = *frame.locals[local].access()?;
645645
if matches!(op, Operand::Immediate(_)) {
646-
assert!(!layout.is_unsized());
646+
if layout.is_unsized() {
647+
// ConstProp marks *all* locals as `Immediate::Uninit` since it cannot
648+
// efficiently check whether they are sized. We have to catch that case here.
649+
throw_inval!(ConstPropNonsense);
650+
}
647651
}
648652
Ok(OpTy { op, layout })
649653
}

compiler/rustc_const_eval/src/interpret/place.rs

+16-3
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,11 @@ where
519519
} else {
520520
// Unsized `Local` isn't okay (we cannot store the metadata).
521521
match frame_ref.locals[local].access()? {
522-
Operand::Immediate(_) => bug!(),
522+
Operand::Immediate(_) => {
523+
// ConstProp marks *all* locals as `Immediate::Uninit` since it cannot
524+
// efficiently check whether they are sized. We have to catch that case here.
525+
throw_inval!(ConstPropNonsense);
526+
}
523527
Operand::Indirect(mplace) => Place::Ptr(*mplace),
524528
}
525529
};
@@ -812,8 +816,17 @@ where
812816
// avoid force_allocation.
813817
let src = match self.read_immediate_raw(src)? {
814818
Right(src_val) => {
815-
assert!(!src.layout().is_unsized());
816-
assert!(!dest.layout().is_unsized());
819+
// FIXME(const_prop): Const-prop can possibly evaluate an
820+
// unsized copy operation when it thinks that the type is
821+
// actually sized, due to a trivially false where-clause
822+
// predicate like `where Self: Sized` with `Self = dyn Trait`.
823+
// See #102553 for an example of such a predicate.
824+
if src.layout().is_unsized() {
825+
throw_inval!(ConstPropNonsense);
826+
}
827+
if dest.layout().is_unsized() {
828+
throw_inval!(ConstPropNonsense);
829+
}
817830
assert_eq!(src.layout().size, dest.layout().size);
818831
// Yay, we got a value that we can write directly.
819832
return if layout_compat {

compiler/rustc_const_eval/src/interpret/projection.rs

+28-21
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,11 @@ where
153153

154154
// Offset may need adjustment for unsized fields.
155155
let (meta, offset) = if field_layout.is_unsized() {
156-
assert!(!base.layout().is_sized());
156+
if base.layout().is_sized() {
157+
// An unsized field of a sized type? Sure...
158+
// But const-prop actually feeds us such nonsense MIR! (see test `const_prop/issue-86351.rs`)
159+
throw_inval!(ConstPropNonsense);
160+
}
157161
let base_meta = base.meta();
158162
// Re-use parent metadata to determine dynamic field layout.
159163
// With custom DSTS, this *will* execute user-defined code, but the same
@@ -201,26 +205,29 @@ where
201205
// see https://github.com/rust-lang/rust/issues/93688#issuecomment-1032929496.)
202206
// So we just "offset" by 0.
203207
let layout = base.layout().for_variant(self, variant);
204-
// In the future we might want to allow this to permit code like this:
205-
// (this is a Rust/MIR pseudocode mix)
206-
// ```
207-
// enum Option2 {
208-
// Some(i32, !),
209-
// None,
210-
// }
211-
//
212-
// fn panic() -> ! { panic!() }
213-
//
214-
// let x: Option2;
215-
// x.Some.0 = 42;
216-
// x.Some.1 = panic();
217-
// SetDiscriminant(x, Some);
218-
// ```
219-
// However, for now we don't generate such MIR, and this check here *has* found real
220-
// bugs (see https://github.com/rust-lang/rust/issues/115145), so we will keep rejecting
221-
// it.
222-
assert!(!layout.abi.is_uninhabited());
223-
208+
if layout.abi.is_uninhabited() {
209+
// `read_discriminant` should have excluded uninhabited variants... but ConstProp calls
210+
// us on dead code.
211+
// In the future we might want to allow this to permit code like this:
212+
// (this is a Rust/MIR pseudocode mix)
213+
// ```
214+
// enum Option2 {
215+
// Some(i32, !),
216+
// None,
217+
// }
218+
//
219+
// fn panic() -> ! { panic!() }
220+
//
221+
// let x: Option2;
222+
// x.Some.0 = 42;
223+
// x.Some.1 = panic();
224+
// SetDiscriminant(x, Some);
225+
// ```
226+
// However, for now we don't generate such MIR, and this check here *has* found real
227+
// bugs (see https://github.com/rust-lang/rust/issues/115145), so we will keep rejecting
228+
// it.
229+
throw_inval!(ConstPropNonsense)
230+
}
224231
// This cannot be `transmute` as variants *can* have a smaller size than the entire enum.
225232
base.offset(Size::ZERO, layout, self)
226233
}

compiler/rustc_const_eval/src/util/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ pub use self::type_name::type_name;
1414
/// Classify whether an operator is "left-homogeneous", i.e., the LHS has the
1515
/// same type as the result.
1616
#[inline]
17-
pub fn binop_left_homogeneous(op: mir::BinOp) -> bool {
17+
pub(crate) fn binop_left_homogeneous(op: mir::BinOp) -> bool {
1818
use rustc_middle::mir::BinOp::*;
1919
match op {
2020
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
@@ -26,7 +26,7 @@ pub fn binop_left_homogeneous(op: mir::BinOp) -> bool {
2626
/// Classify whether an operator is "right-homogeneous", i.e., the RHS has the
2727
/// same type as the LHS.
2828
#[inline]
29-
pub fn binop_right_homogeneous(op: mir::BinOp) -> bool {
29+
pub(crate) fn binop_right_homogeneous(op: mir::BinOp) -> bool {
3030
use rustc_middle::mir::BinOp::*;
3131
match op {
3232
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor

compiler/rustc_middle/src/mir/interpret/error.rs

+2
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,8 @@ pub enum InvalidProgramInfo<'tcx> {
200200
/// (which unfortunately typeck does not reject).
201201
/// Not using `FnAbiError` as that contains a nested `LayoutError`.
202202
FnAbiAdjustForForeignAbi(call::AdjustForForeignAbiError),
203+
/// We are runnning into a nonsense situation due to ConstProp violating our invariants.
204+
ConstPropNonsense,
203205
}
204206

205207
/// Details of why a pointer had to be in-bounds.

compiler/rustc_mir_transform/src/const_prop.rs

+166-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
//! Propagates constants for early reporting of statically known
22
//! assertion failures
33
4+
use rustc_const_eval::interpret::{
5+
self, compile_time_machine, AllocId, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
6+
InterpResult, OpTy, PlaceTy, Pointer,
7+
};
8+
use rustc_data_structures::fx::FxHashSet;
49
use rustc_index::bit_set::BitSet;
510
use rustc_index::IndexVec;
611
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
712
use rustc_middle::mir::*;
8-
use rustc_middle::ty::{ParamEnv, TyCtxt};
13+
use rustc_middle::query::TyCtxtAt;
14+
use rustc_middle::ty::layout::TyAndLayout;
15+
use rustc_middle::ty::{self, ParamEnv, TyCtxt};
16+
use rustc_span::def_id::DefId;
917
use rustc_target::abi::Size;
18+
use rustc_target::spec::abi::Abi as CallAbi;
1019

1120
/// The maximum number of bytes that we'll allocate space for a local or the return value.
1221
/// Needed for #66397, because otherwise we eval into large places and that can cause OOM or just
@@ -40,6 +49,162 @@ pub(crate) macro throw_machine_stop_str($($tt:tt)*) {{
4049
throw_machine_stop!(Zst)
4150
}}
4251

52+
pub(crate) struct ConstPropMachine<'mir, 'tcx> {
53+
/// The virtual call stack.
54+
stack: Vec<Frame<'mir, 'tcx>>,
55+
pub written_only_inside_own_block_locals: FxHashSet<Local>,
56+
pub can_const_prop: IndexVec<Local, ConstPropMode>,
57+
}
58+
59+
impl ConstPropMachine<'_, '_> {
60+
pub fn new(can_const_prop: IndexVec<Local, ConstPropMode>) -> Self {
61+
Self {
62+
stack: Vec::new(),
63+
written_only_inside_own_block_locals: Default::default(),
64+
can_const_prop,
65+
}
66+
}
67+
}
68+
69+
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> {
70+
compile_time_machine!(<'mir, 'tcx>);
71+
72+
const PANIC_ON_ALLOC_FAIL: bool = true; // all allocations are small (see `MAX_ALLOC_LIMIT`)
73+
74+
const POST_MONO_CHECKS: bool = false; // this MIR is still generic!
75+
76+
type MemoryKind = !;
77+
78+
#[inline(always)]
79+
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
80+
false // no reason to enforce alignment
81+
}
82+
83+
#[inline(always)]
84+
fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool {
85+
false // for now, we don't enforce validity
86+
}
87+
88+
fn load_mir(
89+
_ecx: &InterpCx<'mir, 'tcx, Self>,
90+
_instance: ty::InstanceDef<'tcx>,
91+
) -> InterpResult<'tcx, &'tcx Body<'tcx>> {
92+
throw_machine_stop_str!("calling functions isn't supported in ConstProp")
93+
}
94+
95+
fn panic_nounwind(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _msg: &str) -> InterpResult<'tcx> {
96+
throw_machine_stop_str!("panicking isn't supported in ConstProp")
97+
}
98+
99+
fn find_mir_or_eval_fn(
100+
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
101+
_instance: ty::Instance<'tcx>,
102+
_abi: CallAbi,
103+
_args: &[FnArg<'tcx>],
104+
_destination: &PlaceTy<'tcx>,
105+
_target: Option<BasicBlock>,
106+
_unwind: UnwindAction,
107+
) -> InterpResult<'tcx, Option<(&'mir Body<'tcx>, ty::Instance<'tcx>)>> {
108+
Ok(None)
109+
}
110+
111+
fn call_intrinsic(
112+
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
113+
_instance: ty::Instance<'tcx>,
114+
_args: &[OpTy<'tcx>],
115+
_destination: &PlaceTy<'tcx>,
116+
_target: Option<BasicBlock>,
117+
_unwind: UnwindAction,
118+
) -> InterpResult<'tcx> {
119+
throw_machine_stop_str!("calling intrinsics isn't supported in ConstProp")
120+
}
121+
122+
fn assert_panic(
123+
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
124+
_msg: &rustc_middle::mir::AssertMessage<'tcx>,
125+
_unwind: rustc_middle::mir::UnwindAction,
126+
) -> InterpResult<'tcx> {
127+
bug!("panics terminators are not evaluated in ConstProp")
128+
}
129+
130+
fn binary_ptr_op(
131+
_ecx: &InterpCx<'mir, 'tcx, Self>,
132+
_bin_op: BinOp,
133+
_left: &ImmTy<'tcx>,
134+
_right: &ImmTy<'tcx>,
135+
) -> InterpResult<'tcx, (ImmTy<'tcx>, bool)> {
136+
// We can't do this because aliasing of memory can differ between const eval and llvm
137+
throw_machine_stop_str!("pointer arithmetic or comparisons aren't supported in ConstProp")
138+
}
139+
140+
fn before_access_local_mut<'a>(
141+
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
142+
frame: usize,
143+
local: Local,
144+
) -> InterpResult<'tcx> {
145+
assert_eq!(frame, 0);
146+
match ecx.machine.can_const_prop[local] {
147+
ConstPropMode::NoPropagation => {
148+
throw_machine_stop_str!(
149+
"tried to write to a local that is marked as not propagatable"
150+
)
151+
}
152+
ConstPropMode::OnlyInsideOwnBlock => {
153+
ecx.machine.written_only_inside_own_block_locals.insert(local);
154+
}
155+
ConstPropMode::FullConstProp => {}
156+
}
157+
Ok(())
158+
}
159+
160+
fn before_access_global(
161+
_tcx: TyCtxtAt<'tcx>,
162+
_machine: &Self,
163+
_alloc_id: AllocId,
164+
alloc: ConstAllocation<'tcx>,
165+
_static_def_id: Option<DefId>,
166+
is_write: bool,
167+
) -> InterpResult<'tcx> {
168+
if is_write {
169+
throw_machine_stop_str!("can't write to global");
170+
}
171+
// If the static allocation is mutable, then we can't const prop it as its content
172+
// might be different at runtime.
173+
if alloc.inner().mutability.is_mut() {
174+
throw_machine_stop_str!("can't access mutable globals in ConstProp");
175+
}
176+
177+
Ok(())
178+
}
179+
180+
#[inline(always)]
181+
fn expose_ptr(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx> {
182+
throw_machine_stop_str!("exposing pointers isn't supported in ConstProp")
183+
}
184+
185+
#[inline(always)]
186+
fn init_frame_extra(
187+
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
188+
frame: Frame<'mir, 'tcx>,
189+
) -> InterpResult<'tcx, Frame<'mir, 'tcx>> {
190+
Ok(frame)
191+
}
192+
193+
#[inline(always)]
194+
fn stack<'a>(
195+
ecx: &'a InterpCx<'mir, 'tcx, Self>,
196+
) -> &'a [Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>] {
197+
&ecx.machine.stack
198+
}
199+
200+
#[inline(always)]
201+
fn stack_mut<'a>(
202+
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
203+
) -> &'a mut Vec<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
204+
&mut ecx.machine.stack
205+
}
206+
}
207+
43208
/// The mode that `ConstProp` is allowed to run in for a given `Local`.
44209
#[derive(Clone, Copy, Debug, PartialEq)]
45210
pub enum ConstPropMode {

0 commit comments

Comments
 (0)