Skip to content

Improve CTFE UB validation error messages #86275

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.

Sign up for GitHub

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

Merged
bors merged 4 commits into from
Jun 14, 2021
Merged

Improve CTFE UB validation error messages #86275

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87ecf84
Improve CTFE validation error message
lqd Jun 13, 2021
b877240
update tests involving CTFE validation
lqd Jun 13, 2021
19fddc0
Improve documentation on `UndefinedBehaviorInfo::ValidationFailure`
lqd Jun 14, 2021
5af1c72
Adjust `throw_validation_failure` macro to shorten the use of `with_n…
lqd Jun 14, 2021
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
14 changes: 10 additions & 4 deletions compiler/rustc_middle/src/mir/interpret/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,10 @@ pub enum UndefinedBehaviorInfo<'tcx> {
/// The value validity check found a problem.
/// Should only be thrown by `validity.rs` and always point out which part of the value
/// is the problem.
ValidationFailure(String),
ValidationFailure {
path: Option<String>,
msg: String,
},
/// Using a non-boolean `u8` as bool.
InvalidBool(u8),
/// Using a non-character `u32` as character.
Expand Down Expand Up @@ -331,7 +334,10 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
),
WriteToReadOnly(a) => write!(f, "writing to {} which is read-only", a),
DerefFunctionPointer(a) => write!(f, "accessing {} which contains a function", a),
ValidationFailure(ref err) => write!(f, "type validation failed: {}", err),
ValidationFailure { path: None, msg } => write!(f, "type validation failed: {}", msg),
ValidationFailure { path: Some(path), msg } => {
write!(f, "type validation failed at {}: {}", path, msg)
}
InvalidBool(b) => {
write!(f, "interpreting an invalid 8-bit value as a bool: 0x{:02x}", b)
}
Expand Down Expand Up @@ -499,13 +505,13 @@ impl fmt::Debug for InterpError<'_> {
}

impl InterpError<'_> {
/// Some errors to string formatting even if the error is never printed.
/// Some errors do string formatting even if the error is never printed.
/// To avoid performance issues, there are places where we want to be sure to never raise these formatting errors,
/// so this method lets us detect them and `bug!` on unexpected errors.
pub fn formatted_string(&self) -> bool {
match self {
InterpError::Unsupported(UnsupportedOpInfo::Unsupported(_))
| InterpError::UndefinedBehavior(UndefinedBehaviorInfo::ValidationFailure(_))
| InterpError::UndefinedBehavior(UndefinedBehaviorInfo::ValidationFailure { .. })
| InterpError::UndefinedBehavior(UndefinedBehaviorInfo::Ub(_)) => true,
_ => false,
}
Expand Down
20 changes: 12 additions & 8 deletions compiler/rustc_mir/src/interpret/validity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,27 @@ use super::{

macro_rules! throw_validation_failure {
($where:expr, { $( $what_fmt:expr ),+ } $( expected { $( $expected_fmt:expr ),+ } )?) => {{
let msg = rustc_middle::ty::print::with_no_trimmed_paths(|| {
let (path, msg) = rustc_middle::ty::print::with_no_trimmed_paths(|| {
Copy link
Member

@RalfJung RalfJung Jun 14, 2021

Choose a reason for hiding this comment

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

Why a single big let for both of them, and not two separate lets?

Copy link
Member Author

@lqd lqd Jun 14, 2021

Choose a reason for hiding this comment

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

it was just not to have two separate rustc_middle::ty::print::with_no_trimmed_path calls: I'm unsure whether the non-path expressions actually required being wrapped by it when they are serialized into strings (and was unsure of the costs), and so found it safer to not change this macro too much.

I can try changing this to have 2 lets but a single with_no_trimmed_path() call for the actual path, and see whether it changes the test output. Or have 2 lets and 2 calls. Let me know what you prefer and I'll do it.

Copy link
Member

@RalfJung RalfJung Jun 14, 2021

Choose a reason for hiding this comment

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

The msg part should not need that with_no_trimmed_paths thing, so maybe try two lets with just the path part having with_no_trimmed_paths and then the test suite will tell us if that works or not.

Copy link
Member Author

@lqd lqd Jun 14, 2021

Choose a reason for hiding this comment

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

alright, I'll try that tonight, thanks for the review !

Copy link
Member Author

@lqd lqd Jun 14, 2021

Choose a reason for hiding this comment

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

The ui test suite seems to agree that the msg doesn't need anything related to path trimming, so I've pushed a separate commit (to help with review) to address this point.

let mut msg = String::new();
msg.push_str("encountered ");
write!(&mut msg, $($what_fmt),+).unwrap();
let where_ = &$where;
if !where_.is_empty() {
msg.push_str(" at ");
write_path(&mut msg, where_);
}
$(
msg.push_str(", but expected ");
write!(&mut msg, $($expected_fmt),+).unwrap();
)?

msg
let where_ = &$where;
let path = if !where_.is_empty() {
let mut path = String::new();
write_path(&mut path, where_);
Some(path)
} else {
None
};

(path, msg)
});
throw_ub!(ValidationFailure(msg))
throw_ub!(ValidationFailure { path, msg })
}};
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/const-eval/heap/alloc_intrinsic_uninit.32bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/alloc_intrinsic_uninit.rs:9:1
|
LL | const BAR: &i32 = unsafe { &*(intrinsics::const_allocate(4, 4) as *mut i32) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes at .<deref>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<deref>: encountered uninitialized bytes, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/const-eval/heap/alloc_intrinsic_uninit.64bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/alloc_intrinsic_uninit.rs:9:1
|
LL | const BAR: &i32 = unsafe { &*(intrinsics::const_allocate(4, 4) as *mut i32) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes at .<deref>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<deref>: encountered uninitialized bytes, 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.
= note: the raw bytes of the constant (size: 8, align: 8) {
Expand Down
26 changes: 13 additions & 13 deletions src/test/ui/consts/const-eval/ub-enum.32bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 at .<enum-tag>, but expected a valid enum tag
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-tag>: encountered 0x00000001, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -13,7 +13,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:27:1
|
LL | const BAD_ENUM_PTR: Enum = unsafe { mem::transmute(&1) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc8 at .<enum-tag>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-tag>: encountered pointer to alloc8, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -24,7 +24,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:30:1
|
LL | const BAD_ENUM_WRAPPED: Wrap<Enum> = unsafe { mem::transmute(&1) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc12 at .0.<enum-tag>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .0.<enum-tag>: encountered pointer to alloc12, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -35,7 +35,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 at .<enum-tag>, but expected a valid enum tag
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-tag>: encountered 0x00000000, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -46,7 +46,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:44:1
|
LL | const BAD_ENUM2_PTR: Enum2 = unsafe { mem::transmute(&0) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc18 at .<enum-tag>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-tag>: encountered pointer to alloc18, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -57,7 +57,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:47:1
|
LL | const BAD_ENUM2_WRAPPED: Wrap<Enum2> = unsafe { mem::transmute(&0) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc22 at .0.<enum-tag>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .0.<enum-tag>: encountered pointer to alloc22, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -68,7 +68,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:56:1
|
LL | const BAD_ENUM2_UNDEF : Enum2 = unsafe { MaybeUninit { uninit: () }.init };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes at .<enum-tag>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-tag>: encountered uninitialized bytes, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -79,7 +79,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:60:1
|
LL | const BAD_ENUM2_OPTION_PTR: Option<Enum2> = unsafe { mem::transmute(&0) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc28 at .<enum-tag>, but expected initialized plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-tag>: encountered pointer to alloc28, 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.
= note: the raw bytes of the constant (size: 4, align: 4) {
Expand All @@ -90,7 +90,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:77:1
|
LL | const BAD_UNINHABITED_VARIANT1: UninhDiscriminant = unsafe { mem::transmute(1u8) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of the never type `!` at .<enum-variant(B)>.0
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-variant(B)>.0: encountered a value of the never type `!`
|
= 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.
= note: the raw bytes of the constant (size: 1, align: 1) {
Expand All @@ -101,7 +101,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:79:1
|
LL | const BAD_UNINHABITED_VARIANT2: UninhDiscriminant = unsafe { mem::transmute(3u8) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type Never at .<enum-variant(D)>.0
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-variant(D)>.0: encountered a value of uninhabited type Never
|
= 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.
= note: the raw bytes of the constant (size: 1, align: 1) {
Expand All @@ -112,7 +112,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:87:1
|
LL | const BAD_OPTION_CHAR: Option<(char, char)> = Some(('x', unsafe { mem::transmute(!0u32) }));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0xffffffff at .<enum-variant(Some)>.0.1, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-variant(Some)>.0.1: encountered 0xffffffff, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
|
= 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.
= note: the raw bytes of the constant (size: 8, align: 4) {
Expand All @@ -123,7 +123,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:92:1
|
LL | const BAD_UNINHABITED_WITH_DATA1: Result<(i32, Never), (i32, !)> = unsafe { mem::transmute(0u64) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type Never at .<enum-variant(Ok)>.0.1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-variant(Ok)>.0.1: encountered a value of uninhabited type Never
|
= 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.
= note: the raw bytes of the constant (size: 8, align: 4) {
Expand All @@ -134,7 +134,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-enum.rs:94:1
|
LL | const BAD_UNINHABITED_WITH_DATA2: Result<(i32, !), (i32, Never)> = unsafe { mem::transmute(0u64) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of the never type `!` at .<enum-variant(Ok)>.0.1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<enum-variant(Ok)>.0.1: encountered a value of the never type `!`
|
= 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.
= note: the raw bytes of the constant (size: 8, align: 4) {
Expand Down
Loading