Skip to content

Commit 0bbab7d

Browse files
committed
Auto merge of #64470 - ecstatic-morse:split-promotion-and-validation, r=eddyb,oli-obk
Implement dataflow-based const validation This PR adds a separate, dataflow-enabled pass that checks the bodies of `const`s, `static`s and `const fn`s for [const safety](https://github.com/rust-rfcs/const-eval/blob/master/const.md). This is based on my work in #63860, which tried to integrate into the existing pass in [`qualify_consts.rs`](https://github.com/rust-lang/rust/blob/master/src/librustc_mir/transform/qualify_consts.rs). However, the resulting pass was even more unwieldy than the original. Unlike its predecessor, this PR is designed to be combined with #63812 to replace the existing pass completely. The new checker lives in [`librustc_mir/transform/check_consts`](https://github.com/ecstatic-morse/rust/tree/split-promotion-and-validation/src/librustc_mir/transform/check_consts). [`qualifs.rs`](https://github.com/ecstatic-morse/rust/blob/split-promotion-and-validation/src/librustc_mir/transform/check_consts/qualifs.rs) contains small modifications to the existing `Qualif` trait and its implementors, but is mostly unchanged except for the removal of `IsNotPromotable` and `IsNotImplicitlyPromotable`, which are only necessary for promotion. [`resolver.rs`](https://github.com/ecstatic-morse/rust/blob/split-promotion-and-validation/src/librustc_mir/transform/check_consts/resolver.rs) contains the dataflow analysis used to propagate qualifs between locals. Finally, [`validation.rs`](https://github.com/ecstatic-morse/rust/blob/split-promotion-and-validation/src/librustc_mir/transform/check_consts/validation.rs) contains a refactored version of the existing [`Visitor`](https://github.com/rust-lang/rust/blob/ca3766e2e58f462a20922e42c821a37eaf0e13db/src/librustc_mir/transform/qualify_consts.rs#L1024) in `qualfy_consts.rs`. All errors have been associated with a `struct` to make [comparison with the existing pass](https://github.com/ecstatic-morse/rust/blob/1c19f2d540ca0a964900449d79a5d5181b43146d/src/librustc_mir/transform/qualify_consts.rs#L1006) simple. The existing validation logic in [`qualify_consts`](https://github.com/rust-lang/rust/blob/master/src/librustc_mir/transform/qualify_consts.rs) has been modified to allow it to run in parallel with the new validator. If [`use_new_validator`](https://github.com/rust-lang/rust/pull/64470/files#diff-c2552a106550d05b69d5e07612f0f812R950) is not set, the old validation will be responsible for actually generating the errors, but those errors can be compared with the ones from the new validator.
2 parents b61e694 + 0bf1a80 commit 0bbab7d

28 files changed

+2009
-290
lines changed

src/librustc/session/config.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1359,6 +1359,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
13591359
"describes how to render the `rendered` field of json diagnostics"),
13601360
unleash_the_miri_inside_of_you: bool = (false, parse_bool, [TRACKED],
13611361
"take the breaks off const evaluation. NOTE: this is unsound"),
1362+
suppress_const_validation_back_compat_ice: bool = (false, parse_bool, [TRACKED],
1363+
"silence ICE triggered when the new const validator disagrees with the old"),
13621364
osx_rpath_install_name: bool = (false, parse_bool, [TRACKED],
13631365
"pass `-install_name @rpath/...` to the macOS linker"),
13641366
sanitizer: Option<Sanitizer> = (None, parse_sanitizer, [TRACKED],
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
use rustc::mir::visit::Visitor;
2+
use rustc::mir::{self, Local, Location};
3+
use rustc::ty::{self, TyCtxt};
4+
use rustc_data_structures::bit_set::BitSet;
5+
use syntax_pos::DUMMY_SP;
6+
7+
use crate::dataflow::{self, GenKillSet};
8+
9+
/// Whether a borrow to a `Local` has been created that could allow that `Local` to be mutated
10+
/// indirectly. This could either be a mutable reference (`&mut`) or a shared borrow if the type of
11+
/// that `Local` allows interior mutability. Operations that can mutate local's indirectly include:
12+
/// assignments through a pointer (`*p = 42`), function calls, drop terminators and inline assembly.
13+
///
14+
/// If this returns false for a `Local` at a given statement (or terminator), that `Local` could
15+
/// not possibly have been mutated indirectly prior to that statement.
16+
#[derive(Copy, Clone)]
17+
pub struct IndirectlyMutableLocals<'mir, 'tcx> {
18+
body: &'mir mir::Body<'tcx>,
19+
tcx: TyCtxt<'tcx>,
20+
param_env: ty::ParamEnv<'tcx>,
21+
}
22+
23+
impl<'mir, 'tcx> IndirectlyMutableLocals<'mir, 'tcx> {
24+
pub fn new(
25+
tcx: TyCtxt<'tcx>,
26+
body: &'mir mir::Body<'tcx>,
27+
param_env: ty::ParamEnv<'tcx>,
28+
) -> Self {
29+
IndirectlyMutableLocals { body, tcx, param_env }
30+
}
31+
32+
fn transfer_function<'a>(
33+
&self,
34+
trans: &'a mut GenKillSet<Local>,
35+
) -> TransferFunction<'a, 'mir, 'tcx> {
36+
TransferFunction {
37+
body: self.body,
38+
tcx: self.tcx,
39+
param_env: self.param_env,
40+
trans
41+
}
42+
}
43+
}
44+
45+
impl<'mir, 'tcx> dataflow::BitDenotation<'tcx> for IndirectlyMutableLocals<'mir, 'tcx> {
46+
type Idx = Local;
47+
48+
fn name() -> &'static str { "mut_borrowed_locals" }
49+
50+
fn bits_per_block(&self) -> usize {
51+
self.body.local_decls.len()
52+
}
53+
54+
fn start_block_effect(&self, _entry_set: &mut BitSet<Local>) {
55+
// Nothing is borrowed on function entry
56+
}
57+
58+
fn statement_effect(
59+
&self,
60+
trans: &mut GenKillSet<Local>,
61+
loc: Location,
62+
) {
63+
let stmt = &self.body[loc.block].statements[loc.statement_index];
64+
self.transfer_function(trans).visit_statement(stmt, loc);
65+
}
66+
67+
fn terminator_effect(
68+
&self,
69+
trans: &mut GenKillSet<Local>,
70+
loc: Location,
71+
) {
72+
let terminator = self.body[loc.block].terminator();
73+
self.transfer_function(trans).visit_terminator(terminator, loc);
74+
}
75+
76+
fn propagate_call_return(
77+
&self,
78+
_in_out: &mut BitSet<Local>,
79+
_call_bb: mir::BasicBlock,
80+
_dest_bb: mir::BasicBlock,
81+
_dest_place: &mir::Place<'tcx>,
82+
) {
83+
// Nothing to do when a call returns successfully
84+
}
85+
}
86+
87+
impl<'mir, 'tcx> dataflow::BottomValue for IndirectlyMutableLocals<'mir, 'tcx> {
88+
// bottom = unborrowed
89+
const BOTTOM_VALUE: bool = false;
90+
}
91+
92+
/// A `Visitor` that defines the transfer function for `IndirectlyMutableLocals`.
93+
struct TransferFunction<'a, 'mir, 'tcx> {
94+
trans: &'a mut GenKillSet<Local>,
95+
body: &'mir mir::Body<'tcx>,
96+
tcx: TyCtxt<'tcx>,
97+
param_env: ty::ParamEnv<'tcx>,
98+
}
99+
100+
impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> {
101+
fn visit_rvalue(
102+
&mut self,
103+
rvalue: &mir::Rvalue<'tcx>,
104+
location: Location,
105+
) {
106+
if let mir::Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue {
107+
let is_mut = match kind {
108+
mir::BorrowKind::Mut { .. } => true,
109+
110+
| mir::BorrowKind::Shared
111+
| mir::BorrowKind::Shallow
112+
| mir::BorrowKind::Unique
113+
=> {
114+
!borrowed_place
115+
.ty(self.body, self.tcx)
116+
.ty
117+
.is_freeze(self.tcx, self.param_env, DUMMY_SP)
118+
}
119+
};
120+
121+
if is_mut {
122+
match borrowed_place.base {
123+
mir::PlaceBase::Local(borrowed_local) if !borrowed_place.is_indirect()
124+
=> self.trans.gen(borrowed_local),
125+
126+
_ => (),
127+
}
128+
}
129+
}
130+
131+
self.super_rvalue(rvalue, location);
132+
}
133+
134+
135+
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
136+
// This method purposely does nothing except call `super_terminator`. It exists solely to
137+
// document the subtleties around drop terminators.
138+
139+
self.super_terminator(terminator, location);
140+
141+
if let mir::TerminatorKind::Drop { location: _, .. }
142+
| mir::TerminatorKind::DropAndReplace { location: _, .. } = &terminator.kind
143+
{
144+
// Although drop terminators mutably borrow the location being dropped, that borrow
145+
// cannot live beyond the drop terminator because the dropped location is invalidated.
146+
}
147+
}
148+
}

src/librustc_mir/dataflow/impls/mod.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ use super::drop_flag_effects_for_function_entry;
1818
use super::drop_flag_effects_for_location;
1919
use super::on_lookup_result_bits;
2020

21-
mod storage_liveness;
22-
23-
pub use self::storage_liveness::*;
24-
2521
mod borrowed_locals;
22+
mod indirect_mutation;
23+
mod storage_liveness;
2624

2725
pub use self::borrowed_locals::*;
26+
pub use self::indirect_mutation::IndirectlyMutableLocals;
27+
pub use self::storage_liveness::*;
2828

2929
pub(super) mod borrows;
3030

src/librustc_mir/dataflow/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub use self::impls::DefinitelyInitializedPlaces;
2323
pub use self::impls::EverInitializedPlaces;
2424
pub use self::impls::borrows::Borrows;
2525
pub use self::impls::HaveBeenBorrowedLocals;
26+
pub use self::impls::IndirectlyMutableLocals;
2627
pub use self::at_location::{FlowAtLocation, FlowsAtLocation};
2728
pub(crate) use self::drop_flag_effects::*;
2829

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
//! Check the bodies of `const`s, `static`s and `const fn`s for illegal operations.
2+
//!
3+
//! This module will eventually replace the parts of `qualify_consts.rs` that check whether a local
4+
//! has interior mutability or needs to be dropped, as well as the visitor that emits errors when
5+
//! it finds operations that are invalid in a certain context.
6+
7+
use rustc::hir::def_id::DefId;
8+
use rustc::mir;
9+
use rustc::ty::{self, TyCtxt};
10+
11+
pub use self::qualifs::Qualif;
12+
13+
pub mod ops;
14+
mod qualifs;
15+
mod resolver;
16+
pub mod validation;
17+
18+
/// Information about the item currently being validated, as well as a reference to the global
19+
/// context.
20+
pub struct Item<'mir, 'tcx> {
21+
body: &'mir mir::Body<'tcx>,
22+
tcx: TyCtxt<'tcx>,
23+
def_id: DefId,
24+
param_env: ty::ParamEnv<'tcx>,
25+
mode: validation::Mode,
26+
}
27+
28+
impl Item<'mir, 'tcx> {
29+
pub fn new(
30+
tcx: TyCtxt<'tcx>,
31+
def_id: DefId,
32+
body: &'mir mir::Body<'tcx>,
33+
) -> Self {
34+
let param_env = tcx.param_env(def_id);
35+
let mode = validation::Mode::for_item(tcx, def_id)
36+
.expect("const validation must only be run inside a const context");
37+
38+
Item {
39+
body,
40+
tcx,
41+
def_id,
42+
param_env,
43+
mode,
44+
}
45+
}
46+
}
47+
48+
49+
fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool {
50+
Some(def_id) == tcx.lang_items().panic_fn() ||
51+
Some(def_id) == tcx.lang_items().begin_panic_fn()
52+
}

0 commit comments

Comments
 (0)