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

Replace fragile erroneous const sys #70820

Merged
merged 11 commits into from
Apr 24, 2020
14 changes: 14 additions & 0 deletions src/librustc_codegen_ssa/mir/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::base;
use crate::traits::*;
use rustc_errors::ErrorReported;
use rustc_middle::mir;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt, TyAndLayout};
use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
use rustc_target::abi::call::{FnAbi, PassMode};
Expand Down Expand Up @@ -189,6 +191,18 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info();

for const_ in &mir.required_consts {
if let Err(err) = fx.eval_mir_constant(const_) {
match err {
// errored or at least linted
ErrorHandled::Reported(ErrorReported) | ErrorHandled::Linted => {}
ErrorHandled::TooGeneric => {
span_bug!(const_.span, "codgen encountered polymorphic constant: {:?}", err)
}
}
}
}

let memory_locals = analyze::non_ssa_locals(&fx);

// Allocate variable and temp allocas
Expand Down
8 changes: 7 additions & 1 deletion src/librustc_middle/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ pub struct Body<'tcx> {
/// A span representing this MIR, for error reporting.
pub span: Span,

/// Constants that are required to evaluate successfully for this MIR to be well-formed.
/// We hold in this field all the constants we are not able to evaluate yet.
pub required_consts: Vec<Constant<'tcx>>,

/// The user may be writing e.g. &[(SOME_CELL, 42)][i].1 and this would get promoted, because
/// we'd statically know that no thing with interior mutability will ever be available to the
/// user without some serious unsafe code. Now this means that our promoted is actually
Expand Down Expand Up @@ -203,6 +207,7 @@ impl<'tcx> Body<'tcx> {
spread_arg: None,
var_debug_info,
span,
required_consts: Vec::new(),
ignore_interior_mut_in_const_validation: false,
control_flow_destroyed,
predecessor_cache: PredecessorCache::new(),
Expand All @@ -227,6 +232,7 @@ impl<'tcx> Body<'tcx> {
arg_count: 0,
spread_arg: None,
span: DUMMY_SP,
required_consts: Vec::new(),
control_flow_destroyed: Vec::new(),
generator_kind: None,
var_debug_info: Vec::new(),
Expand Down Expand Up @@ -2393,7 +2399,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
/// this does not necessarily mean that they are "==" in Rust -- in
/// particular one must be wary of `NaN`!

#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
#[derive(Clone, Copy, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
pub struct Constant<'tcx> {
pub span: Span,

Expand Down
5 changes: 5 additions & 0 deletions src/librustc_middle/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,11 @@ macro_rules! make_mir_visitor {
}

self.visit_span(&$($mutability)? body.span);

for const_ in &$($mutability)? body.required_consts {
let location = START_BLOCK.start_location();
self.visit_constant(const_, location);
}
}

fn super_basic_block_data(&mut self,
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,12 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
self.tcx
}

fn visit_body(&mut self, body: &mut Body<'tcx>) {
for (bb, data) in body.basic_blocks_mut().iter_enumerated_mut() {
self.visit_basic_block_data(bb, data);
}
}

fn visit_constant(&mut self, constant: &mut Constant<'tcx>, location: Location) {
trace!("visit_constant: {:?}", constant);
self.super_constant(constant, location);
Expand Down
12 changes: 11 additions & 1 deletion src/librustc_mir/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::subst::{Subst, SubstsRef};
use rustc_middle::ty::{self, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_session::config::Sanitizer;
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -123,6 +123,16 @@ impl Inliner<'tcx> {
continue;
};

// Copy only unevaluated constants from the callee_body into the caller_body.
// Although we are only pushing `ConstKind::Unevaluated` consts to
// `required_consts`, here we may not only have `ConstKind::Unevaluated`
// because we are calling `subst_and_normalize_erasing_regions`.
caller_body.required_consts.extend(
callee_body.required_consts.iter().copied().filter(|&constant| {
matches!(constant.literal.val, ConstKind::Unevaluated(_, _, _))
spastorino marked this conversation as resolved.
Show resolved Hide resolved
}),
);

let start = caller_body.basic_blocks().len();
debug!("attempting to inline callsite {:?} - body={:?}", callsite, callee_body);
if !self.inline_call(callsite, caller_body, callee_body) {
Expand Down
13 changes: 12 additions & 1 deletion src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use crate::{shim, util};
use required_consts::RequiredConstsVisitor;
use rustc_ast::ast;
use rustc_hir as hir;
use rustc_hir::def_id::{CrateNum, DefId, DefIdSet, LocalDefId, LOCAL_CRATE};
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_index::vec::IndexVec;
use rustc_middle::mir::{Body, ConstQualifs, MirPhase, Promoted};
use rustc_middle::mir::visit::Visitor as _;
use rustc_middle::mir::{traversal, Body, ConstQualifs, MirPhase, Promoted};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::steal::Steal;
use rustc_middle::ty::{InstanceDef, TyCtxt, TypeFoldable};
Expand All @@ -29,6 +31,7 @@ pub mod no_landing_pads;
pub mod promote_consts;
pub mod qualify_min_const_fn;
pub mod remove_noop_landing_pads;
pub mod required_consts;
pub mod rustc_peek;
pub mod simplify;
pub mod simplify_branches;
Expand Down Expand Up @@ -237,6 +240,14 @@ fn mir_validated(
let _ = tcx.mir_const_qualif(def_id);

let mut body = tcx.mir_const(def_id).steal();

let mut required_consts = Vec::new();
let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts);
for (bb, bb_data) in traversal::reverse_postorder(&body) {
required_consts_visitor.visit_basic_block_data(bb, bb_data);
}
body.required_consts = required_consts;

let promote_pass = promote_consts::PromoteTemps::default();
run_passes(
tcx,
Expand Down
23 changes: 23 additions & 0 deletions src/librustc_mir/transform/required_consts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{Constant, Location};
use rustc_middle::ty::ConstKind;

pub struct RequiredConstsVisitor<'a, 'tcx> {
required_consts: &'a mut Vec<Constant<'tcx>>,
}

impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> {
pub fn new(required_consts: &'a mut Vec<Constant<'tcx>>) -> Self {
RequiredConstsVisitor { required_consts }
}
}

impl<'a, 'tcx> Visitor<'tcx> for RequiredConstsVisitor<'a, 'tcx> {
fn visit_constant(&mut self, constant: &Constant<'tcx>, _: Location) {
let const_kind = constant.literal.val;

if let ConstKind::Unevaluated(_, _, _) = const_kind {
self.required_consts.push(*constant);
}
}
}
35 changes: 10 additions & 25 deletions src/librustc_mir/transform/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use rustc_index::bit_set::BitSet;
use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::TyCtxt;
use std::borrow::Cow;

pub struct SimplifyCfg {
Expand Down Expand Up @@ -400,33 +400,18 @@ impl<'a, 'tcx> Visitor<'tcx> for DeclMarker<'a, 'tcx> {
if location.statement_index != block.statements.len() {
let stmt = &block.statements[location.statement_index];

fn can_skip_constant(c: &ty::Const<'tcx>) -> bool {
// Keep assignments from unevaluated constants around, since the
// evaluation may report errors, even if the use of the constant
// is dead code.
!matches!(c.val, ty::ConstKind::Unevaluated(..))
}

fn can_skip_operand(o: &Operand<'_>) -> bool {
match o {
Operand::Copy(_) | Operand::Move(_) => true,
Operand::Constant(c) => can_skip_constant(c.literal),
}
}

if let StatementKind::Assign(box (dest, rvalue)) = &stmt.kind {
if !dest.is_indirect() && dest.local == *local {
let can_skip = match rvalue {
Rvalue::Use(op) => can_skip_operand(op),
Rvalue::Discriminant(_) => true,
Rvalue::BinaryOp(_, l, r) | Rvalue::CheckedBinaryOp(_, l, r) => {
can_skip_operand(l) && can_skip_operand(r)
}
Rvalue::Repeat(op, c) => can_skip_operand(op) && can_skip_constant(c),
Rvalue::AddressOf(_, _) => true,
Rvalue::Len(_) => true,
Rvalue::UnaryOp(_, op) => can_skip_operand(op),
Rvalue::Aggregate(_, operands) => operands.iter().all(can_skip_operand),
Rvalue::Use(_)
| Rvalue::Discriminant(_)
| Rvalue::BinaryOp(_, _, _)
| Rvalue::CheckedBinaryOp(_, _, _)
| Rvalue::Repeat(_, _)
| Rvalue::AddressOf(_, _)
| Rvalue::Len(_)
| Rvalue::UnaryOp(_, _)
| Rvalue::Aggregate(_, _) => true,

_ => false,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,41 +30,41 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 8, align: 4) {
alloc25+0╼ 03 00 00 00 │ ╾──╼....
alloc21+0╼ 03 00 00 00 │ ╾──╼....
}

alloc25 (size: 48, align: 4) {
0x00 │ 00 00 00 00 __ __ __ __ ╾alloc10+0╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾alloc15+0╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc23+0╼ 03 00 00 00 │ ....*...╾──╼....
alloc21 (size: 48, align: 4) {
0x00 │ 00 00 00 00 __ __ __ __ ╾alloc4+0─╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾alloc9+0─╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc19+0╼ 03 00 00 00 │ ....*...╾──╼....
}

alloc10 (size: 0, align: 4) {}
alloc4 (size: 0, align: 4) {}

alloc15 (size: 8, align: 4) {
alloc13+0╼ ╾alloc14+0╼ │ ╾──╼╾──╼
alloc9 (size: 8, align: 4) {
alloc7+0─╼ ╾alloc8+0─╼ │ ╾──╼╾──╼
}

alloc13 (size: 1, align: 1) {
alloc7 (size: 1, align: 1) {
05 │ .
}

alloc14 (size: 1, align: 1) {
alloc8 (size: 1, align: 1) {
06 │ .
}

alloc23 (size: 12, align: 4) {
alloc19+3╼ ╾alloc20+0╼ ╾alloc22+2╼ │ ╾──╼╾──╼╾──╼
alloc19 (size: 12, align: 4) {
alloc15+3╼ ╾alloc16+0╼ ╾alloc18+2╼ │ ╾──╼╾──╼╾──╼
}

alloc19 (size: 4, align: 1) {
alloc15 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}

alloc20 (size: 1, align: 1) {
alloc16 (size: 1, align: 1) {
2a │ *
}

alloc22 (size: 4, align: 1) {
alloc18 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,44 +30,44 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 16, align: 8) {
╾──────alloc25+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
╾──────alloc21+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
}

alloc25 (size: 72, align: 8) {
0x00 │ 00 00 00 00 __ __ __ __ ╾──────alloc10+0──────╼ │ ....░░░░╾──────╼
alloc21 (size: 72, align: 8) {
0x00 │ 00 00 00 00 __ __ __ __ ╾──────alloc4+0───────╼ │ ....░░░░╾──────╼
0x10 │ 00 00 00 00 00 00 00 00 00 00 00 00 __ __ __ __ │ ............░░░░
0x20 │ ╾──────alloc15+0──────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾──────alloc23+0──────╼ │ ....*...╾──────╼
0x20 │ ╾──────alloc9+0───────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾──────alloc19+0──────╼ │ ....*...╾──────╼
0x40 │ 03 00 00 00 00 00 00 00 │ ........
}

alloc10 (size: 0, align: 8) {}
alloc4 (size: 0, align: 8) {}

alloc15 (size: 16, align: 8) {
╾──────alloc13+0──────╼ ╾──────alloc14+0──────╼ │ ╾──────╼╾──────╼
alloc9 (size: 16, align: 8) {
╾──────alloc7+0──────╼ ╾──────alloc8+0───────╼ │ ╾──────╼╾──────╼
}

alloc13 (size: 1, align: 1) {
alloc7 (size: 1, align: 1) {
05 │ .
}

alloc14 (size: 1, align: 1) {
alloc8 (size: 1, align: 1) {
06 │ .
}

alloc23 (size: 24, align: 8) {
0x00 │ ╾──────alloc19+3──────╼ ╾──────alloc20+0──────╼ │ ╾──────╼╾──────╼
0x10 │ ╾──────alloc22+2──────╼ │ ╾──────╼
alloc19 (size: 24, align: 8) {
0x00 │ ╾──────alloc15+3──────╼ ╾──────alloc16+0──────╼ │ ╾──────╼╾──────╼
0x10 │ ╾──────alloc18+2──────╼ │ ╾──────╼
}

alloc19 (size: 4, align: 1) {
alloc15 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}

alloc20 (size: 1, align: 1) {
alloc16 (size: 1, align: 1) {
2a │ *
}

alloc22 (size: 4, align: 1) {
alloc18 (size: 4, align: 1) {
2a 45 15 6f │ *E.o
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Regression test for #66975 - ensure that we don't keep unevaluated
// `!`-typed constants until codegen.
// This was originally a regression test for #66975 - ensure that we do not generate never typed
// consts in codegen. We also have tests for this that catches the error, see
// src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs.

// Force generation of optimized mir for functions that do not reach codegen.
// compile-flags: --emit mir,link
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// MIR for `no_codegen` after PreCodegen

fn no_codegen() -> () {
let mut _0: (); // return place in scope 0 at $DIR/remove-never-const.rs:19:20: 19:20
scope 1 {
}

bb0: {
unreachable; // bb0[0]: scope 0 at $DIR/remove-never-const.rs:20:13: 20:33
}
}

This file was deleted.