提交 49999e9b 编写于 作者: R Ralf Jung

optimize sanity check path printing

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.
上级 42a1239a
......@@ -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,
)?;
......
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 @@
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 @@
)))
}};
($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 @@
}};
}
/// 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 @@ fn validate_scalar(
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 @@ fn validate_scalar(
/// 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 @@ pub fn validate_mplace(
// 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 @@ pub fn validate_mplace(
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 @@ pub fn validate_mplace(
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 @@ pub fn validate_mplace(
// 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 @@ pub fn validate_mplace(
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 @@ pub fn validate_mplace(
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 @@ pub fn validate_mplace(
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),
}
}
}
......@@ -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
......
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`.
......@@ -13,14 +13,36 @@
enum Enum {
A = 0,
}
union Foo {
union TransmuteEnum {
a: &'static u8,
b: Enum,
}
// A pointer is guaranteed non-null
const BAD_ENUM: Enum = unsafe { Foo { a: &1 }.b};
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() {
......
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`.
......@@ -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
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册