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

don't ICE when encountering an extern type field during validation #126833

Merged
merged 1 commit into from
Jun 23, 2024
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
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ const_eval_exact_div_has_remainder =
const_eval_extern_static =
cannot access extern static ({$did})
const_eval_extern_type_field = `extern type` field does not have a known offset
const_eval_fn_ptr_call =
function pointers need an RFC before allowed to be called in {const_eval_const_context}s
const_eval_for_loop_into_iter_non_const =
Expand Down
76 changes: 44 additions & 32 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,33 +386,8 @@ fn eval_in_interpreter<'tcx, R: InterpretationResult<'tcx>>(
CompileTimeMachine::new(CanAccessMutGlobal::from(is_static), CheckAlignment::Error),
);
let res = ecx.load_mir(cid.instance.def, cid.promoted);
res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body)).map_err(|error| {
let (error, backtrace) = error.into_parts();
backtrace.print_backtrace();

let (kind, instance) = if ecx.tcx.is_static(cid.instance.def_id()) {
("static", String::new())
} else {
// If the current item has generics, we'd like to enrich the message with the
// instance and its args: to show the actual compile-time values, in addition to
// the expression, leading to the const eval error.
let instance = &cid.instance;
if !instance.args.is_empty() {
let instance = with_no_trimmed_paths!(instance.to_string());
("const_with_path", instance)
} else {
("const", String::new())
}
};

super::report(
*ecx.tcx,
error,
DUMMY_SP,
|| super::get_span_and_frames(ecx.tcx, ecx.stack()),
|span, frames| ConstEvalError { span, error_kind: kind, instance, frame_notes: frames },
)
})
res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body))
.map_err(|error| report_eval_error(&ecx, cid, error))
}

#[inline(always)]
Expand All @@ -438,24 +413,61 @@ fn const_validate_mplace<'tcx>(
ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)
// Instead of just reporting the `InterpError` via the usual machinery, we give a more targeted
// error about the validation failure.
.map_err(|error| report_validation_error(&ecx, error, alloc_id))?;
.map_err(|error| report_validation_error(&ecx, cid, error, alloc_id))?;
inner = true;
}

Ok(())
}

#[inline(always)]
#[inline(never)]
fn report_eval_error<'tcx>(
ecx: &InterpCx<'tcx, CompileTimeMachine<'tcx>>,
cid: GlobalId<'tcx>,
error: InterpErrorInfo<'tcx>,
) -> ErrorHandled {
let (error, backtrace) = error.into_parts();
backtrace.print_backtrace();

let (kind, instance) = if ecx.tcx.is_static(cid.instance.def_id()) {
("static", String::new())
} else {
// If the current item has generics, we'd like to enrich the message with the
// instance and its args: to show the actual compile-time values, in addition to
// the expression, leading to the const eval error.
let instance = &cid.instance;
if !instance.args.is_empty() {
let instance = with_no_trimmed_paths!(instance.to_string());
("const_with_path", instance)
} else {
("const", String::new())
}
};

super::report(
*ecx.tcx,
error,
DUMMY_SP,
|| super::get_span_and_frames(ecx.tcx, ecx.stack()),
|span, frames| ConstEvalError { span, error_kind: kind, instance, frame_notes: frames },
)
}

#[inline(never)]
fn report_validation_error<'tcx>(
ecx: &InterpCx<'tcx, CompileTimeMachine<'tcx>>,
cid: GlobalId<'tcx>,
error: InterpErrorInfo<'tcx>,
alloc_id: AllocId,
) -> ErrorHandled {
if !matches!(error.kind(), InterpError::UndefinedBehavior(_)) {
// Some other error happened during validation, e.g. an unsupported operation.
return report_eval_error(ecx, cid, error);
}

let (error, backtrace) = error.into_parts();
backtrace.print_backtrace();

let ub_note = matches!(error, InterpError::UndefinedBehavior(_)).then(|| {});

let bytes = ecx.print_alloc_bytes_for_diagnostics(alloc_id);
let (size, align, _) = ecx.get_alloc_info(alloc_id);
let raw_bytes = errors::RawBytesNote { size: size.bytes(), align: align.bytes(), bytes };
Expand All @@ -465,6 +477,6 @@ fn report_validation_error<'tcx>(
error,
DUMMY_SP,
|| crate::const_eval::get_span_and_frames(ecx.tcx, ecx.stack()),
move |span, frames| errors::ValidationFailure { span, ub_note, frames, raw_bytes },
move |span, frames| errors::ValidationFailure { span, ub_note: (), frames, raw_bytes },
)
}
8 changes: 6 additions & 2 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ pub struct ValidationFailure {
#[primary_span]
pub span: Span,
#[note(const_eval_validation_failure_note)]
pub ub_note: Option<()>,
pub ub_note: (),
Copy link
Member Author

Choose a reason for hiding this comment

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

There's probably something else that should be done instead of a () field here...? I don't know enough about our diagnostics stuff, unfortunately.

Copy link
Member

@compiler-errors compiler-errors Jun 23, 2024

Choose a reason for hiding this comment

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

() is fine. You could put #[note] on the struct itself but it changes the ordering of the labels/notes in some cases.

#[subdiagnostic]
pub frames: Vec<FrameNote>,
#[subdiagnostic]
Expand Down Expand Up @@ -825,6 +825,7 @@ impl ReportErrorExt for UnsupportedOpInfo {
use crate::fluent_generated::*;
match self {
UnsupportedOpInfo::Unsupported(s) => s.clone().into(),
UnsupportedOpInfo::ExternTypeField => const_eval_extern_type_field,
UnsupportedOpInfo::UnsizedLocal => const_eval_unsized_local,
UnsupportedOpInfo::OverwritePartialPointer(_) => const_eval_partial_pointer_overwrite,
UnsupportedOpInfo::ReadPartialPointer(_) => const_eval_partial_pointer_copy,
Expand All @@ -845,7 +846,10 @@ impl ReportErrorExt for UnsupportedOpInfo {
// `ReadPointerAsInt(Some(info))` is never printed anyway, it only serves as an error to
// be further processed by validity checking which then turns it into something nice to
// print. So it's not worth the effort of having diagnostics that can print the `info`.
UnsizedLocal | Unsupported(_) | ReadPointerAsInt(_) => {}
UnsizedLocal
| UnsupportedOpInfo::ExternTypeField
| Unsupported(_)
| ReadPointerAsInt(_) => {}
OverwritePartialPointer(ptr) | ReadPartialPointer(ptr) => {
diag.arg("ptr", ptr);
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_target::abi::{self, VariantIdx};
use tracing::{debug, instrument};

use super::{
throw_ub, throw_unsup_format, InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy,
throw_ub, throw_unsup, InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy,
Provenance, Scalar,
};

Expand Down Expand Up @@ -186,8 +186,8 @@ where
(base_meta, offset)
}
None => {
// We don't know the alignment of this field, so we cannot adjust.
throw_unsup_format!("`extern type` does not have a known offset")
// We cannot know the alignment of this field, so we cannot adjust.
throw_unsup!(ExternTypeField)
}
}
} else {
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//! to be const-safe.
use std::fmt::Write;
use std::hash::Hash;
use std::num::NonZero;

use either::{Left, Right};
Expand All @@ -17,7 +18,8 @@ use rustc_hir as hir;
use rustc_middle::bug;
use rustc_middle::mir::interpret::{
ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance,
ValidationErrorInfo, ValidationErrorKind, ValidationErrorKind::*,
UnsupportedOpInfo, ValidationErrorInfo,
ValidationErrorKind::{self, *},
};
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, Ty};
Expand All @@ -26,8 +28,6 @@ use rustc_target::abi::{
Abi, FieldIdx, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange,
};

use std::hash::Hash;

use super::{
err_ub, format_interp_error, machine::AllocMap, throw_ub, AllocId, AllocKind, CheckInAllocMsg,
GlobalAlloc, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy,
Expand Down Expand Up @@ -1028,7 +1028,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
Err(err)
if matches!(
err.kind(),
err_ub!(ValidationError { .. }) | InterpError::InvalidProgram(_)
err_ub!(ValidationError { .. })
| InterpError::InvalidProgram(_)
| InterpError::Unsupported(UnsupportedOpInfo::ExternTypeField)
) =>
{
Err(err)
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,8 @@ pub enum UnsupportedOpInfo {
Unsupported(String),
/// Unsized local variables.
UnsizedLocal,
/// Extern type field with an indeterminate offset.
ExternTypeField,
//
// The variants below are only reachable from CTFE/const prop, miri will never emit them.
//
Expand Down
4 changes: 3 additions & 1 deletion src/tools/miri/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,9 @@ pub fn report_error<'tcx>(
ResourceExhaustion(_) => "resource exhaustion",
Unsupported(
// We list only the ones that can actually happen.
UnsupportedOpInfo::Unsupported(_) | UnsupportedOpInfo::UnsizedLocal,
UnsupportedOpInfo::Unsupported(_)
| UnsupportedOpInfo::UnsizedLocal
| UnsupportedOpInfo::ExternTypeField,
) => "unsupported operation",
InvalidProgram(
// We list only the ones that can actually happen.
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/tests/fail/extern-type-field-offset.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: unsupported operation: `extern type` does not have a known offset
error: unsupported operation: `extern type` field does not have a known offset
--> $DIR/extern-type-field-offset.rs:LL:CC
|
LL | let _field = &x.a;
| ^^^^ `extern type` does not have a known offset
| ^^^^ `extern type` field does not have a known offset
|
= help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support
= note: BACKTRACE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0080]: evaluation of constant value failed
--> $DIR/issue-91827-extern-types-field-offset.rs:38:17
|
LL | let field = &x.a;
| ^^^^ `extern type` does not have a known offset
| ^^^^ `extern type` field does not have a known offset

error: aborting due to 1 previous error

Expand Down
15 changes: 15 additions & 0 deletions tests/ui/consts/const-eval/validation-ice-extern-type-field.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![feature(extern_types)]

extern {
type Opaque;
}

struct ThinDst {
x: u8,
tail: Opaque,
}

const C1: &ThinDst = unsafe { std::mem::transmute(b"d".as_ptr()) };
//~^ERROR: evaluation of constant value failed

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0080]: evaluation of constant value failed
--> $DIR/validation-ice-extern-type-field.rs:12:1
|
LL | const C1: &ThinDst = unsafe { std::mem::transmute(b"d".as_ptr()) };
| ^^^^^^^^^^^^^^^^^^ `extern type` field does not have a known offset

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0080`.
6 changes: 1 addition & 5 deletions tests/ui/sized/stack-overflow-trait-infer-98842.32bit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,11 @@ LL | const _: *const Foo = 0 as _;
| ^^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error[E0080]: it is undefined behavior to use this value
error[E0080]: evaluation of constant value failed
--> $DIR/stack-overflow-trait-infer-98842.rs:15:1
|
LL | const _: *const Foo = 0 as _;
| ^^^^^^^^^^^^^^^^^^^ a cycle occurred during layout computation
|
= note: the raw bytes of the constant (size: 4, align: 4) {
00 00 00 00 │ ....
}

error: aborting due to 2 previous errors

Expand Down
6 changes: 1 addition & 5 deletions tests/ui/sized/stack-overflow-trait-infer-98842.64bit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,11 @@ LL | const _: *const Foo = 0 as _;
| ^^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error[E0080]: it is undefined behavior to use this value
error[E0080]: evaluation of constant value failed
--> $DIR/stack-overflow-trait-infer-98842.rs:15:1
|
LL | const _: *const Foo = 0 as _;
| ^^^^^^^^^^^^^^^^^^^ a cycle occurred during layout computation
|
= note: the raw bytes of the constant (size: 8, align: 8) {
00 00 00 00 00 00 00 00 │ ........
}

error: aborting due to 2 previous errors

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/sized/stack-overflow-trait-infer-98842.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ struct Foo(<&'static Foo as ::core::ops::Deref>::Target);
// and it will infinitely recurse somewhere trying to figure out the
// size of this pointer (is my guess):
const _: *const Foo = 0 as _;
//~^ ERROR it is undefined behavior to use this value
//~^ ERROR evaluation of constant value failed

pub fn main() {}
25 changes: 0 additions & 25 deletions tests/ui/sized/stack-overflow-trait-infer-98842.stderr

This file was deleted.

Loading