Auto merge of #74059 - RalfJung:miri-uninit-validation, r=oli-obk

Miri value validation: fix handling of uninit memory

Fixes https://github.com/rust-lang/miri/issues/1456
Fixes https://github.com/rust-lang/miri/issues/1467

r? @oli-obk
This commit is contained in:
bors 2020-07-07 14:21:18 +00:00
commit e1beee4992
6 changed files with 69 additions and 28 deletions

View File

@ -276,19 +276,21 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
}
}
fn visit_elem(
fn with_elem<R>(
&mut self,
new_op: OpTy<'tcx, M::PointerTag>,
elem: PathElem,
) -> InterpResult<'tcx> {
f: impl FnOnce(&mut Self) -> InterpResult<'tcx, R>,
) -> InterpResult<'tcx, R> {
// Remember the old state
let path_len = self.path.len();
// Perform operation
// Record new element
self.path.push(elem);
self.visit_value(new_op)?;
// Perform operation
let r = f(self)?;
// Undo changes
self.path.truncate(path_len);
Ok(())
// Done
Ok(r)
}
fn check_wide_ptr_meta(
@ -366,7 +368,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let place = try_validation!(
self.ecx.ref_to_mplace(value),
self.path,
err_ub!(InvalidUninitBytes { .. }) => { "uninitialized {}", kind },
err_ub!(InvalidUninitBytes(None)) => { "uninitialized {}", kind },
);
if place.layout.is_unsized() {
self.check_wide_ptr_meta(place.meta, place.layout)?;
@ -477,7 +479,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
try_validation!(
value.to_bool(),
self.path,
err_ub!(InvalidBool(..)) => { "{}", value } expected { "a boolean" },
err_ub!(InvalidBool(..)) | err_ub!(InvalidUninitBytes(None)) =>
{ "{}", value } expected { "a boolean" },
);
Ok(true)
}
@ -486,7 +489,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
try_validation!(
value.to_char(),
self.path,
err_ub!(InvalidChar(..)) => { "{}", value } expected { "a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)" },
err_ub!(InvalidChar(..)) | err_ub!(InvalidUninitBytes(None)) =>
{ "{}", value } expected { "a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)" },
);
Ok(true)
}
@ -515,7 +519,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let place = try_validation!(
self.ecx.ref_to_mplace(self.ecx.read_immediate(value)?),
self.path,
err_ub!(InvalidUninitBytes { .. } ) => { "uninitialized raw pointer" },
err_ub!(InvalidUninitBytes(None)) => { "uninitialized raw pointer" },
);
if place.layout.is_unsized() {
self.check_wide_ptr_meta(place.meta, place.layout)?;
@ -537,6 +541,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
self.path,
err_ub!(DanglingIntPointer(..)) |
err_ub!(InvalidFunctionPointer(..)) |
err_ub!(InvalidUninitBytes(None)) |
err_unsup!(ReadBytesAsPointer) =>
{ "{}", value } expected { "a function pointer" },
);
@ -593,7 +598,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let value = try_validation!(
value.not_undef(),
self.path,
err_ub!(InvalidUninitBytes { .. }) => { "{}", value }
err_ub!(InvalidUninitBytes(None)) => { "{}", value }
expected { "something {}", wrapping_range_format(valid_range, max_hi) },
);
let bits = match value.to_bits_or_ptr(op.layout.size, self.ecx) {
@ -646,6 +651,25 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
&self.ecx
}
fn read_discriminant(
&mut self,
op: OpTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, VariantIdx> {
self.with_elem(PathElem::EnumTag, move |this| {
Ok(try_validation!(
this.ecx.read_discriminant(op),
this.path,
err_ub!(InvalidTag(val)) =>
{ "{}", val } expected { "a valid enum tag" },
err_ub!(InvalidUninitBytes(None)) =>
{ "uninitialized bytes" } expected { "a valid enum tag" },
err_unsup!(ReadPointerAsBytes) =>
{ "a pointer" } expected { "a valid enum tag" },
)
.1)
})
}
#[inline]
fn visit_field(
&mut self,
@ -654,7 +678,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
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)
self.with_elem(elem, move |this| this.visit_value(new_op))
}
#[inline]
@ -670,7 +694,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
ty::Generator(..) => PathElem::GeneratorState(variant_id),
_ => bug!("Unexpected type with variant: {:?}", old_op.layout.ty),
};
self.visit_elem(new_op, name)
self.with_elem(name, move |this| this.visit_value(new_op))
}
#[inline(always)]
@ -693,15 +717,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
// Sanity check: `builtin_deref` does not know any pointers that are not primitive.
assert!(op.layout.ty.builtin_deref(true).is_none());
// Recursively walk the type. Translate some possible errors to something nicer.
try_validation!(
self.walk_value(op),
self.path,
err_ub!(InvalidTag(val)) =>
{ "{}", val } expected { "a valid enum tag" },
err_unsup!(ReadPointerAsBytes) =>
{ "a pointer" } expected { "plain (non-pointer) bytes" },
);
// Recursively walk the value at its type.
self.walk_value(op)?;
// *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
@ -816,6 +833,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
throw_validation_failure!(self.path, { "uninitialized bytes" })
}
err_unsup!(ReadPointerAsBytes) => {
throw_validation_failure!(self.path, { "a pointer" } expected { "plain (non-pointer) bytes" })
}
// Propagate upwards (that will also check for unexpected errors).
_ => return Err(err),
}

View File

@ -125,6 +125,15 @@ macro_rules! make_value_visitor {
fn ecx(&$($mutability)? self)
-> &$($mutability)? InterpCx<'mir, 'tcx, M>;
/// `read_discriminant` can be hooked for better error messages.
#[inline(always)]
fn read_discriminant(
&mut self,
op: OpTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, VariantIdx> {
Ok(self.ecx().read_discriminant(op)?.1)
}
// Recursive actions, ready to be overloaded.
/// Visits the given value, dispatching as appropriate to more specialized visitors.
#[inline(always)]
@ -245,7 +254,7 @@ macro_rules! make_value_visitor {
// with *its* fields.
Variants::Multiple { .. } => {
let op = v.to_op(self.ecx())?;
let idx = self.ecx().read_discriminant(op)?.1;
let idx = self.read_discriminant(op)?;
let inner = v.project_downcast(self.ecx(), idx)?;
trace!("walk_value: variant layout: {:#?}", inner.layout());
// recurse with the inner type

View File

@ -5,7 +5,7 @@ LL | / static FOO: (&Foo, &Bar) = unsafe {(
LL | | Union { u8: &BAR }.foo,
LL | | Union { u8: &BAR }.bar,
LL | | )};
| |___^ type validation failed: encountered 0x05 at .1.<deref>, but expected a valid enum tag
| |___^ type validation failed: encountered 0x05 at .1.<deref>.<enum-tag>, but expected a valid enum tag
|
= 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.

View File

@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:24:1
|
LL | const BAD_ENUM: Enum = unsafe { mem::transmute(1usize) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x00000001, but expected a valid enum tag
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x00000001 at .<enum-tag>, but expected a valid enum tag
|
= 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.
@ -26,7 +26,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:42:1
|
LL | const BAD_ENUM2: Enum2 = unsafe { mem::transmute(0usize) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x00000000, but expected a valid enum tag
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x00000000 at .<enum-tag>, but expected a valid enum tag
|
= 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.

View File

@ -2,6 +2,7 @@
#[repr(C)]
union DummyUnion {
unit: (),
u8: u8,
bool: bool,
}
@ -30,6 +31,8 @@ union Bar {
// the value is not valid for bools
const BAD_BOOL: bool = unsafe { DummyUnion { u8: 42 }.bool};
//~^ ERROR it is undefined behavior to use this value
const UNINIT_BOOL: bool = unsafe { DummyUnion { unit: () }.bool};
//~^ ERROR it is undefined behavior to use this value
// The value is not valid for any union variant, but that's fine
// unions are just a convenient way to transmute bits around

View File

@ -1,11 +1,19 @@
error[E0080]: it is undefined behavior to use this value
--> $DIR/union-ub.rs:31:1
--> $DIR/union-ub.rs:32:1
|
LL | const BAD_BOOL: bool = unsafe { DummyUnion { u8: 42 }.bool};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x2a, 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.
error: aborting due to previous error
error[E0080]: it is undefined behavior to use this value
--> $DIR/union-ub.rs:34:1
|
LL | const UNINIT_BOOL: bool = unsafe { DummyUnion { unit: () }.bool};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, 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.
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0080`.