OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

optimize sanity check path printing

Browse Source 
During the sanity check, we keep track of the path we are below in a `Vec`.  We
avoid cloning that `Vec` unless we hit a pointer indirection.  The `String`
representation is only computed when validation actually fails.
This commit is contained in:
Ralf Jung 2018-08-18 13:46:52 +02:00
parent 42a1239a18
commit 49999e9b1d
8 changed files with 179 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/librustc_lint/builtin.rs
View File

@ -1622,13 +1622,13 @@ fn validate_const<'a, 'tcx>(
let layout = ecx.layout_of(constant.ty)?;
let place = ecx.allocate_op(OpTy { op, layout })?.into();
let mut todo = vec![(place, String::new())];
let mut todo = vec![(place, Vec::new())];
let mut seen = FxHashSet();
seen.insert(place);
while let Some((place, path)) = todo.pop() {
while let Some((place, mut path)) = todo.pop() {
ecx.validate_mplace(
place,
path,
&mut path,
&mut seen,
&mut todo,
)?;

179
src/librustc_mir/interpret/validity.rs
View File

@ -1,5 +1,6 @@
use std::fmt::Write;
use syntax_pos::symbol::Symbol;
use rustc::ty::layout::{self, Size, Primitive};
use rustc::ty::{self, Ty};
use rustc_data_structures::fx::FxHashSet;
@ -13,10 +14,11 @@ use super::{
macro_rules! validation_failure{
($what:expr, $where:expr, $details:expr) => {{
let where_ = if $where.is_empty() {
let where_ = path_format($where);
let where_ = if where_.is_empty() {
String::new()
} else {
format!(" at {}", $where)
format!(" at {}", where_)
};
err!(ValidationFailure(format!(
"encountered {}{}, but expected {}",
@ -24,10 +26,11 @@ macro_rules! validation_failure{
)))
}};
($what:expr, $where:expr) => {{
let where_ = if $where.is_empty() {
let where_ = path_format($where);
let where_ = if where_.is_empty() {
String::new()
} else {
format!(" at {}", $where)
format!(" at {}", where_)
};
err!(ValidationFailure(format!(
"encountered {}{}",
@ -36,13 +39,59 @@ macro_rules! validation_failure{
}};
}
/// We want to show a nice path to the invalid field for diagnotsics,
/// but avoid string operations in the happy case where no error happens.
/// So we track a `Vec<PathElem>` where `PathElem` contains all the data we
/// need to later print something for the user.
#[derive(Copy, Clone, Debug)]
pub enum PathElem {
Field(Symbol),
ClosureVar(Symbol),
ArrayElem(usize),
TupleElem(usize),
Deref,
Tag,
}
// Adding a Deref and making a copy of the path to be put into the queue
// always go together. This one does it with only new allocation.
fn path_clone_and_deref(path: &Vec<PathElem>) -> Vec<PathElem> {
let mut new_path = Vec::with_capacity(path.len()+1);
new_path.clone_from(path);
new_path.push(PathElem::Deref);
new_path
}
/// Format a path
fn path_format(path: &Vec<PathElem>) -> String {
use self::PathElem::*;
let mut out = String::new();
for elem in path.iter() {
match elem {
Field(name) => write!(out, ".{}", name).unwrap(),
ClosureVar(name) => write!(out, ".<closure-var({})>", name).unwrap(),
TupleElem(idx) => write!(out, ".{}", idx).unwrap(),
ArrayElem(idx) => write!(out, "[{}]", idx).unwrap(),
Deref =>
// This does not match Rust syntax, but it is more readable for long paths -- and
// some of the other items here also are not Rust syntax. Actually we can't
// even use the usual syntax because we are just showing the projections,
// not the root.
write!(out, ".<deref>").unwrap(),
Tag => write!(out, ".<enum-tag>").unwrap(),
}
}
out
}
impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
fn validate_scalar(
&self,
value: ScalarMaybeUndef,
size: Size,
scalar: &layout::Scalar,
path: &str,
path: &Vec<PathElem>,
ty: Ty,
) -> EvalResult<'tcx> {
trace!("validate scalar: {:#?}, {:#?}, {:#?}, {}", value, size, scalar, ty);
@ -93,7 +142,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
ty::TyChar => {
debug_assert_eq!(size.bytes(), 4);
if ::std::char::from_u32(bits as u32).is_none() {
return err!(InvalidChar(bits));
return validation_failure!(
"character",
path,
"a valid unicode codepoint"
);
}
}
_ => {},
@ -127,17 +180,21 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
/// This function checks the memory where `dest` points to. The place must be sized
/// (i.e., dest.extra == PlaceExtra::None).
/// It will error if the bits at the destination do not match the ones described by the layout.
/// The `path` may be pushed to, but the part that is present when the function
/// starts must not be changed!
pub fn validate_mplace(
&self,
dest: MPlaceTy<'tcx>,
path: String,
path: &mut Vec<PathElem>,
seen: &mut FxHashSet<(MPlaceTy<'tcx>)>,
todo: &mut Vec<(MPlaceTy<'tcx>, String)>,
todo: &mut Vec<(MPlaceTy<'tcx>, Vec<PathElem>)>,
) -> EvalResult<'tcx> {
self.memory.dump_alloc(dest.to_ptr()?.alloc_id);
trace!("validate_mplace: {:?}, {:#?}", *dest, dest.layout);
// Find the right variant
// Find the right variant. We have to handle this as a prelude, not via
// proper recursion with the new inner layout, to be able to later nicely
// print the field names of the enum field that is being accessed.
let (variant, dest) = match dest.layout.variants {
layout::Variants::NicheFilling { niche: ref tag, .. } |
layout::Variants::Tagged { ref tag, .. } => {
@ -145,28 +202,35 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
// we first read the tag value as scalar, to be able to validate it
let tag_mplace = self.mplace_field(dest, 0)?;
let tag_value = self.read_scalar(tag_mplace.into())?;
let path = format!("{}.TAG", path);
path.push(PathElem::Tag);
self.validate_scalar(
tag_value, size, tag, &path, tag_mplace.layout.ty
)?;
path.pop(); // remove the element again
// then we read it again to get the index, to continue
let variant = self.read_discriminant_as_variant_index(dest.into())?;
let dest = self.mplace_downcast(dest, variant)?;
let inner_dest = self.mplace_downcast(dest, variant)?;
// Put the variant projection onto the path, as a field
path.push(PathElem::Field(dest.layout.ty.ty_adt_def().unwrap().variants[variant].name));
trace!("variant layout: {:#?}", dest.layout);
(variant, dest)
(variant, inner_dest)
},
layout::Variants::Single { index } => {
(index, dest)
}
};
// Remember the length, in case we need to truncate
let path_len = path.len();
// Validate all fields
match dest.layout.fields {
// primitives are unions with zero fields
layout::FieldPlacement::Union(0) => {
match dest.layout.abi {
// nothing to do, whatever the pointer points to, it is never going to be read
layout::Abi::Uninhabited => validation_failure!("a value of an uninhabited type", path),
layout::Abi::Uninhabited =>
return validation_failure!("a value of an uninhabited type", path),
// check that the scalar is a valid pointer or that its bit range matches the
// expectation.
layout::Abi::Scalar(ref scalar_layout) => {
@ -179,8 +243,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
if let Scalar::Ptr(ptr) = scalar.not_undef()? {
let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
if let Some(AllocType::Static(did)) = alloc_kind {
// statics from other crates are already checked
// extern statics should not be validated as they have no body
// statics from other crates are already checked.
// extern statics should not be validated as they have no body.
if !did.is_local() || self.tcx.is_foreign_item(did) {
return Ok(());
}
@ -190,12 +254,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
let ptr_place = self.ref_to_mplace(value)?;
// we have not encountered this pointer+layout combination before
if seen.insert(ptr_place) {
todo.push((ptr_place, format!("(*{})", path)))
todo.push((ptr_place, path_clone_and_deref(path)));
}
}
}
}
Ok(())
},
_ => bug!("bad abi for FieldPlacement::Union(0): {:#?}", dest.layout.abi),
}
@ -204,16 +267,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
// We can't check unions, their bits are allowed to be anything.
// The fields don't need to correspond to any bit pattern of the union's fields.
// See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389
Ok(())
},
layout::FieldPlacement::Array { .. } => {
for (i, field) in self.mplace_array_fields(dest)?.enumerate() {
let field = field?;
let mut path = path.clone();
self.dump_field_name(&mut path, dest.layout.ty, i as usize, variant).unwrap();
path.push(PathElem::ArrayElem(i));
self.validate_mplace(field, path, seen, todo)?;
path.truncate(path_len);
}
Ok(())
},
layout::FieldPlacement::Arbitrary { ref offsets, .. } => {
// fat pointers need special treatment
@ -232,9 +293,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
assert_eq!(ptr.extra, PlaceExtra::Length(len));
let unpacked_ptr = self.unpack_unsized_mplace(ptr)?;
if seen.insert(unpacked_ptr) {
let mut path = path.clone();
self.dump_field_name(&mut path, dest.layout.ty, 0, 0).unwrap();
todo.push((unpacked_ptr, path))
todo.push((unpacked_ptr, path_clone_and_deref(path)));
}
},
Some(ty::TyDynamic(..)) => {
@ -249,9 +308,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
assert_eq!(ptr.extra, PlaceExtra::Vtable(vtable));
let unpacked_ptr = self.unpack_unsized_mplace(ptr)?;
if seen.insert(unpacked_ptr) {
let mut path = path.clone();
self.dump_field_name(&mut path, dest.layout.ty, 0, 0).unwrap();
todo.push((unpacked_ptr, path))
todo.push((unpacked_ptr, path_clone_and_deref(path)));
}
// FIXME: More checks for the vtable... making sure it is exactly
// the one one would expect for this type.
@ -261,84 +318,42 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
None => {
// Not a pointer, perform regular aggregate handling below
for i in 0..offsets.len() {
let mut path = path.clone();
self.dump_field_name(&mut path, dest.layout.ty, i, variant).unwrap();
let field = self.mplace_field(dest, i as u64)?;
path.push(self.aggregate_field_path_elem(dest.layout.ty, variant, i));
self.validate_mplace(field, path, seen, todo)?;
path.truncate(path_len);
}
// FIXME: For a TyStr, check that this is valid UTF-8.
},
}
Ok(())
}
}
Ok(())
}
fn dump_field_name(&self, s: &mut String, ty: Ty<'tcx>, i: usize, variant: usize) -> ::std::fmt::Result {
fn aggregate_field_path_elem(&self, ty: Ty<'tcx>, variant: usize, field: usize) -> PathElem {
match ty.sty {
ty::TyBool |
ty::TyChar |
ty::TyInt(_) |
ty::TyUint(_) |
ty::TyFloat(_) |
ty::TyFnPtr(_) |
ty::TyNever |
ty::TyFnDef(..) |
ty::TyGeneratorWitness(..) |
ty::TyForeign(..) |
ty::TyDynamic(..) => {
bug!("field_name({:?}): not applicable", ty)
}
// Potentially-fat pointers.
ty::TyRef(_, pointee, _) |
ty::TyRawPtr(ty::TypeAndMut { ty: pointee, .. }) => {
assert!(i < 2);
// Reuse the fat *T type as its own thin pointer data field.
// This provides information about e.g. DST struct pointees
// (which may have no non-DST form), and will work as long
// as the `Abi` or `FieldPlacement` is checked by users.
if i == 0 {
return write!(s, ".data_ptr");
}
match self.tcx.struct_tail(pointee).sty {
ty::TySlice(_) |
ty::TyStr => write!(s, ".len"),
ty::TyDynamic(..) => write!(s, ".vtable_ptr"),
_ => bug!("field_name({:?}): not applicable", ty)
}
}
// Arrays and slices.
ty::TyArray(_, _) |
ty::TySlice(_) |
ty::TyStr => write!(s, "[{}]", i),
// generators and closures.
ty::TyClosure(def_id, _) | ty::TyGenerator(def_id, _, _) => {
let node_id = self.tcx.hir.as_local_node_id(def_id).unwrap();
let freevar = self.tcx.with_freevars(node_id, |fv| fv[i]);
write!(s, ".upvar({})", self.tcx.hir.name(freevar.var_id()))
let freevar = self.tcx.with_freevars(node_id, |fv| fv[field]);
PathElem::ClosureVar(self.tcx.hir.name(freevar.var_id()))
}
ty::TyTuple(_) => write!(s, ".{}", i),
// tuples
ty::TyTuple(_) => PathElem::TupleElem(field),
// enums
ty::TyAdt(def, ..) if def.is_enum() => {
let variant = &def.variants[variant];
write!(s, ".{}::{}", variant.name, variant.fields[i].ident)
PathElem::Field(variant.fields[field].ident.name)
}
// other ADTs.
ty::TyAdt(def, _) => write!(s, ".{}", def.non_enum_variant().fields[i].ident),
// other ADTs
ty::TyAdt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name),
ty::TyProjection(_) | ty::TyAnon(..) | ty::TyParam(_) |
ty::TyInfer(_) | ty::TyError => {
bug!("dump_field_name: unexpected type `{}`", ty)
}
// nothing else has an aggregate layout
_ => bug!("aggregate_field_path_elem: got non-aggregate type {:?}", ty),
}
}
}

2
src/test/ui/consts/const-eval/double_check2.stderr
View File

@ -5,7 +5,7 @@ LL | / static FOO: (&Foo, &Bar) = unsafe {( //~ undefined behavior
LL | | Union { usize: &BAR }.foo,
LL | | Union { usize: &BAR }.bar,
LL | | )};
| |___^ type validation failed: encountered 5 at (*.1).TAG, but expected something in the range 42..=99
| |___^ type validation failed: encountered 5 at .1.<deref>.<enum-tag>, but expected something in the range 42..=99
|
= 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

27
src/test/ui/consts/const-eval/ub-enum-ptr.rs
View File

@ -1,27 +0,0 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#[repr(usize)]
#[derive(Copy, Clone)]
enum Enum {
A = 0,
}
union Foo {
a: &'static u8,
b: Enum,
}
// A pointer is guaranteed non-null
const BAD_ENUM: Enum = unsafe { Foo { a: &1 }.b};
//~^ ERROR this constant likely exhibits undefined behavior
fn main() {
}

11
src/test/ui/consts/const-eval/ub-enum-ptr.stderr
View File

@ -1,11 +0,0 @@
error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/ub-enum-ptr.rs:23:1
|
LL | const BAD_ENUM: Enum = unsafe { Foo { a: &1 }.b};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer at .TAG, but expected something in the range 0..=0
|
= 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
error: aborting due to previous error
For more information about this error, try `rustc --explain E0080`.

49
src/test/ui/consts/const-eval/ub-enum.rs Normal file
View File

@ -0,0 +1,49 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#[repr(usize)]
#[derive(Copy, Clone)]
enum Enum {
A = 0,
}
union TransmuteEnum {
a: &'static u8,
b: Enum,
}
// A pointer is guaranteed non-null
const BAD_ENUM: Enum = unsafe { TransmuteEnum { a: &1 }.b };
//~^ ERROR this constant likely exhibits undefined behavior
// Invalid enum discriminant
#[repr(usize)]
#[derive(Copy, Clone)]
enum Enum2 {
A = 2,
}
union TransmuteEnum2 {
a: usize,
b: Enum2,
}
const BAD_ENUM2 : Enum2 = unsafe { TransmuteEnum2 { a: 0 }.b };
//~^ ERROR this constant likely exhibits undefined behavior
// Invalid enum field content (mostly to test printing of apths for enum tuple
// variants and tuples).
union TransmuteChar {
a: u32,
b: char,
}
// Need to create something which does not clash with enum layout optimizations.
const BAD_ENUM_CHAR : Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b }));
//~^ ERROR this constant likely exhibits undefined behavior
fn main() {
}

27
src/test/ui/consts/const-eval/ub-enum.stderr Normal file
View File

@ -0,0 +1,27 @@
error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/ub-enum.rs:22:1
|
LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { a: &1 }.b };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer at .<enum-tag>, but expected something in the range 0..=0
|
= 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
error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/ub-enum.rs:35:1
|
LL | const BAD_ENUM2 : Enum2 = unsafe { TransmuteEnum2 { a: 0 }.b };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0 at .<enum-tag>, but expected something in the range 2..=2
|
= 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
error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/ub-enum.rs:45:1
|
LL | const BAD_ENUM_CHAR : Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b }));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered character at .Some.0.1, but expected a valid unicode codepoint
|
= 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
error: aborting due to 3 previous errors
For more information about this error, try `rustc --explain E0080`.

4
src/test/ui/union-ub-fat-ptr.stderr
View File

@ -58,7 +58,7 @@ error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/union-ub-fat-ptr.rs:102:1
|
LL | const G: &Trait = &unsafe { BoolTransmute { val: 3 }.bl };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .data_ptr, but expected something in the range 0..=1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>, but expected something in the range 0..=1
|
= 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
@ -66,7 +66,7 @@ error[E0080]: this constant likely exhibits undefined behavior
--> $DIR/union-ub-fat-ptr.rs:106:1
|
LL | const H: &[bool] = &[unsafe { BoolTransmute { val: 3 }.bl }];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .data_ptr[0], but expected something in the range 0..=1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>[0], but expected something in the range 0..=1
|
= 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