Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reword Miri validity errors: undefined -> uninitialized #71164

Merged
merged 3 commits into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/librustc_mir/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,11 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M
let value = self.ecx.read_immediate(value)?;
// Handle wide pointers.
// Check metadata early, for better diagnostics
let place = try_validation!(self.ecx.ref_to_mplace(value), "undefined pointer", self.path);
let place = try_validation!(
self.ecx.ref_to_mplace(value),
format_args!("uninitialized {}", kind),
self.path
);
if place.layout.is_unsized() {
self.check_wide_ptr_meta(place.meta, place.layout)?;
}
Expand All @@ -334,7 +338,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M
format_args!("invalid {} metadata: {}", kind, msg),
self.path
),
_ => bug!("Unexpected error during ptr size_and_align_of: {}", err),
_ => bug!("unexpected error during ptr size_and_align_of: {}", err),
},
};
let (size, align) = size_and_align
Expand Down Expand Up @@ -477,10 +481,11 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M
}
ty::RawPtr(..) => {
// We are conservative with undef for integers, but try to
// actually enforce our current rules for raw pointers.
// actually enforce the strict rules for raw pointers (mostly because
// that lets us re-use `ref_to_mplace`).
let place = try_validation!(
self.ecx.ref_to_mplace(self.ecx.read_immediate(value)?),
"undefined pointer",
"uninitialized raw pointer",
self.path
);
if place.layout.is_unsized() {
Expand Down Expand Up @@ -776,14 +781,14 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
// For some errors we might be able to provide extra information
match err.kind {
err_ub!(InvalidUndefBytes(Some(ptr))) => {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
// Some byte was undefined, determine which
// Some byte was uninitialized, determine which
// element that byte belongs to so we can
// provide an index.
let i = usize::try_from(ptr.offset.bytes() / layout.size.bytes())
.unwrap();
self.path.push(PathElem::ArrayElem(i));

throw_validation_failure!("undefined bytes", self.path)
throw_validation_failure!("uninitialized bytes", self.path)
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
}
// Other errors shouldn't be possible
_ => return Err(err),
Expand Down
65 changes: 65 additions & 0 deletions src/test/ui/consts/const-eval/ub-int-array.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#![feature(const_transmute)]
#![allow(const_err)] // make sure we cannot allow away the errors tested here

//! Test the "array of int" fast path in validity checking, and in particular whether it
//! points at the right array element.

use std::mem;

#[repr(C)]
union MaybeUninit<T: Copy> {
uninit: (),
init: T,
}

const UNINIT_INT_0: [u32; 3] = unsafe {
//~^ ERROR it is undefined behavior to use this value
//~| type validation failed: encountered uninitialized bytes at [0]
[
MaybeUninit { uninit: () }.init,
1,
2,
]
};
const UNINIT_INT_1: [u32; 3] = unsafe {
//~^ ERROR it is undefined behavior to use this value
//~| type validation failed: encountered uninitialized bytes at [1]
mem::transmute(
[
0u8,
0u8,
0u8,
0u8,
1u8,
MaybeUninit { uninit: () }.init,
1u8,
1u8,
2u8,
2u8,
MaybeUninit { uninit: () }.init,
2u8,
]
)
};
const UNINIT_INT_2: [u32; 3] = unsafe {
//~^ ERROR it is undefined behavior to use this value
//~| type validation failed: encountered uninitialized bytes at [2]
mem::transmute(
[
0u8,
0u8,
0u8,
0u8,
1u8,
1u8,
1u8,
1u8,
2u8,
2u8,
2u8,
MaybeUninit { uninit: () }.init,
]
)
};

fn main() {}
45 changes: 45 additions & 0 deletions src/test/ui/consts/const-eval/ub-int-array.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-int-array.rs:15:1
|
LL | / const UNINIT_INT_0: [u32; 3] = unsafe {
LL | |
LL | |
LL | | [
... |
LL | | ]
LL | | };
| |__^ type validation failed: encountered uninitialized bytes at [0]
Copy link
Contributor

@oli-obk oli-obk Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should dump the memory of constants at some point and have diagnostics point into them (but in general it's useless for validation to point to the entire constant's span, just pointing to the name should suffice)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's not great -- but also pre-existing.

|
= 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[E0080]: it is undefined behavior to use this value
--> $DIR/ub-int-array.rs:24:1
|
LL | / const UNINIT_INT_1: [u32; 3] = unsafe {
LL | |
LL | |
LL | | mem::transmute(
... |
LL | | )
LL | | };
| |__^ type validation failed: encountered uninitialized bytes at [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.

error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-int-array.rs:44:1
|
LL | / const UNINIT_INT_2: [u32; 3] = unsafe {
LL | |
LL | |
LL | | mem::transmute(
... |
LL | | )
LL | | };
| |__^ type validation failed: encountered uninitialized bytes at [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 rustc repository if you believe it should not be considered undefined behavior.

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0080`.
4 changes: 2 additions & 2 deletions src/test/ui/consts/const-eval/ub-ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ use std::mem;

const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) };
//~^ ERROR it is undefined behavior to use this value
//~^^ type validation failed: encountered an unaligned reference (required 2 byte alignment but found 1)
//~| type validation failed: encountered an unaligned reference (required 2 byte alignment but found 1)

const UNALIGNED_BOX: Box<u16> = unsafe { mem::transmute(&[0u8; 4]) };
//~^ ERROR it is undefined behavior to use this value
//~^^ type validation failed: encountered an unaligned box (required 2 byte alignment but found 1)
//~| type validation failed: encountered an unaligned box (required 2 byte alignment but found 1)

const NULL: &u16 = unsafe { mem::transmute(0usize) };
//~^ ERROR it is undefined behavior to use this value
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/consts/const-eval/ub-wide-ptr.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ LL | |
LL | | let uninit_len = MaybeUninit::<usize> { uninit: () };
LL | | mem::transmute((42, uninit_len))
LL | | };
| |__^ type validation failed: encountered undefined pointer
| |__^ type validation failed: encountered uninitialized 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.

Expand Down Expand Up @@ -130,7 +130,7 @@ LL | |
LL | | let uninit_len = MaybeUninit::<usize> { uninit: () };
LL | | mem::transmute((42, uninit_len))
LL | | };
| |__^ type validation failed: encountered undefined pointer
| |__^ type validation failed: encountered uninitialized raw pointer
|
= 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.

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/const-eval/union-ice.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ LL | | unsafe { UNION.field3 },
... |
LL | | a: 42,
LL | | };
| |__^ type validation failed: encountered undefined bytes at .b[1]
| |__^ type validation failed: encountered uninitialized bytes at .b[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.

Expand Down