Skip to content

Enforce VarDebugInfo::Place in MIR validation. #109901

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

Merged
merged 1 commit into from
Apr 4, 2023
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
14 changes: 9 additions & 5 deletions compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,15 @@ fn calculate_debuginfo_offset<
mir::ProjectionElem::Downcast(_, variant) => {
place = place.downcast(bx, variant);
}
_ => span_bug!(
var.source_info.span,
"unsupported var debuginfo place `{:?}`",
mir::Place { local, projection: var.projection },
),
_ => {
// Sanity check for `can_use_in_debuginfo`.
debug_assert!(!elem.can_use_in_debuginfo());
span_bug!(
var.source_info.span,
"unsupported var debuginfo place `{:?}`",
mir::Place { local, projection: var.projection },
)
}
}
}

Expand Down
43 changes: 39 additions & 4 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ use rustc_index::bit_set::BitSet;
use rustc_index::vec::IndexVec;
use rustc_infer::traits::Reveal;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::visit::NonUseContext::VarDebugInfo;
use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::visit::{NonUseContext, PlaceContext, Visitor};
use rustc_middle::mir::{
traversal, BasicBlock, BinOp, Body, BorrowKind, CastKind, CopyNonOverlapping, Local, Location,
MirPass, MirPhase, NonDivergingIntrinsic, Operand, Place, PlaceElem, PlaceRef, ProjectionElem,
RetagKind, RuntimePhase, Rvalue, SourceScope, Statement, StatementKind, Terminator,
TerminatorKind, UnOp, START_BLOCK,
TerminatorKind, UnOp, VarDebugInfo, VarDebugInfoContents, START_BLOCK,
};
use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitableExt};
use rustc_mir_dataflow::impls::MaybeStorageLive;
Expand Down Expand Up @@ -419,13 +418,49 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.super_projection_elem(local, proj_base, elem, context, location);
}

fn visit_var_debug_info(&mut self, debuginfo: &VarDebugInfo<'tcx>) {
let check_place = |place: Place<'_>| {
if place.projection.iter().any(|p| !p.can_use_in_debuginfo()) {
self.fail(
START_BLOCK.start_location(),
format!("illegal place {:?} in debuginfo for {:?}", place, debuginfo.name),
);
}
};
match debuginfo.value {
VarDebugInfoContents::Const(_) => {}
VarDebugInfoContents::Place(place) => check_place(place),
VarDebugInfoContents::Composite { ty, ref fragments } => {
for f in fragments {
check_place(f.contents);
if ty.is_union() || ty.is_enum() {
self.fail(
START_BLOCK.start_location(),
format!("invalid type {:?} for composite debuginfo", ty),
);
}
if f.projection.iter().any(|p| !matches!(p, PlaceElem::Field(..))) {
self.fail(
START_BLOCK.start_location(),
format!(
"illegal projection {:?} in debuginfo for {:?}",
f.projection, debuginfo.name
),
);
}
}
}
}
self.super_var_debug_info(debuginfo);
}

fn visit_place(&mut self, place: &Place<'tcx>, cntxt: PlaceContext, location: Location) {
// Set off any `bug!`s in the type computation code
let _ = place.ty(&self.body.local_decls, self.tcx);

if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial)
&& place.projection.len() > 1
&& cntxt != PlaceContext::NonUse(VarDebugInfo)
&& cntxt != PlaceContext::NonUse(NonUseContext::VarDebugInfo)
&& place.projection[1..].contains(&ProjectionElem::Deref)
{
self.fail(location, format!("{:?}, has deref at the wrong place", place));
Expand Down
24 changes: 17 additions & 7 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,8 +1029,7 @@ impl<'tcx> LocalDecl<'tcx> {

#[derive(Clone, TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable)]
pub enum VarDebugInfoContents<'tcx> {
/// NOTE(eddyb) There's an unenforced invariant that this `Place` is
/// based on a `Local`, not a `Static`, and contains no indexing.
/// This `Place` only contains projection which satisfy `can_use_in_debuginfo`.
Place(Place<'tcx>),
Const(Constant<'tcx>),
/// The user variable's data is split across several fragments,
Expand All @@ -1040,6 +1039,7 @@ pub enum VarDebugInfoContents<'tcx> {
/// the underlying debuginfo feature this relies on.
Composite {
/// Type of the original user variable.
/// This cannot contain a union or an enum.
ty: Ty<'tcx>,
/// All the parts of the original user variable, which ended
/// up in disjoint places, due to optimizations.
Expand Down Expand Up @@ -1068,17 +1068,16 @@ pub struct VarDebugInfoFragment<'tcx> {
/// Where in the composite user variable this fragment is,
/// represented as a "projection" into the composite variable.
/// At lower levels, this corresponds to a byte/bit range.
// NOTE(eddyb) there's an unenforced invariant that this contains
// only `Field`s, and not into `enum` variants or `union`s.
// FIXME(eddyb) support this for `enum`s by either using DWARF's
///
/// This can only contain `PlaceElem::Field`.
// FIXME support this for `enum`s by either using DWARF's
// more advanced control-flow features (unsupported by LLVM?)
// to match on the discriminant, or by using custom type debuginfo
// with non-overlapping variants for the composite variable.
pub projection: Vec<PlaceElem<'tcx>>,

/// Where the data for this fragment can be found.
// NOTE(eddyb) There's an unenforced invariant that this `Place` is
// contains no indexing (with a non-constant index).
/// This `Place` only contains projection which satisfy `can_use_in_debuginfo`.
pub contents: Place<'tcx>,
}

Expand Down Expand Up @@ -1515,6 +1514,17 @@ impl<V, T> ProjectionElem<V, T> {
pub fn is_field_to(&self, f: Field) -> bool {
matches!(*self, Self::Field(x, _) if x == f)
}

/// Returns `true` if this is accepted inside `VarDebugInfoContents::Place`.
pub fn can_use_in_debuginfo(&self) -> bool {
match self {
Self::Deref | Self::Downcast(_, _) | Self::Field(_, _) => true,
Self::ConstantIndex { .. }
| Self::Index(_)
| Self::OpaqueCast(_)
| Self::Subslice { .. } => false,
}
}
}

/// Alias for projections as they appear in `UserTypeProjection`, where we
Expand Down