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

Do a basic sanity check for all constant values #51361

Merged
merged 5 commits into from
Jul 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ dependencies = [
"cargo_metadata 0.5.8 (registry+https://github.com/rust-lang/crates.io-index)",
"clippy-mini-macro-test 0.2.0",
"clippy_lints 0.0.212",
"compiletest_rs 0.3.11 (registry+https://github.com/rust-lang/crates.io-index)",
"compiletest_rs 0.3.13 (registry+https://github.com/rust-lang/crates.io-index)",
"derive-new 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
"num-traits 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -461,7 +461,7 @@ dependencies = [

[[package]]
name = "compiletest_rs"
version = "0.3.11"
version = "0.3.13"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"diff 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -1319,7 +1319,7 @@ dependencies = [
"byteorder 1.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
"cargo_metadata 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
"colored 1.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
"compiletest_rs 0.3.11 (registry+https://github.com/rust-lang/crates.io-index)",
"compiletest_rs 0.3.13 (registry+https://github.com/rust-lang/crates.io-index)",
"env_logger 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -3118,7 +3118,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum colored 1.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b0aa3473e85a3161b59845d6096b289bb577874cafeaf75ea1b1beaa6572c7fc"
"checksum commoncrypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d056a8586ba25a1e4d61cb090900e495952c7886786fc55f909ab2f819b69007"
"checksum commoncrypto-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1fed34f46747aa73dfaa578069fd8279d2818ade2b55f38f22a9401c7f4083e2"
"checksum compiletest_rs 0.3.11 (registry+https://github.com/rust-lang/crates.io-index)" = "04cea0fe8b8aaca8143af607ad69076866c9f08b83c4b7faca0e993e5486831b"
"checksum compiletest_rs 0.3.13 (registry+https://github.com/rust-lang/crates.io-index)" = "d3064bc712922596dd5ab449fca9261d411893356581fe5297b96aa8f53bb1b8"
"checksum core-foundation 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "286e0b41c3a20da26536c6000a280585d519fd07b3956b43aed8a79e9edce980"
"checksum core-foundation 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cc3532ec724375c7cb7ff0a097b714fde180bb1f6ed2ab27cfcd99ffca873cd2"
"checksum core-foundation-sys 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "716c271e8613ace48344f723b60b900a93150271e5be206212d052bbc0883efa"
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
#![feature(in_band_lifetimes)]
#![feature(macro_at_most_once_rep)]
#![feature(inclusive_range_methods)]
#![feature(crate_in_paths)]

#![recursion_limit="512"]

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<'a, 'gcx, 'tcx> ConstEvalErr<'tcx> {
tcx: TyCtxtAt<'a, 'gcx, 'tcx>,
message: &str
) {
let err = self.struct_generic(tcx, message, None);
let err = self.struct_error(tcx, message);
if let Some(mut err) = err {
err.emit();
}
Expand Down
25 changes: 20 additions & 5 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1599,7 +1599,7 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx>
// Potentially-fat pointers.
ty::TyRef(_, pointee, _) |
ty::TyRawPtr(ty::TypeAndMut { ty: pointee, .. }) => {
assert!(i < 2);
assert!(i < this.fields.count());

// Reuse the fat *T type as its own thin pointer data field.
// This provides information about e.g. DST struct pointees
Expand All @@ -1621,10 +1621,25 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx>
match tcx.struct_tail(pointee).sty {
ty::TySlice(_) |
ty::TyStr => tcx.types.usize,
ty::TyDynamic(..) => {
// FIXME(eddyb) use an usize/fn() array with
// the correct number of vtables slots.
tcx.mk_imm_ref(tcx.types.re_static, tcx.mk_nil())
ty::TyDynamic(data, _) => {
let trait_def_id = data.principal().unwrap().def_id();
let num_fns: u64 = crate::traits::supertrait_def_ids(tcx, trait_def_id)
.map(|trait_def_id| {
tcx.associated_items(trait_def_id)
.filter(|item| item.kind == ty::AssociatedKind::Method)
.count() as u64
})
.sum();
tcx.mk_imm_ref(
tcx.types.re_static,
tcx.mk_array(tcx.types.usize, 3 + num_fns),
)
/* FIXME use actual fn pointers
tcx.mk_tup(&[
tcx.mk_array(tcx.types.usize, 3),
tcx.mk_array(Option<fn()>),
])
*/
}
_ => bug!("TyLayout::field_type({:?}): not applicable", this)
}
Expand Down
75 changes: 68 additions & 7 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use lint::{LateContext, LintContext, LintArray};
use lint::{LintPass, LateLintPass, EarlyLintPass, EarlyContext};

use std::collections::HashSet;
use rustc::util::nodemap::FxHashSet;

use syntax::tokenstream::{TokenTree, TokenStream};
use syntax::ast;
Expand Down Expand Up @@ -1576,20 +1577,77 @@ impl LintPass for UnusedBrokenConst {
}
}

fn validate_const<'a, 'tcx>(
tcx: ty::TyCtxt<'a, 'tcx, 'tcx>,
constant: &ty::Const<'tcx>,
param_env: ty::ParamEnv<'tcx>,
gid: ::rustc::mir::interpret::GlobalId<'tcx>,
what: &str,
) {
let mut ecx = ::rustc_mir::interpret::mk_eval_cx(tcx, gid.instance, param_env).unwrap();
let result = (|| {
let val = ecx.const_to_value(constant.val)?;
use rustc_target::abi::LayoutOf;
let layout = ecx.layout_of(constant.ty)?;
let place = ecx.allocate_place_for_value(val, layout, None)?;
let ptr = place.to_ptr()?;
let mut todo = vec![(ptr, layout.ty, String::new())];
let mut seen = FxHashSet();
seen.insert((ptr, layout.ty));
while let Some((ptr, ty, path)) = todo.pop() {
let layout = ecx.layout_of(ty)?;
ecx.validate_ptr_target(
ptr,
layout.align,
layout,
path,
&mut seen,
&mut todo,
)?;
}
Ok(())
})();
if let Err(err) = result {
let (trace, span) = ecx.generate_stacktrace(None);
let err = ::rustc::mir::interpret::ConstEvalErr {
error: err,
stacktrace: trace,
span,
};
let err = err.struct_error(
tcx.at(span),
&format!("this {} likely exhibits undefined behavior", what),
);
if let Some(mut err) = err {
err.note("The rules on what exactly is undefined behavior aren't clear, \
so this check might be overzealous. Please open an issue on the rust compiler \
repository if you believe it should not be considered undefined behavior",
);
err.emit();
}
}
}

fn check_const(cx: &LateContext, body_id: hir::BodyId, what: &str) {
let def_id = cx.tcx.hir.body_owner_def_id(body_id);
let param_env = cx.tcx.param_env(def_id);
let cid = ::rustc::mir::interpret::GlobalId {
instance: ty::Instance::mono(cx.tcx, def_id),
promoted: None
};
if let Err(err) = cx.tcx.const_eval(param_env.and(cid)) {
let span = cx.tcx.def_span(def_id);
err.report_as_lint(
cx.tcx.at(span),
&format!("this {} cannot be used", what),
cx.current_lint_root(),
);
match cx.tcx.const_eval(param_env.and(cid)) {
Ok(val) => validate_const(cx.tcx, val, param_env, cid, what),
Err(err) => {
// errors for statics are already reported directly in the query
if cx.tcx.is_static(def_id).is_none() {
let span = cx.tcx.def_span(def_id);
err.report_as_lint(
cx.tcx.at(span),
&format!("this {} cannot be used", what),
cx.current_lint_root(),
);
}
},
}
}

Expand All @@ -1610,6 +1668,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedBrokenConst {
hir::ItemKind::Const(_, body_id) => {
check_const(cx, body_id, "constant");
},
hir::ItemKind::Static(_, _, body_id) => {
check_const(cx, body_id, "static");
},
hir::ItemKind::Ty(ref ty, _) => hir::intravisit::walk_ty(
&mut UnusedBrokenConstVisitor(cx),
ty
Expand Down
36 changes: 12 additions & 24 deletions src/librustc_mir/interpret/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc::hir;
use rustc::mir::interpret::{ConstEvalErr};
use rustc::mir;
use rustc::ty::{self, TyCtxt, Ty, Instance};
use rustc::ty::layout::{self, LayoutOf, Primitive};
use rustc::ty::layout::{self, LayoutOf, Primitive, TyLayout};
use rustc::ty::subst::Subst;

use syntax::ast::Mutability;
Expand All @@ -16,7 +16,7 @@ use rustc::mir::interpret::{
EvalResult, EvalError, EvalErrorKind, GlobalId,
Value, Scalar, AllocId, Allocation, ConstValue,
};
use super::{Place, EvalContext, StackPopCleanup, ValTy, PlaceExtra, Memory, MemoryKind};
use super::{Place, EvalContext, StackPopCleanup, ValTy, Memory, MemoryKind};

pub fn mk_borrowck_eval_cx<'a, 'mir, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
Expand Down Expand Up @@ -63,7 +63,7 @@ pub fn eval_promoted<'a, 'mir, 'tcx>(
cid: GlobalId<'tcx>,
mir: &'mir mir::Mir<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> EvalResult<'tcx, (Value, Scalar, Ty<'tcx>)> {
) -> EvalResult<'tcx, (Value, Scalar, TyLayout<'tcx>)> {
ecx.with_fresh_body(|ecx| {
eval_body_using_ecx(ecx, cid, Some(mir), param_env)
})
Expand Down Expand Up @@ -121,7 +121,7 @@ fn eval_body_and_ecx<'a, 'mir, 'tcx>(
cid: GlobalId<'tcx>,
mir: Option<&'mir mir::Mir<'tcx>>,
param_env: ty::ParamEnv<'tcx>,
) -> (EvalResult<'tcx, (Value, Scalar, Ty<'tcx>)>, EvalContext<'a, 'mir, 'tcx, CompileTimeEvaluator>) {
) -> (EvalResult<'tcx, (Value, Scalar, TyLayout<'tcx>)>, EvalContext<'a, 'mir, 'tcx, CompileTimeEvaluator>) {
debug!("eval_body_and_ecx: {:?}, {:?}", cid, param_env);
// we start out with the best span we have
// and try improving it down the road when more information is available
Expand All @@ -137,7 +137,7 @@ fn eval_body_using_ecx<'a, 'mir, 'tcx>(
cid: GlobalId<'tcx>,
mir: Option<&'mir mir::Mir<'tcx>>,
param_env: ty::ParamEnv<'tcx>,
) -> EvalResult<'tcx, (Value, Scalar, Ty<'tcx>)> {
) -> EvalResult<'tcx, (Value, Scalar, TyLayout<'tcx>)> {
debug!("eval_body: {:?}, {:?}", cid, param_env);
let tcx = ecx.tcx.tcx;
let mut mir = match mir {
Expand Down Expand Up @@ -182,7 +182,7 @@ fn eval_body_using_ecx<'a, 'mir, 'tcx>(
// point at the allocation
_ => Value::ByRef(ptr, layout.align),
};
Ok((value, ptr, layout.ty))
Ok((value, ptr, layout))
}

#[derive(Debug, Clone, Eq, PartialEq, Hash)]
Expand Down Expand Up @@ -434,19 +434,7 @@ pub fn const_val_field<'a, 'tcx>(
let ty = value.ty;
let value = ecx.const_to_value(value.val)?;
let layout = ecx.layout_of(ty)?;
let (ptr, align) = match value {
Value::ByRef(ptr, align) => (ptr, align),
Value::ScalarPair(..) | Value::Scalar(_) => {
let ptr = ecx.alloc_ptr(ty)?.into();
ecx.write_value_to_ptr(value, ptr, layout.align, ty)?;
(ptr, layout.align)
},
};
let place = Place::Ptr {
ptr,
align,
extra: variant.map_or(PlaceExtra::None, PlaceExtra::DowncastVariant),
};
let place = ecx.allocate_place_for_value(value, layout, variant)?;
let (place, layout) = ecx.place_field(place, field, layout)?;
let (ptr, align) = place.to_ptr_align();
let mut new_value = Value::ByRef(ptr, align);
Expand Down Expand Up @@ -484,17 +472,17 @@ pub fn const_variant_index<'a, 'tcx>(
trace!("const_variant_index: {:?}, {:?}", instance, val);
let mut ecx = mk_eval_cx(tcx, instance, param_env).unwrap();
let value = ecx.const_to_value(val.val)?;
let layout = ecx.layout_of(val.ty)?;
let (ptr, align) = match value {
Value::ScalarPair(..) | Value::Scalar(_) => {
let layout = ecx.layout_of(val.ty)?;
let ptr = ecx.memory.allocate(layout.size, layout.align, MemoryKind::Stack)?.into();
ecx.write_value_to_ptr(value, ptr, layout.align, val.ty)?;
(ptr, layout.align)
},
Value::ByRef(ptr, align) => (ptr, align),
};
let place = Place::from_scalar_ptr(ptr, align);
ecx.read_discriminant_as_variant_index(place, val.ty)
ecx.read_discriminant_as_variant_index(place, layout)
}

pub fn const_value_to_allocation_provider<'a, 'tcx>(
Expand Down Expand Up @@ -560,11 +548,11 @@ pub fn const_eval_provider<'a, 'tcx>(
};

let (res, ecx) = eval_body_and_ecx(tcx, cid, None, key.param_env);
res.and_then(|(mut val, _, miri_ty)| {
res.and_then(|(mut val, _, layout)| {
if tcx.is_static(def_id).is_none() && cid.promoted.is_none() {
val = ecx.try_read_by_ref(val, miri_ty)?;
val = ecx.try_read_by_ref(val, layout.ty)?;
}
Ok(value_to_const_value(&ecx, val, miri_ty))
Ok(value_to_const_value(&ecx, val, layout.ty))
}).map_err(|err| {
let (trace, span) = ecx.generate_stacktrace(None);
let err = ConstEvalErr {
Expand Down
Loading