Skip to content

Commit e1beee4

Browse files
borsbors
committed
Auto merge of #74059 - RalfJung:miri-uninit-validation, r=oli-obk
Miri value validation: fix handling of uninit memory Fixes rust-lang/miri#1456 Fixes rust-lang/miri#1467 r? @oli-obk
2 parents 51eeabf + 319c7f7 commit e1beee4

File tree

6 files changed

+69
-28
lines changed

6 files changed

+69
-28
lines changed

‎src/librustc_mir/interpret/validity.rs

+43-22
Original file line numberDiff line numberDiff line change
@@ -276,19 +276,21 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
276276
}
277277
}
278278

279-
fn visit_elem(
279+
fn with_elem<R>(
280280
&mut self,
281-
new_op: OpTy<'tcx, M::PointerTag>,
282281
elem: PathElem,
283-
) -> InterpResult<'tcx> {
282+
f: impl FnOnce(&mut Self) -> InterpResult<'tcx, R>,
283+
) -> InterpResult<'tcx, R> {
284284
// Remember the old state
285285
let path_len = self.path.len();
286-
// Perform operation
286+
// Record new element
287287
self.path.push(elem);
288-
self.visit_value(new_op)?;
288+
// Perform operation
289+
let r = f(self)?;
289290
// Undo changes
290291
self.path.truncate(path_len);
291-
Ok(())
292+
// Done
293+
Ok(r)
292294
}
293295

294296
fn check_wide_ptr_meta(
@@ -366,7 +368,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
366368
let place = try_validation!(
367369
self.ecx.ref_to_mplace(value),
368370
self.path,
369-
err_ub!(InvalidUninitBytes { .. }) => { "uninitialized {}", kind },
371+
err_ub!(InvalidUninitBytes(None)) => { "uninitialized {}", kind },
370372
);
371373
if place.layout.is_unsized() {
372374
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, '
477479
try_validation!(
478480
value.to_bool(),
479481
self.path,
480-
err_ub!(InvalidBool(..)) => { "{}", value } expected { "a boolean" },
482+
err_ub!(InvalidBool(..)) | err_ub!(InvalidUninitBytes(None)) =>
483+
{ "{}", value } expected { "a boolean" },
481484
);
482485
Ok(true)
483486
}
@@ -486,7 +489,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
486489
try_validation!(
487490
value.to_char(),
488491
self.path,
489-
err_ub!(InvalidChar(..)) => { "{}", value } expected { "a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)" },
492+
err_ub!(InvalidChar(..)) | err_ub!(InvalidUninitBytes(None)) =>
493+
{ "{}", value } expected { "a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)" },
490494
);
491495
Ok(true)
492496
}
@@ -515,7 +519,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
515519
let place = try_validation!(
516520
self.ecx.ref_to_mplace(self.ecx.read_immediate(value)?),
517521
self.path,
518-
err_ub!(InvalidUninitBytes { .. } ) => { "uninitialized raw pointer" },
522+
err_ub!(InvalidUninitBytes(None)) => { "uninitialized raw pointer" },
519523
);
520524
if place.layout.is_unsized() {
521525
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, '
537541
self.path,
538542
err_ub!(DanglingIntPointer(..)) |
539543
err_ub!(InvalidFunctionPointer(..)) |
544+
err_ub!(InvalidUninitBytes(None)) |
540545
err_unsup!(ReadBytesAsPointer) =>
541546
{ "{}", value } expected { "a function pointer" },
542547
);
@@ -593,7 +598,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
593598
let value = try_validation!(
594599
value.not_undef(),
595600
self.path,
596-
err_ub!(InvalidUninitBytes { .. }) => { "{}", value }
601+
err_ub!(InvalidUninitBytes(None)) => { "{}", value }
597602
expected { "something {}", wrapping_range_format(valid_range, max_hi) },
598603
);
599604
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>
646651
&self.ecx
647652
}
648653

654+
fn read_discriminant(
655+
&mut self,
656+
op: OpTy<'tcx, M::PointerTag>,
657+
) -> InterpResult<'tcx, VariantIdx> {
658+
self.with_elem(PathElem::EnumTag, move |this| {
659+
Ok(try_validation!(
660+
this.ecx.read_discriminant(op),
661+
this.path,
662+
err_ub!(InvalidTag(val)) =>
663+
{ "{}", val } expected { "a valid enum tag" },
664+
err_ub!(InvalidUninitBytes(None)) =>
665+
{ "uninitialized bytes" } expected { "a valid enum tag" },
666+
err_unsup!(ReadPointerAsBytes) =>
667+
{ "a pointer" } expected { "a valid enum tag" },
668+
)
669+
.1)
670+
})
671+
}
672+
649673
#[inline]
650674
fn visit_field(
651675
&mut self,
@@ -654,7 +678,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
654678
new_op: OpTy<'tcx, M::PointerTag>,
655679
) -> InterpResult<'tcx> {
656680
let elem = self.aggregate_field_path_elem(old_op.layout, field);
657-
self.visit_elem(new_op, elem)
681+
self.with_elem(elem, move |this| this.visit_value(new_op))
658682
}
659683

660684
#[inline]
@@ -670,7 +694,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
670694
ty::Generator(..) => PathElem::GeneratorState(variant_id),
671695
_ => bug!("Unexpected type with variant: {:?}", old_op.layout.ty),
672696
};
673-
self.visit_elem(new_op, name)
697+
self.with_elem(name, move |this| this.visit_value(new_op))
674698
}
675699

676700
#[inline(always)]
@@ -693,15 +717,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
693717
// Sanity check: `builtin_deref` does not know any pointers that are not primitive.
694718
assert!(op.layout.ty.builtin_deref(true).is_none());
695719

696-
// Recursively walk the type. Translate some possible errors to something nicer.
697-
try_validation!(
698-
self.walk_value(op),
699-
self.path,
700-
err_ub!(InvalidTag(val)) =>
701-
{ "{}", val } expected { "a valid enum tag" },
702-
err_unsup!(ReadPointerAsBytes) =>
703-
{ "a pointer" } expected { "plain (non-pointer) bytes" },
704-
);
720+
// Recursively walk the value at its type.
721+
self.walk_value(op)?;
705722

706723
// *After* all of this, check the ABI. We need to check the ABI to handle
707724
// 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>
816833

817834
throw_validation_failure!(self.path, { "uninitialized bytes" })
818835
}
836+
err_unsup!(ReadPointerAsBytes) => {
837+
throw_validation_failure!(self.path, { "a pointer" } expected { "plain (non-pointer) bytes" })
838+
}
839+
819840
// Propagate upwards (that will also check for unexpected errors).
820841
_ => return Err(err),
821842
}

‎src/librustc_mir/interpret/visitor.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,15 @@ macro_rules! make_value_visitor {
125125
fn ecx(&$($mutability)? self)
126126
-> &$($mutability)? InterpCx<'mir, 'tcx, M>;
127127

128+
/// `read_discriminant` can be hooked for better error messages.
129+
#[inline(always)]
130+
fn read_discriminant(
131+
&mut self,
132+
op: OpTy<'tcx, M::PointerTag>,
133+
) -> InterpResult<'tcx, VariantIdx> {
134+
Ok(self.ecx().read_discriminant(op)?.1)
135+
}
136+
128137
// Recursive actions, ready to be overloaded.
129138
/// Visits the given value, dispatching as appropriate to more specialized visitors.
130139
#[inline(always)]
@@ -245,7 +254,7 @@ macro_rules! make_value_visitor {
245254
// with *its* fields.
246255
Variants::Multiple { .. } => {
247256
let op = v.to_op(self.ecx())?;
248-
let idx = self.ecx().read_discriminant(op)?.1;
257+
let idx = self.read_discriminant(op)?;
249258
let inner = v.project_downcast(self.ecx(), idx)?;
250259
trace!("walk_value: variant layout: {:#?}", inner.layout());
251260
// recurse with the inner type

‎src/test/ui/consts/const-eval/double_check2.stderr

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

‎src/test/ui/consts/const-eval/ub-enum.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
22
--> $DIR/ub-enum.rs:24:1
33
|
44
LL | const BAD_ENUM: Enum = unsafe { mem::transmute(1usize) };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x00000001, but expected a valid enum tag
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x00000001 at .<enum-tag>, but expected a valid enum tag
66
|
77
= 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.
88

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

‎src/test/ui/consts/const-eval/union-ub.rs

+3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#[repr(C)]
44
union DummyUnion {
5+
unit: (),
56
u8: u8,
67
bool: bool,
78
}
@@ -30,6 +31,8 @@ union Bar {
3031
// the value is not valid for bools
3132
const BAD_BOOL: bool = unsafe { DummyUnion { u8: 42 }.bool};
3233
//~^ ERROR it is undefined behavior to use this value
34+
const UNINIT_BOOL: bool = unsafe { DummyUnion { unit: () }.bool};
35+
//~^ ERROR it is undefined behavior to use this value
3336

3437
// The value is not valid for any union variant, but that's fine
3538
// unions are just a convenient way to transmute bits around

‎src/test/ui/consts/const-eval/union-ub.stderr

+10-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
11
error[E0080]: it is undefined behavior to use this value
2-
--> $DIR/union-ub.rs:31:1
2+
--> $DIR/union-ub.rs:32:1
33
|
44
LL | const BAD_BOOL: bool = unsafe { DummyUnion { u8: 42 }.bool};
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x2a, but expected a boolean
66
|
77
= 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.
88

9-
error: aborting due to previous error
9+
error[E0080]: it is undefined behavior to use this value
10+
--> $DIR/union-ub.rs:34:1
11+
|
12+
LL | const UNINIT_BOOL: bool = unsafe { DummyUnion { unit: () }.bool};
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a boolean
14+
|
15+
= 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.
16+
17+
error: aborting due to 2 previous errors
1018

1119
For more information about this error, try `rustc --explain E0080`.

0 commit comments

Comments
 (0)