From ee49e95e787b353c0697862ca2d703c7b9480d25 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Feb 2020 22:58:24 +0100 Subject: [PATCH 1/5] doc comments for layout components --- src/librustc_target/abi/mod.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 3f44339bf4c..447487e4fb2 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -871,8 +871,26 @@ impl Niche { #[derive(PartialEq, Eq, Hash, Debug, HashStable_Generic)] pub struct LayoutDetails { - pub variants: Variants, + /// Says where the fields are located. + /// Primitives and fieldless enums appear as unions without fields. pub fields: FieldPlacement, + + /// Encodes information about multi-variant layouts. + /// Even with `Multiple` variants, a layout can still have fields! Those are then + /// shared between all variants. One of them will be the discriminant, + /// but e.g. generators can have more. + /// + /// A layout-guided recursive descent must first look at all the fields, + /// and only then check if this is a multi-variant layout and if so, proceed + /// with the active variant. + pub variants: Variants, + + /// The `abi` defines how this data is passed between functions, and it defines + /// value restrictions via `valid_range`. + /// + /// Note that this is entirely orthogonal to the recursive structrue defined by + /// `variants` and `fields`; for example, `ManuallyDrop>` has + /// `Abi::ScalarPair`! So, having a non-`Aggregate` `abi` should not stop a recursive descent. pub abi: Abi, /// The leaf scalar with the largest number of invalid values From f8999fbdcf7bc924938d94eed1ecce337e315e93 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Feb 2020 22:59:16 +0100 Subject: [PATCH 2/5] miri value visitor: fix some wrong assumptions about layout; improve error messages --- src/librustc_mir/interpret/intern.rs | 7 +- src/librustc_mir/interpret/validity.rs | 181 +++++++++++------- src/librustc_mir/interpret/visitor.rs | 120 +++--------- .../consts/const-eval/transmute-const.stderr | 2 +- src/test/ui/consts/const-eval/ub-enum.stderr | 14 +- .../ui/consts/const-eval/ub-nonnull.stderr | 2 +- src/test/ui/consts/const-eval/ub-ref.stderr | 2 +- .../ui/consts/const-eval/ub-uninhabit.stderr | 6 +- .../ui/consts/const-eval/ub-upvars.stderr | 2 +- .../ui/consts/const-eval/ub-wide-ptr.stderr | 8 +- src/test/ui/consts/const-eval/union-ub.stderr | 2 +- .../validate_uninhabited_zsts.stderr | 2 +- .../ui/consts/validate_never_arrays.stderr | 6 +- 13 files changed, 174 insertions(+), 180 deletions(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 9fea82d78c9..13d5f5f6a37 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -187,7 +187,7 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx self.walk_aggregate(mplace, fields) } - fn visit_primitive(&mut self, mplace: MPlaceTy<'tcx>) -> InterpResult<'tcx> { + fn visit_value(&mut self, mplace: MPlaceTy<'tcx>) -> InterpResult<'tcx> { // Handle Reference types, as these are the only relocations supported by const eval. // Raw pointers (and boxes) are handled by the `leftover_relocations` logic. let ty = mplace.layout.ty; @@ -263,8 +263,11 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx None => self.ref_tracking.track((mplace, mutability, mode), || ()), } } + Ok(()) + } else { + // Not a reference -- proceed recursively. + self.walk_value(mplace) } - Ok(()) } } diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index aa2b3040a71..e90a5985677 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -201,13 +201,16 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M // enums ty::Adt(def, ..) if def.is_enum() => { - // we might be projecting *to* a variant, or to a field *in*a variant. + // we might be projecting *to* a variant, or to a field *in* a variant. match layout.variants { layout::Variants::Single { index } => { // Inside a variant PathElem::Field(def.variants[index].fields[field].ident.name) } - _ => bug!(), + layout::Variants::Multiple { discr_index, .. } => { + assert_eq!(discr_index, field); + PathElem::Tag + } } } @@ -288,62 +291,6 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M Ok(()) } -} - -impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> - for ValidityVisitor<'rt, 'mir, 'tcx, M> -{ - type V = OpTy<'tcx, M::PointerTag>; - - #[inline(always)] - fn ecx(&self) -> &InterpCx<'mir, 'tcx, M> { - &self.ecx - } - - #[inline] - fn visit_field( - &mut self, - old_op: OpTy<'tcx, M::PointerTag>, - field: usize, - new_op: OpTy<'tcx, M::PointerTag>, - ) -> InterpResult<'tcx> { - let elem = self.aggregate_field_path_elem(old_op.layout, field); - self.visit_elem(new_op, elem) - } - - #[inline] - fn visit_variant( - &mut self, - old_op: OpTy<'tcx, M::PointerTag>, - variant_id: VariantIdx, - new_op: OpTy<'tcx, M::PointerTag>, - ) -> InterpResult<'tcx> { - let name = match old_op.layout.ty.kind { - ty::Adt(adt, _) => PathElem::Variant(adt.variants[variant_id].ident.name), - // Generators also have variants - ty::Generator(..) => PathElem::GeneratorState(variant_id), - _ => bug!("Unexpected type with variant: {:?}", old_op.layout.ty), - }; - self.visit_elem(new_op, name) - } - - #[inline] - fn visit_value(&mut self, op: OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> { - trace!("visit_value: {:?}, {:?}", *op, op.layout); - // Translate some possible errors to something nicer. - match self.walk_value(op) { - Ok(()) => Ok(()), - Err(err) => match err.kind { - err_ub!(InvalidDiscriminant(val)) => { - throw_validation_failure!(val, self.path, "a valid enum discriminant") - } - err_unsup!(ReadPointerAsBytes) => { - throw_validation_failure!("a pointer", self.path, "plain (non-pointer) bytes") - } - _ => Err(err), - }, - } - } fn visit_primitive(&mut self, value: OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> { let value = self.ecx.read_immediate(value)?; @@ -421,7 +368,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> throw_validation_failure!( format_args!( "unaligned reference \ - (required {} byte alignment but found {})", + (required {} byte alignment but found {})", required.bytes(), has.bytes() ), @@ -485,16 +432,12 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> ); // FIXME: Check if the signature matches } - // This should be all the primitive types + // This should be all the (inhabited) primitive types _ => bug!("Unexpected primitive type {}", value.layout.ty), } Ok(()) } - fn visit_uninhabited(&mut self) -> InterpResult<'tcx> { - throw_validation_failure!("a value of an uninhabited type", self.path) - } - fn visit_scalar( &mut self, op: OpTy<'tcx, M::PointerTag>, @@ -559,6 +502,116 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> ) } } +} + +impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> + for ValidityVisitor<'rt, 'mir, 'tcx, M> +{ + type V = OpTy<'tcx, M::PointerTag>; + + #[inline(always)] + fn ecx(&self) -> &InterpCx<'mir, 'tcx, M> { + &self.ecx + } + + #[inline] + fn visit_field( + &mut self, + old_op: OpTy<'tcx, M::PointerTag>, + field: usize, + new_op: OpTy<'tcx, M::PointerTag>, + ) -> InterpResult<'tcx> { + let elem = self.aggregate_field_path_elem(old_op.layout, field); + self.visit_elem(new_op, elem) + } + + #[inline] + fn visit_variant( + &mut self, + old_op: OpTy<'tcx, M::PointerTag>, + variant_id: VariantIdx, + new_op: OpTy<'tcx, M::PointerTag>, + ) -> InterpResult<'tcx> { + let name = match old_op.layout.ty.kind { + ty::Adt(adt, _) => PathElem::Variant(adt.variants[variant_id].ident.name), + // Generators also have variants + ty::Generator(..) => PathElem::GeneratorState(variant_id), + _ => bug!("Unexpected type with variant: {:?}", old_op.layout.ty), + }; + self.visit_elem(new_op, name) + } + + #[inline(always)] + fn visit_union(&mut self, _v: Self::V, fields: usize) -> InterpResult<'tcx> { + // Empty unions are not accepted by rustc. That's great, it means we can + // use that as a signal for detecting primitives. Make sure + // we did not miss any primitive. + assert!(fields > 0); + Ok(()) + } + + #[inline] + fn visit_value(&mut self, op: OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> { + trace!("visit_value: {:?}, {:?}", *op, op.layout); + + if op.layout.abi.is_uninhabited() { + // Uninhabited types do not have sensible layout, stop right here. + throw_validation_failure!( + format_args!("a value of uninhabited type {:?}", op.layout.ty), + self.path + ) + } + + // Check primitive types. We do this after checking for uninhabited types, + // to exclude fieldless enums (that also appear as fieldless unions here). + // Primitives can have varying layout, so we check them separately and before aggregate + // handling. + // It is CRITICAL that we get this check right, or we might be validating the wrong thing! + let primitive = match op.layout.fields { + // Primitives appear as Union with 0 fields - except for Boxes and fat pointers. + // (Fieldless enums also appear here, but they are uninhabited and thus handled above.) + layout::FieldPlacement::Union(0) => true, + _ => op.layout.ty.builtin_deref(true).is_some(), + }; + if primitive { + // No need to recurse further or check scalar layout, this is a leaf type. + return self.visit_primitive(op); + } + + // Recursively walk the type. Translate some possible errors to something nicer. + match self.walk_value(op) { + Ok(()) => {} + Err(err) => match err.kind { + err_ub!(InvalidDiscriminant(val)) => { + throw_validation_failure!(val, self.path, "a valid enum discriminant") + } + err_unsup!(ReadPointerAsBytes) => { + throw_validation_failure!("a pointer", self.path, "plain (non-pointer) bytes") + } + _ => return Err(err), + }, + } + + // *After* all of this, check the ABI. We need to check the ABI to handle + // types like `NonNull` where the `Scalar` info is more restrictive than what + // the fields say. But in most cases, this will just propagate what the fields say, + // and then we want the error to point at the field -- so, first recurse, + // then check ABI. + // + // FIXME: We could avoid some redundant checks here. For newtypes wrapping + // scalars, we do the same check on every "level" (e.g., first we check + // MyNewtype and then the scalar in there). + match op.layout.abi { + layout::Abi::Uninhabited => unreachable!(), // checked above + layout::Abi::Scalar(ref layout) => { + self.visit_scalar(op, layout)?; + } + // FIXME: Should we do something for ScalarPair? Vector? + _ => {} + } + + Ok(()) + } fn visit_aggregate( &mut self, diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index d2594e87071..8808fc70cf7 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -120,7 +120,7 @@ macro_rules! make_value_visitor { } /// Visits the given value as a union. No automatic recursion can happen here. #[inline(always)] - fn visit_union(&mut self, _v: Self::V) -> InterpResult<'tcx> + fn visit_union(&mut self, _v: Self::V, _fields: usize) -> InterpResult<'tcx> { Ok(()) } @@ -150,8 +150,9 @@ macro_rules! make_value_visitor { ) -> InterpResult<'tcx> { self.visit_value(new_val) } - /// Called when recursing into an enum variant. + /// This gives the visitor the chance to track the stack of nested fields that + /// we are descending through. #[inline(always)] fn visit_variant( &mut self, @@ -162,33 +163,6 @@ macro_rules! make_value_visitor { self.visit_value(new_val) } - /// Called whenever we reach a value with uninhabited layout. - /// Recursing to fields will *always* continue after this! This is not meant to control - /// whether and how we descend recursively/ into the scalar's fields if there are any, - /// it is meant to provide the chance for additional checks when a value of uninhabited - /// layout is detected. - #[inline(always)] - fn visit_uninhabited(&mut self) -> InterpResult<'tcx> - { Ok(()) } - /// Called whenever we reach a value with scalar layout. - /// We do NOT provide a `ScalarMaybeUndef` here to avoid accessing memory if the - /// visitor is not even interested in scalars. - /// Recursing to fields will *always* continue after this! This is not meant to control - /// whether and how we descend recursively/ into the scalar's fields if there are any, - /// it is meant to provide the chance for additional checks when a value of scalar - /// layout is detected. - #[inline(always)] - fn visit_scalar(&mut self, _v: Self::V, _layout: &layout::Scalar) -> InterpResult<'tcx> - { Ok(()) } - - /// Called whenever we reach a value of primitive type. There can be no recursion - /// below such a value. This is the leaf function. - /// We do *not* provide an `ImmTy` here because some implementations might want - /// to write to the place this primitive lives in. - #[inline(always)] - fn visit_primitive(&mut self, _v: Self::V) -> InterpResult<'tcx> - { Ok(()) } - // Default recursors. Not meant to be overloaded. fn walk_aggregate( &mut self, @@ -204,23 +178,10 @@ macro_rules! make_value_visitor { fn walk_value(&mut self, v: Self::V) -> InterpResult<'tcx> { trace!("walk_value: type: {}", v.layout().ty); - // If this is a multi-variant layout, we have to find the right one and proceed with - // that. - match v.layout().variants { - layout::Variants::Multiple { .. } => { - let op = v.to_op(self.ecx())?; - let idx = self.ecx().read_discriminant(op)?.1; - let inner = v.project_downcast(self.ecx(), idx)?; - trace!("walk_value: variant layout: {:#?}", inner.layout()); - // recurse with the inner type - return self.visit_variant(v, idx, inner); - } - layout::Variants::Single { .. } => {} - } - // Even for single variants, we might be able to get a more refined type: - // If it is a trait object, switch to the actual type that was used to create it. + // Special treatment for special types, where the (static) layout is not sufficient. match v.layout().ty.kind { + // If it is a trait object, switch to the real type that was used to create it. ty::Dynamic(..) => { // immediate trait objects are not a thing let dest = v.to_op(self.ecx())?.assert_mem_place(self.ecx()); @@ -229,56 +190,16 @@ macro_rules! make_value_visitor { // recurse with the inner type return self.visit_field(v, 0, Value::from_mem_place(inner)); }, - ty::Generator(..) => { - // FIXME: Generator layout is lying: it claims a whole bunch of fields exist - // when really many of them can be uninitialized. - // Just treat them as a union for now, until hopefully the layout - // computation is fixed. - return self.visit_union(v); - } + // Slices do not need special handling here: they have `Array` field + // placement with length 0, so we enter the `Array` case below which + // indirectly uses the metadata to determine the actual length. _ => {}, }; - // If this is a scalar, visit it as such. - // Things can be aggregates and have scalar layout at the same time, and that - // is very relevant for `NonNull` and similar structs: We need to visit them - // at their scalar layout *before* descending into their fields. - // FIXME: We could avoid some redundant checks here. For newtypes wrapping - // scalars, we do the same check on every "level" (e.g., first we check - // MyNewtype and then the scalar in there). - match v.layout().abi { - layout::Abi::Uninhabited => { - self.visit_uninhabited()?; - } - layout::Abi::Scalar(ref layout) => { - self.visit_scalar(v, layout)?; - } - // FIXME: Should we do something for ScalarPair? Vector? - _ => {} - } - - // Check primitive types. We do this after checking the scalar layout, - // just to have that done as well. Primitives can have varying layout, - // so we check them separately and before aggregate handling. - // It is CRITICAL that we get this check right, or we might be - // validating the wrong thing! - let primitive = match v.layout().fields { - // Primitives appear as Union with 0 fields - except for Boxes and fat pointers. - layout::FieldPlacement::Union(0) => true, - _ => v.layout().ty.builtin_deref(true).is_some(), - }; - if primitive { - return self.visit_primitive(v); - } - - // Proceed into the fields. + // Visit the fields of this value. match v.layout().fields { layout::FieldPlacement::Union(fields) => { - // Empty unions are not accepted by rustc. That's great, it means we can - // use that as an unambiguous signal for detecting primitives. Make sure - // we did not miss any primitive. - assert!(fields > 0); - self.visit_union(v) + self.visit_union(v, fields)?; }, layout::FieldPlacement::Arbitrary { ref offsets, .. } => { // FIXME: We collect in a vec because otherwise there are lifetime @@ -288,19 +209,36 @@ macro_rules! make_value_visitor { v.project_field(self.ecx(), i as u64) }) .collect(); - self.visit_aggregate(v, fields.into_iter()) + self.visit_aggregate(v, fields.into_iter())?; }, layout::FieldPlacement::Array { .. } => { // Let's get an mplace first. let mplace = v.to_op(self.ecx())?.assert_mem_place(self.ecx()); // Now we can go over all the fields. + // This uses the *run-time length*, i.e., if we are a slice, + // the dynamic info from the metadata is used. let iter = self.ecx().mplace_array_fields(mplace)? .map(|f| f.and_then(|f| { Ok(Value::from_mem_place(f)) })); - self.visit_aggregate(v, iter) + self.visit_aggregate(v, iter)?; } } + + match v.layout().variants { + // If this is a multi-variant layout, find the right variant and proceed + // with *its* fields. + layout::Variants::Multiple { .. } => { + let op = v.to_op(self.ecx())?; + let idx = self.ecx().read_discriminant(op)?.1; + let inner = v.project_downcast(self.ecx(), idx)?; + trace!("walk_value: variant layout: {:#?}", inner.layout()); + // recurse with the inner type + self.visit_variant(v, idx, inner) + } + // For single-variant layouts, we already did anything there is to do. + layout::Variants::Single { .. } => Ok(()) + } } } } diff --git a/src/test/ui/consts/const-eval/transmute-const.stderr b/src/test/ui/consts/const-eval/transmute-const.stderr index 47f89fccf7a..e93a6887ba8 100644 --- a/src/test/ui/consts/const-eval/transmute-const.stderr +++ b/src/test/ui/consts/const-eval/transmute-const.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/transmute-const.rs:5:1 | LL | static FOO: bool = unsafe { mem::transmute(3u8) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3, but expected something less or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3, but expected a boolean | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 8ebc9dbec8a..3680f4f2ac2 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -10,7 +10,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:26:1 | LL | const BAD_ENUM_PTR: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at ., but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -18,7 +18,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:29:1 | LL | const BAD_ENUM_WRAPPED: Wrap = unsafe { TransmuteEnum { in1: &1 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be equal to 0 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at .0., but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -34,7 +34,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:50:1 | LL | const BAD_ENUM2_PTR: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at ., but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -42,7 +42,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:52:1 | LL | const BAD_ENUM2_WRAPPED: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be equal to 2 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at .0., but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -50,7 +50,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:56:1 | LL | const BAD_ENUM2_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid enum discriminant + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes at ., but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -58,7 +58,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:60:1 | LL | const BAD_ENUM2_OPTION_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at ., but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -66,7 +66,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:71:1 | LL | const BAD_OPTION_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..0.1, but expected something less or equal to 1114111 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..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 rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr index 4d9d258f808..ec056187057 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.stderr @@ -44,7 +44,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-nonnull.rs:32:1 | LL | const UNINIT: NonZeroU8 = unsafe { Transmute { uninit: () }.out }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected something greater or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes at .0, but expected initialized plain (non-pointer) bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-ref.stderr b/src/test/ui/consts/const-eval/ub-ref.stderr index 01bde413c0d..86d5d17c538 100644 --- a/src/test/ui/consts/const-eval/ub-ref.stderr +++ b/src/test/ui/consts/const-eval/ub-ref.stderr @@ -10,7 +10,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-ref.rs:11:1 | LL | const NULL: &u16 = unsafe { mem::transmute(0usize) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered NULL reference | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-uninhabit.stderr b/src/test/ui/consts/const-eval/ub-uninhabit.stderr index 3877f3cab6d..4fef9aa84ea 100644 --- a/src/test/ui/consts/const-eval/ub-uninhabit.stderr +++ b/src/test/ui/consts/const-eval/ub-uninhabit.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-uninhabit.rs:15:1 | LL | const BAD_BAD_BAD: Bar = unsafe { (TransmuteUnion::<(), Bar> { a: () }).b }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type Bar | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -10,7 +10,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-uninhabit.rs:18:1 | LL | const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at . + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type Bar at . | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -18,7 +18,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-uninhabit.rs:21:1 | LL | const BAD_BAD_ARRAY: [Bar; 1] = unsafe { (TransmuteUnion::<(), [Bar; 1]> { a: () }).b }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type [Bar; 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 rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-upvars.stderr b/src/test/ui/consts/const-eval/ub-upvars.stderr index ea6ab3ae5b5..c1df447cd85 100644 --- a/src/test/ui/consts/const-eval/ub-upvars.stderr +++ b/src/test/ui/consts/const-eval/ub-upvars.stderr @@ -6,7 +6,7 @@ LL | | let bad_ref: &'static u16 = unsafe { mem::transmute(0usize) }; LL | | let another_var = 13; LL | | move || { let _ = bad_ref; let _ = another_var; } LL | | }; - | |__^ type validation failed: encountered 0 at ..., but expected something greater or equal to 1 + | |__^ type validation failed: encountered NULL reference at ... | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-wide-ptr.stderr b/src/test/ui/consts/const-eval/ub-wide-ptr.stderr index ce57d680dc9..170037b44da 100644 --- a/src/test/ui/consts/const-eval/ub-wide-ptr.stderr +++ b/src/test/ui/consts/const-eval/ub-wide-ptr.stderr @@ -66,7 +66,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:116:1 | LL | const SLICE_CONTENT_INVALID: &[bool] = &[unsafe { BoolTransmute { val: 3 }.bl }]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .[0], but expected something less or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .[0], but expected a boolean | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -74,7 +74,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:122:1 | LL | const MYSLICE_PREFIX_BAD: &MySliceBool = &MySlice(unsafe { BoolTransmute { val: 3 }.bl }, [false]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..0, but expected something less or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..0, but expected a boolean | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -82,7 +82,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:125:1 | LL | const MYSLICE_SUFFIX_BAD: &MySliceBool = &MySlice(true, [unsafe { BoolTransmute { val: 3 }.bl }]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..1[0], but expected something less or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..1[0], but expected a boolean | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -122,7 +122,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:147:1 | LL | const TRAIT_OBJ_CONTENT_INVALID: &dyn Trait = &unsafe { BoolTransmute { val: 3 }.bl }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .., but expected something less or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .., but expected a boolean | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/union-ub.stderr b/src/test/ui/consts/const-eval/union-ub.stderr index fa67bc0d8e7..9d90d6e8548 100644 --- a/src/test/ui/consts/const-eval/union-ub.stderr +++ b/src/test/ui/consts/const-eval/union-ub.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/union-ub.rs:31:1 | LL | const BAD_BOOL: bool = unsafe { DummyUnion { u8: 42 }.bool}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something less or equal to 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected a boolean | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/validate_uninhabited_zsts.stderr b/src/test/ui/consts/const-eval/validate_uninhabited_zsts.stderr index c98e206e88c..2a338e27640 100644 --- a/src/test/ui/consts/const-eval/validate_uninhabited_zsts.stderr +++ b/src/test/ui/consts/const-eval/validate_uninhabited_zsts.stderr @@ -20,7 +20,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/validate_uninhabited_zsts.rs:17:1 | LL | const BAR: [Empty; 3] = [unsafe { std::mem::transmute(()) }; 3]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type [Empty; 3] | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/validate_never_arrays.stderr b/src/test/ui/consts/validate_never_arrays.stderr index cb995b8216f..203620a771b 100644 --- a/src/test/ui/consts/validate_never_arrays.stderr +++ b/src/test/ui/consts/validate_never_arrays.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/validate_never_arrays.rs:3:1 | LL | const _: &[!; 1] = unsafe { &*(1_usize as *const [!; 1]) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at . + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type [!; 1] at . | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -10,7 +10,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/validate_never_arrays.rs:6:1 | LL | const _: &[!] = unsafe { &*(1_usize as *const [!; 1]) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at .[0] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type ! at .[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 rustc repository if you believe it should not be considered undefined behavior. @@ -18,7 +18,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/validate_never_arrays.rs:7:1 | LL | const _: &[!] = unsafe { &*(1_usize as *const [!; 42]) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at .[0] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type ! at .[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 rustc repository if you believe it should not be considered undefined behavior. From bd979ed48cb94c271a953a32723aca73f4edb551 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 18 Feb 2020 09:28:04 +0100 Subject: [PATCH 3/5] more English grammar --- src/librustc_mir/interpret/validity.rs | 8 ++++---- src/test/ui/consts/const-eval/ub-ref.rs | 2 +- src/test/ui/consts/const-eval/ub-ref.stderr | 6 +++--- src/test/ui/consts/const-eval/ub-upvars.stderr | 2 +- src/test/ui/consts/const-eval/ub-wide-ptr.stderr | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index e90a5985677..f2c956c80f7 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -362,12 +362,12 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M ); match err.kind { err_unsup!(InvalidNullPointerUsage) => { - throw_validation_failure!("NULL reference", self.path) + throw_validation_failure!("a NULL reference", self.path) } err_unsup!(AlignmentCheckFailed { required, has }) => { throw_validation_failure!( format_args!( - "unaligned reference \ + "an unaligned reference \ (required {} byte alignment but found {})", required.bytes(), has.bytes() @@ -376,11 +376,11 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M ) } err_unsup!(ReadBytesAsPointer) => throw_validation_failure!( - "dangling reference (created from integer)", + "a dangling reference (created from integer)", self.path ), _ => throw_validation_failure!( - "dangling reference (not entirely in bounds)", + "a dangling reference (not entirely in bounds)", self.path ), } diff --git a/src/test/ui/consts/const-eval/ub-ref.rs b/src/test/ui/consts/const-eval/ub-ref.rs index 03ac12c8b1a..889579ca1ec 100644 --- a/src/test/ui/consts/const-eval/ub-ref.rs +++ b/src/test/ui/consts/const-eval/ub-ref.rs @@ -6,7 +6,7 @@ use std::mem; const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) }; //~^ ERROR it is undefined behavior to use this value -//~^^ type validation failed: encountered unaligned reference (required 2 byte alignment but found 1) +//~^^ type validation failed: encountered an unaligned reference (required 2 byte alignment but found 1) const NULL: &u16 = unsafe { mem::transmute(0usize) }; //~^ ERROR it is undefined behavior to use this value diff --git a/src/test/ui/consts/const-eval/ub-ref.stderr b/src/test/ui/consts/const-eval/ub-ref.stderr index 86d5d17c538..5cef0a488eb 100644 --- a/src/test/ui/consts/const-eval/ub-ref.stderr +++ b/src/test/ui/consts/const-eval/ub-ref.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-ref.rs:7:1 | LL | const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered unaligned reference (required 2 byte alignment but found 1) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered an unaligned reference (required 2 byte alignment but found 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 rustc repository if you believe it should not be considered undefined behavior. @@ -10,7 +10,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-ref.rs:11:1 | LL | const NULL: &u16 = unsafe { mem::transmute(0usize) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered NULL reference + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a NULL reference | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -34,7 +34,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-ref.rs:23:1 | LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (created from integer) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (created from integer) | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-upvars.stderr b/src/test/ui/consts/const-eval/ub-upvars.stderr index c1df447cd85..7e4633b0908 100644 --- a/src/test/ui/consts/const-eval/ub-upvars.stderr +++ b/src/test/ui/consts/const-eval/ub-upvars.stderr @@ -6,7 +6,7 @@ LL | | let bad_ref: &'static u16 = unsafe { mem::transmute(0usize) }; LL | | let another_var = 13; LL | | move || { let _ = bad_ref; let _ = another_var; } LL | | }; - | |__^ type validation failed: encountered NULL reference at ... + | |__^ type validation failed: encountered a NULL reference at ... | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-wide-ptr.stderr b/src/test/ui/consts/const-eval/ub-wide-ptr.stderr index 170037b44da..4da9ad6c332 100644 --- a/src/test/ui/consts/const-eval/ub-wide-ptr.stderr +++ b/src/test/ui/consts/const-eval/ub-wide-ptr.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:86:1 | LL | const STR_TOO_LONG: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.str}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (not entirely in bounds) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (not entirely in bounds) | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -50,7 +50,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:109:1 | LL | const SLICE_TOO_LONG: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.slice}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (not entirely in bounds) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (not entirely in bounds) | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. From dc92ad4d5bd06f7c62a924090001d1620fc74f3e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 26 Feb 2020 10:26:14 +0100 Subject: [PATCH 4/5] adjust LayoutDetails comments --- src/librustc_target/abi/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 447487e4fb2..67a5246012d 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -871,26 +871,26 @@ impl Niche { #[derive(PartialEq, Eq, Hash, Debug, HashStable_Generic)] pub struct LayoutDetails { - /// Says where the fields are located. + /// Says where the fields are located within the layout. /// Primitives and fieldless enums appear as unions without fields. pub fields: FieldPlacement, /// Encodes information about multi-variant layouts. - /// Even with `Multiple` variants, a layout can still have fields! Those are then + /// Even with `Multiple` variants, a layout still has its own fields! Those are then /// shared between all variants. One of them will be the discriminant, /// but e.g. generators can have more. /// - /// A layout-guided recursive descent must first look at all the fields, - /// and only then check if this is a multi-variant layout and if so, proceed - /// with the active variant. + /// To access all fields of this layout, both `fields` and the fields of the active variant + /// must be taken into account. pub variants: Variants, /// The `abi` defines how this data is passed between functions, and it defines /// value restrictions via `valid_range`. /// - /// Note that this is entirely orthogonal to the recursive structrue defined by + /// Note that this is entirely orthogonal to the recursive structure defined by /// `variants` and `fields`; for example, `ManuallyDrop>` has - /// `Abi::ScalarPair`! So, having a non-`Aggregate` `abi` should not stop a recursive descent. + /// `Abi::ScalarPair`! So, even with non-`Aggregate` `abi`, `fields` and `variants` + /// have to be taken into account to find all fields of this layout. pub abi: Abi, /// The leaf scalar with the largest number of invalid values From 12054dce9ac82ec63aedb91082e3ea9cdc8c2e5c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 26 Feb 2020 12:00:33 +0100 Subject: [PATCH 5/5] miri: validity visitor comments and path printing improvements --- src/librustc_mir/interpret/validity.rs | 67 +++++++++++++------ src/test/ui/consts/const-eval/ub-enum.stderr | 2 +- .../ui/consts/const-eval/ub-upvars.stderr | 2 +- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index f2c956c80f7..263883d5639 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -67,11 +67,12 @@ pub enum PathElem { Field(Symbol), Variant(Symbol), GeneratorState(VariantIdx), - ClosureVar(Symbol), + CapturedVar(Symbol), ArrayElem(usize), TupleElem(usize), Deref, - Tag, + EnumTag, + GeneratorTag, DynDowncast, } @@ -109,9 +110,11 @@ fn write_path(out: &mut String, path: &Vec) { for elem in path.iter() { match elem { Field(name) => write!(out, ".{}", name), - Variant(name) => write!(out, ".", name), + EnumTag => write!(out, "."), + Variant(name) => write!(out, ".", name), + GeneratorTag => write!(out, "."), GeneratorState(idx) => write!(out, ".", idx.index()), - ClosureVar(name) => write!(out, ".", name), + CapturedVar(name) => write!(out, ".", name), TupleElem(idx) => write!(out, ".{}", idx), ArrayElem(idx) => write!(out, "[{}]", idx), // `.` does not match Rust syntax, but it is more readable for long paths -- and @@ -119,7 +122,6 @@ fn write_path(out: &mut String, path: &Vec) { // even use the usual syntax because we are just showing the projections, // not the root. Deref => write!(out, "."), - Tag => write!(out, "."), DynDowncast => write!(out, "."), } .unwrap() @@ -170,6 +172,21 @@ struct ValidityVisitor<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> { impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M> { fn aggregate_field_path_elem(&mut self, layout: TyLayout<'tcx>, field: usize) -> PathElem { + // First, check if we are projecting to a variant. + match layout.variants { + layout::Variants::Multiple { discr_index, .. } => { + if discr_index == field { + return match layout.ty.kind { + ty::Adt(def, ..) if def.is_enum() => PathElem::EnumTag, + ty::Generator(..) => PathElem::GeneratorTag, + _ => bug!("non-variant type {:?}", layout.ty), + }; + } + } + layout::Variants::Single { .. } => {} + } + + // Now we know we are projecting to a field, so figure out which one. match layout.ty.kind { // generators and closures. ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { @@ -190,7 +207,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M } } - PathElem::ClosureVar(name.unwrap_or_else(|| { + PathElem::CapturedVar(name.unwrap_or_else(|| { // Fall back to showing the field index. sym::integer(field) })) @@ -207,10 +224,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M // Inside a variant PathElem::Field(def.variants[index].fields[field].ident.name) } - layout::Variants::Multiple { discr_index, .. } => { - assert_eq!(discr_index, field); - PathElem::Tag - } + layout::Variants::Multiple { .. } => bug!("we handled variants above"), } } @@ -441,11 +455,12 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M fn visit_scalar( &mut self, op: OpTy<'tcx, M::PointerTag>, - layout: &layout::Scalar, + scalar_layout: &layout::Scalar, ) -> InterpResult<'tcx> { let value = self.ecx.read_scalar(op)?; + let valid_range = &scalar_layout.valid_range; + let (lo, hi) = valid_range.clone().into_inner(); // Determine the allowed range - let (lo, hi) = layout.valid_range.clone().into_inner(); // `max_hi` is as big as the size fits let max_hi = u128::max_value() >> (128 - op.layout.size.bits()); assert!(hi <= max_hi); @@ -459,7 +474,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M value.not_undef(), value, self.path, - format_args!("something {}", wrapping_range_format(&layout.valid_range, max_hi),) + format_args!("something {}", wrapping_range_format(valid_range, max_hi),) ); let bits = match value.to_bits_or_ptr(op.layout.size, self.ecx) { Err(ptr) => { @@ -471,7 +486,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M self.path, format_args!( "something that cannot possibly fail to be {}", - wrapping_range_format(&layout.valid_range, max_hi) + wrapping_range_format(valid_range, max_hi) ) ) } @@ -484,7 +499,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M self.path, format_args!( "something that cannot possibly fail to be {}", - wrapping_range_format(&layout.valid_range, max_hi) + wrapping_range_format(valid_range, max_hi) ) ) } @@ -492,13 +507,13 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M Ok(data) => data, }; // Now compare. This is slightly subtle because this is a special "wrap-around" range. - if wrapping_range_contains(&layout.valid_range, bits) { + if wrapping_range_contains(&valid_range, bits) { Ok(()) } else { throw_validation_failure!( bits, self.path, - format_args!("something {}", wrapping_range_format(&layout.valid_range, max_hi)) + format_args!("something {}", wrapping_range_format(valid_range, max_hi)) ) } } @@ -594,7 +609,8 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // *After* all of this, check the ABI. We need to check the ABI to handle // types like `NonNull` where the `Scalar` info is more restrictive than what - // the fields say. But in most cases, this will just propagate what the fields say, + // the fields say (`rustc_layout_scalar_valid_range_start`). + // But in most cases, this will just propagate what the fields say, // and then we want the error to point at the field -- so, first recurse, // then check ABI. // @@ -603,11 +619,18 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // MyNewtype and then the scalar in there). match op.layout.abi { layout::Abi::Uninhabited => unreachable!(), // checked above - layout::Abi::Scalar(ref layout) => { - self.visit_scalar(op, layout)?; + layout::Abi::Scalar(ref scalar_layout) => { + self.visit_scalar(op, scalar_layout)?; + } + layout::Abi::ScalarPair { .. } | layout::Abi::Vector { .. } => { + // These have fields that we already visited above, so we already checked + // all their scalar-level restrictions. + // There is also no equivalent to `rustc_layout_scalar_valid_range_start` + // that would make skipping them here an issue. + } + layout::Abi::Aggregate { .. } => { + // Nothing to do. } - // FIXME: Should we do something for ScalarPair? Vector? - _ => {} } Ok(()) diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 3680f4f2ac2..8c47d68e968 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -66,7 +66,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:71:1 | LL | const BAD_OPTION_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..0.1, but expected a valid unicode codepoint + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..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 rustc repository if you believe it should not be considered undefined behavior. diff --git a/src/test/ui/consts/const-eval/ub-upvars.stderr b/src/test/ui/consts/const-eval/ub-upvars.stderr index 7e4633b0908..972c9eb38c8 100644 --- a/src/test/ui/consts/const-eval/ub-upvars.stderr +++ b/src/test/ui/consts/const-eval/ub-upvars.stderr @@ -6,7 +6,7 @@ LL | | let bad_ref: &'static u16 = unsafe { mem::transmute(0usize) }; LL | | let another_var = 13; LL | | move || { let _ = bad_ref; let _ = another_var; } LL | | }; - | |__^ type validation failed: encountered a NULL reference at ... + | |__^ type validation failed: encountered a NULL reference at ... | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.