-
Notifications
You must be signed in to change notification settings - Fork 13.4k
validate basic sanity for TerminatorKind #72810
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,11 @@ | |
use super::{MirPass, MirSource}; | ||
use rustc_middle::mir::visit::Visitor; | ||
use rustc_middle::{ | ||
mir::{Body, Location, Operand, Rvalue, Statement, StatementKind}, | ||
ty::{ParamEnv, TyCtxt}, | ||
mir::{ | ||
BasicBlock, Body, Location, Operand, Rvalue, Statement, StatementKind, Terminator, | ||
TerminatorKind, | ||
}, | ||
ty::{self, ParamEnv, TyCtxt}, | ||
}; | ||
use rustc_span::{def_id::DefId, Span, DUMMY_SP}; | ||
|
||
|
@@ -38,6 +41,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { | |
&format!("broken MIR in {:?} ({}): {}", self.def_id, self.when, msg.as_ref()), | ||
); | ||
} | ||
|
||
fn check_bb(&self, span: Span, bb: BasicBlock) { | ||
if self.body.basic_blocks().get(bb).is_none() { | ||
self.fail(span, format!("encountered jump to invalid basic block {:?}", bb)) | ||
} | ||
} | ||
} | ||
|
||
impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | ||
|
@@ -77,4 +86,94 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
} | ||
} | ||
} | ||
|
||
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _location: Location) { | ||
match &terminator.kind { | ||
TerminatorKind::Goto { target } => { | ||
self.check_bb(terminator.source_info.span, *target); | ||
} | ||
TerminatorKind::SwitchInt { targets, .. } => { | ||
if targets.is_empty() { | ||
self.fail( | ||
terminator.source_info.span, | ||
"encountered `SwitchInt` terminator with no target to jump to", | ||
); | ||
} | ||
for target in targets { | ||
self.check_bb(terminator.source_info.span, *target); | ||
} | ||
} | ||
TerminatorKind::Drop { target, unwind, .. } => { | ||
self.check_bb(terminator.source_info.span, *target); | ||
if let Some(unwind) = unwind { | ||
self.check_bb(terminator.source_info.span, *unwind); | ||
} | ||
} | ||
TerminatorKind::DropAndReplace { target, unwind, .. } => { | ||
self.check_bb(terminator.source_info.span, *target); | ||
if let Some(unwind) = unwind { | ||
self.check_bb(terminator.source_info.span, *unwind); | ||
} | ||
} | ||
TerminatorKind::Call { func, destination, cleanup, .. } => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, now I wonder if Miri would have caught that (for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It probably would have, but I haven't yet figured out how to use a locally built miri with external code (like with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I usually do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I've only ever used the script in a standalone clone... yeah it probably doesn't work in the rustc folder layout.
How do you run them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that's what I tried to run, but it enters unreachable code while decoding or encoding something. Not really sure what's happening, but I'll do more digging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds like an incremental problem or |
||
let func_ty = func.ty(&self.body.local_decls, self.tcx); | ||
match func_ty.kind { | ||
ty::FnPtr(..) | ty::FnDef(..) => {} | ||
_ => self.fail( | ||
terminator.source_info.span, | ||
format!("encountered non-callable type {} in `Call` terminator", func_ty), | ||
), | ||
} | ||
if let Some((_, target)) = destination { | ||
self.check_bb(terminator.source_info.span, *target); | ||
} | ||
if let Some(cleanup) = cleanup { | ||
self.check_bb(terminator.source_info.span, *cleanup); | ||
} | ||
} | ||
TerminatorKind::Assert { cond, target, cleanup, .. } => { | ||
let cond_ty = cond.ty(&self.body.local_decls, self.tcx); | ||
if cond_ty != self.tcx.types.bool { | ||
self.fail( | ||
terminator.source_info.span, | ||
format!( | ||
"encountered non-boolean condition of type {} in `Assert` terminator", | ||
cond_ty | ||
), | ||
); | ||
} | ||
self.check_bb(terminator.source_info.span, *target); | ||
if let Some(cleanup) = cleanup { | ||
self.check_bb(terminator.source_info.span, *cleanup); | ||
} | ||
} | ||
TerminatorKind::Yield { resume, drop, .. } => { | ||
self.check_bb(terminator.source_info.span, *resume); | ||
if let Some(drop) = drop { | ||
self.check_bb(terminator.source_info.span, *drop); | ||
} | ||
} | ||
TerminatorKind::FalseEdges { real_target, imaginary_target } => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw, does anyone know why this is named in plural? Shouldn't it be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that it may have been accurate in the past. Feel free to change it. |
||
self.check_bb(terminator.source_info.span, *real_target); | ||
self.check_bb(terminator.source_info.span, *imaginary_target); | ||
} | ||
TerminatorKind::FalseUnwind { real_target, unwind } => { | ||
self.check_bb(terminator.source_info.span, *real_target); | ||
if let Some(unwind) = unwind { | ||
self.check_bb(terminator.source_info.span, *unwind); | ||
} | ||
} | ||
TerminatorKind::InlineAsm { destination, .. } => { | ||
if let Some(destination) = destination { | ||
self.check_bb(terminator.source_info.span, *destination); | ||
} | ||
} | ||
// Nothing to validate for these. | ||
TerminatorKind::Resume | ||
| TerminatorKind::Abort | ||
| TerminatorKind::Return | ||
| TerminatorKind::Unreachable | ||
| TerminatorKind::GeneratorDrop => {} | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.