T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove macros from verifier; fixes #1248

Browse Source 
This removes `report!`, `fatal!`, and `nonfatal!` from the verifier code and replaces them with methods on `VerifierErrors`. In order to maintain similar ease-of-use, `VerifierError` is expanded with several `From` implementations that convert a tuple to a verifier error.
This commit is contained in:
Andrew Brown
2019-11-20 14:17:04 -08:00
parent e1e4e0bfc6
commit 3e5f039333
5 changed files with 585 additions and 576 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
cranelift/codegen/src/verifier/cssa.rs
View File

@@ -65,13 +65,13 @@ impl<'a> CssaVerifier<'a> {
for (idx, &val) in values.iter().enumerate() { for (idx, &val) in values.iter().enumerate() {
if !self.func.dfg.value_is_valid(val) { if !self.func.dfg.value_is_valid(val) {
return fatal!(errors, val, "Invalid value in {}", vreg); return errors.fatal((val, format!("Invalid value in {}", vreg)));
} }
if !self.func.dfg.value_is_attached(val) { if !self.func.dfg.value_is_attached(val) {
return fatal!(errors, val, "Detached value in {}", vreg); return errors.fatal((val, format!("Detached value in {}", vreg)));
} }
if self.liveness.get(val).is_none() { if self.liveness.get(val).is_none() {
return fatal!(errors, val, "Value in {} has no live range", vreg); return errors.fatal((val, format!("Value in {} has no live range", vreg)));
}; };
// Check topological ordering with the previous values in the virtual register. // Check topological ordering with the previous values in the virtual register.
@@ -82,29 +82,31 @@ impl<'a> CssaVerifier<'a> {
let prev_ebb = self.func.layout.pp_ebb(prev_def); let prev_ebb = self.func.layout.pp_ebb(prev_def);
if prev_def == def { if prev_def == def {
return fatal!( return errors.fatal((
errors,
val, val,
format!(
"Values {} and {} in {} = {} defined at the same program point", "Values {} and {} in {} = {} defined at the same program point",
prev_val, prev_val,
val, val,
vreg, vreg,
DisplayList(values) DisplayList(values)
); ),
));
} }
// Enforce topological ordering of defs in the virtual register. // Enforce topological ordering of defs in the virtual register.
if self.preorder.dominates(def_ebb, prev_ebb) if self.preorder.dominates(def_ebb, prev_ebb)
&& self.domtree.dominates(def, prev_def, &self.func.layout) && self.domtree.dominates(def, prev_def, &self.func.layout)
{ {
return fatal!( return errors.fatal((
errors,
val, val,
format!(
"Value in {} = {} def dominates previous {}", "Value in {} = {} def dominates previous {}",
vreg, vreg,
DisplayList(values), DisplayList(values),
prev_val prev_val
); ),
));
} }
} }
@@ -119,14 +121,15 @@ impl<'a> CssaVerifier<'a> {
&& self.domtree.dominates(prev_def, def, &self.func.layout) && self.domtree.dominates(prev_def, def, &self.func.layout)
{ {
if self.liveness[prev_val].overlaps_def(def, def_ebb, &self.func.layout) { if self.liveness[prev_val].overlaps_def(def, def_ebb, &self.func.layout) {
return fatal!( return errors.fatal((
errors,
val, val,
format!(
"Value def in {} = {} interferes with {}", "Value def in {} = {} interferes with {}",
vreg, vreg,
DisplayList(values), DisplayList(values),
prev_val prev_val
); ),
));
} else { } else {
break; break;
} }
@@ -152,13 +155,13 @@ impl<'a> CssaVerifier<'a> {
for (&ebb_param, &pred_arg) in ebb_params.iter().zip(pred_args) { for (&ebb_param, &pred_arg) in ebb_params.iter().zip(pred_args) {
if !self.virtregs.same_class(ebb_param, pred_arg) { if !self.virtregs.same_class(ebb_param, pred_arg) {
return fatal!( return errors.fatal((
errors,
pred, pred,
format!(
"{} and {} must be in the same virtual register", "{} and {} must be in the same virtual register",
ebb_param, ebb_param, pred_arg
pred_arg ),
); ));
} }
} }
} }

24
cranelift/codegen/src/verifier/flags.rs
View File

@@ -67,13 +67,10 @@ impl<'a> FlagsVerifier<'a> {
} }
} }
Some(old) if old != value => { Some(old) if old != value => {
return fatal!( return errors.fatal((
errors,
ebb, ebb,
"conflicting live-in CPU flags: {} and {}", format!("conflicting live-in CPU flags: {} and {}", old, value),
old, ));
value
);
} }
x => assert_eq!(x, Some(value)), x => assert_eq!(x, Some(value)),
} }
@@ -104,7 +101,9 @@ impl<'a> FlagsVerifier<'a> {
// We've reached the def of `live_flags`, so it is no longer live above. // We've reached the def of `live_flags`, so it is no longer live above.
live_val = None; live_val = None;
} else if self.func.dfg.value_type(res).is_flags() { } else if self.func.dfg.value_type(res).is_flags() {
return fatal!(errors, inst, "{} clobbers live CPU flags in {}", res, live); errors
.report((inst, format!("{} clobbers live CPU flags in {}", res, live)));
return Err(());
} }
} }
@@ -116,7 +115,11 @@ impl<'a> FlagsVerifier<'a> {
.map_or(false, |c| c.clobbers_flags) .map_or(false, |c| c.clobbers_flags)
&& live_val.is_some() && live_val.is_some()
{ {
return fatal!(errors, inst, "encoding clobbers live CPU flags in {}", live); errors.report((
inst,
format!("encoding clobbers live CPU flags in {}", live),
));
return Err(());
} }
} }
@@ -164,7 +167,10 @@ fn merge(
) -> VerifierStepResult<()> { ) -> VerifierStepResult<()> {
if let Some(va) = *a { if let Some(va) = *a {
if b != va { if b != va {
return fatal!(errors, inst, "conflicting live CPU flags: {} and {}", va, b); return errors.fatal((
inst,
format!("conflicting live CPU flags: {} and {}", va, b),
));
} }
} else { } else {
*a = Some(b); *a = Some(b);

93
cranelift/codegen/src/verifier/liveness.rs
View File

@@ -54,7 +54,9 @@ impl<'a> LivenessVerifier<'a> {
for &val in self.func.dfg.ebb_params(ebb) { for &val in self.func.dfg.ebb_params(ebb) {
let lr = match self.liveness.get(val) { let lr = match self.liveness.get(val) {
Some(lr) => lr, Some(lr) => lr,
None => return fatal!(errors, ebb, "EBB arg {} has no live range", val), None => {
return errors.fatal((ebb, format!("EBB arg {} has no live range", val)))
}
}; };
self.check_lr(ebb.into(), val, lr, errors)?; self.check_lr(ebb.into(), val, lr, errors)?;
} }
@@ -72,30 +74,32 @@ impl<'a> LivenessVerifier<'a> {
for &val in self.func.dfg.inst_results(inst) { for &val in self.func.dfg.inst_results(inst) {
let lr = match self.liveness.get(val) { let lr = match self.liveness.get(val) {
Some(lr) => lr, Some(lr) => lr,
None => return fatal!(errors, inst, "{} has no live range", val), None => return errors.fatal((inst, format!("{} has no live range", val))),
}; };
self.check_lr(inst.into(), val, lr, errors)?; self.check_lr(inst.into(), val, lr, errors)?;
if encoding.is_legal() { if encoding.is_legal() {
// A legal instruction is not allowed to define ghost values. // A legal instruction is not allowed to define ghost values.
if lr.affinity.is_unassigned() { if lr.affinity.is_unassigned() {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"{} is a ghost value defined by a real [{}] instruction", "{} is a ghost value defined by a real [{}] instruction",
val, val,
self.isa.encoding_info().display(encoding) self.isa.encoding_info().display(encoding)
); ),
));
} }
} else if !lr.affinity.is_unassigned() { } else if !lr.affinity.is_unassigned() {
// A non-encoded instruction can only define ghost values. // A non-encoded instruction can only define ghost values.
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"{} is a real {} value defined by a ghost instruction", "{} is a real {} value defined by a ghost instruction",
val, val,
lr.affinity.display(&self.isa.register_info()) lr.affinity.display(&self.isa.register_info())
); ),
));
} }
} }
@@ -103,23 +107,24 @@ impl<'a> LivenessVerifier<'a> {
for &val in self.func.dfg.inst_args(inst) { for &val in self.func.dfg.inst_args(inst) {
let lr = match self.liveness.get(val) { let lr = match self.liveness.get(val) {
Some(lr) => lr, Some(lr) => lr,
None => return fatal!(errors, inst, "{} has no live range", val), None => return errors.fatal((inst, format!("{} has no live range", val))),
}; };
debug_assert!(self.func.layout.inst_ebb(inst).unwrap() == ebb); debug_assert!(self.func.layout.inst_ebb(inst).unwrap() == ebb);
if !lr.reaches_use(inst, ebb, &self.func.layout) { if !lr.reaches_use(inst, ebb, &self.func.layout) {
return fatal!(errors, inst, "{} is not live at this use", val); return errors.fatal((inst, format!("{} is not live at this use", val)));
} }
// A legal instruction is not allowed to depend on ghost values. // A legal instruction is not allowed to depend on ghost values.
if encoding.is_legal() && lr.affinity.is_unassigned() { if encoding.is_legal() && lr.affinity.is_unassigned() {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"{} is a ghost value used by a real [{}] instruction", "{} is a ghost value used by a real [{}] instruction",
val, val,
self.isa.encoding_info().display(encoding) self.isa.encoding_info().display(encoding),
); ),
));
} }
} }
} }
@@ -142,17 +147,14 @@ impl<'a> LivenessVerifier<'a> {
ExpandedProgramPoint::Inst(i) => i.into(), ExpandedProgramPoint::Inst(i) => i.into(),
}; };
if lr.def() != def { if lr.def() != def {
return fatal!( return errors.fatal((
errors,
loc, loc,
"Wrong live range def ({}) for {}", format!("Wrong live range def ({}) for {}", lr.def(), val),
lr.def(), ));
val
);
} }
if lr.is_dead() { if lr.is_dead() {
if !lr.is_local() { if !lr.is_local() {
return fatal!(errors, loc, "Dead live range {} should be local", val); return errors.fatal((loc, format!("Dead live range {} should be local", val)));
} else { } else {
return Ok(()); return Ok(());
} }
@@ -163,17 +165,14 @@ impl<'a> LivenessVerifier<'a> {
}; };
match lr.def_local_end().into() { match lr.def_local_end().into() {
ExpandedProgramPoint::Ebb(e) => { ExpandedProgramPoint::Ebb(e) => {
return fatal!( return errors.fatal((
errors,
loc, loc,
"Def local range for {} can't end at {}", format!("Def local range for {} can't end at {}", val, e),
val, ));
e
);
} }
ExpandedProgramPoint::Inst(i) => { ExpandedProgramPoint::Inst(i) => {
if self.func.layout.inst_ebb(i) != Some(def_ebb) { if self.func.layout.inst_ebb(i) != Some(def_ebb) {
return fatal!(errors, loc, "Def local end for {} in wrong ebb", val); return errors.fatal((loc, format!("Def local end for {} in wrong ebb", val)));
} }
} }
} }
@@ -181,25 +180,21 @@ impl<'a> LivenessVerifier<'a> {
// Now check the live-in intervals against the CFG. // Now check the live-in intervals against the CFG.
for (mut ebb, end) in lr.liveins() { for (mut ebb, end) in lr.liveins() {
if !l.is_ebb_inserted(ebb) { if !l.is_ebb_inserted(ebb) {
return fatal!( return errors.fatal((
errors,
loc, loc,
"{} livein at {} which is not in the layout", format!("{} livein at {} which is not in the layout", val, ebb),
val, ));
ebb
);
} }
let end_ebb = match l.inst_ebb(end) { let end_ebb = match l.inst_ebb(end) {
Some(e) => e, Some(e) => e,
None => { None => {
return fatal!( return errors.fatal((
errors,
loc, loc,
format!(
"{} livein for {} ends at {} which is not in the layout", "{} livein for {} ends at {} which is not in the layout",
val, val, ebb, end
ebb, ),
end ));
);
} }
}; };
@@ -208,13 +203,10 @@ impl<'a> LivenessVerifier<'a> {
// If `val` is live-in at `ebb`, it must be live at all the predecessors. // If `val` is live-in at `ebb`, it must be live at all the predecessors.
for BasicBlock { inst: pred, ebb } in self.cfg.pred_iter(ebb) { for BasicBlock { inst: pred, ebb } in self.cfg.pred_iter(ebb) {
if !lr.reaches_use(pred, ebb, &self.func.layout) { if !lr.reaches_use(pred, ebb, &self.func.layout) {
return fatal!( return errors.fatal((
errors,
pred, pred,
"{} is live in to {} but not live at predecessor", format!("{} is live in to {} but not live at predecessor", val, ebb),
val, ));
ebb
);
} }
} }
@@ -224,13 +216,10 @@ impl<'a> LivenessVerifier<'a> {
ebb = match l.next_ebb(ebb) { ebb = match l.next_ebb(ebb) {
Some(e) => e, Some(e) => e,
None => { None => {
return fatal!( return errors.fatal((
errors,
loc, loc,
"end of {} livein ({}) never reached", format!("end of {} livein ({}) never reached", val, end_ebb),
val, ));
end_ebb
);
} }
}; };
} }

92
cranelift/codegen/src/verifier/locations.rs
View File

@@ -104,14 +104,15 @@ impl<'a> LocationVerifier<'a> {
} }
// TODO: We could give a better error message here. // TODO: We could give a better error message here.
fatal!( errors.fatal((
errors,
inst, inst,
format!(
"{} constraints not satisfied in: {}\n{}", "{} constraints not satisfied in: {}\n{}",
self.encinfo.display(enc), self.encinfo.display(enc),
self.func.dfg.display_inst(inst, self.isa), self.func.dfg.display_inst(inst, self.isa),
self.func.display(self.isa) self.func.display(self.isa),
) ),
))
} }
/// Check that the result values produced by a ghost instruction are not assigned a value /// Check that the result values produced by a ghost instruction are not assigned a value
@@ -126,13 +127,14 @@ impl<'a> LocationVerifier<'a> {
for &res in results { for &res in results {
let loc = self.func.locations[res]; let loc = self.func.locations[res];
if loc.is_assigned() { if loc.is_assigned() {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"ghost result {} value must not have a location ({}).", "ghost result {} value must not have a location ({}).",
res, res,
loc.display(&self.reginfo) loc.display(&self.reginfo)
); ),
));
} }
} }
@@ -214,50 +216,51 @@ impl<'a> LocationVerifier<'a> {
ir::ArgumentLoc::Unassigned => {} ir::ArgumentLoc::Unassigned => {}
ir::ArgumentLoc::Reg(reg) => { ir::ArgumentLoc::Reg(reg) => {
if loc != ir::ValueLoc::Reg(reg) { if loc != ir::ValueLoc::Reg(reg) {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"ABI expects {} in {}, got {}", "ABI expects {} in {}, got {}",
value, value,
abi.location.display(&self.reginfo), abi.location.display(&self.reginfo),
loc.display(&self.reginfo) loc.display(&self.reginfo),
); ),
));
} }
} }
ir::ArgumentLoc::Stack(offset) => { ir::ArgumentLoc::Stack(offset) => {
if let ir::ValueLoc::Stack(ss) = loc { if let ir::ValueLoc::Stack(ss) = loc {
let slot = &self.func.stack_slots[ss]; let slot = &self.func.stack_slots[ss];
if slot.kind != want_kind { if slot.kind != want_kind {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"call argument {} should be in a {} slot, but {} is {}", "call argument {} should be in a {} slot, but {} is {}",
value, value, want_kind, ss, slot.kind
want_kind, ),
ss, ));
slot.kind
);
} }
if slot.offset.unwrap() != offset { if slot.offset.unwrap() != offset {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"ABI expects {} at stack offset {}, but {} is at {}", "ABI expects {} at stack offset {}, but {} is at {}",
value, value,
offset, offset,
ss, ss,
slot.offset.unwrap() slot.offset.unwrap()
); ),
));
} }
} else { } else {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"ABI expects {} at stack offset {}, got {}", "ABI expects {} at stack offset {}, got {}",
value, value,
offset, offset,
loc.display(&self.reginfo) loc.display(&self.reginfo)
); ),
));
} }
} }
} }
@@ -281,21 +284,23 @@ impl<'a> LocationVerifier<'a> {
if let Some(d) = divert.diversion(arg) { if let Some(d) = divert.diversion(arg) {
if d.to != src { if d.to != src {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"inconsistent with current diversion to {}", "inconsistent with current diversion to {}",
d.to.display(&self.reginfo) d.to.display(&self.reginfo)
); ),
));
} }
} else if self.func.locations[arg] != src { } else if self.func.locations[arg] != src {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"inconsistent with global location {} ({})", "inconsistent with global location {} ({})",
self.func.locations[arg].display(&self.reginfo), self.func.locations[arg].display(&self.reginfo),
self.func.dfg.display_inst(inst, None) self.func.dfg.display_inst(inst, None)
); ),
));
} }
divert.apply(&self.func.dfg[inst]); divert.apply(&self.func.dfg[inst]);
@@ -338,14 +343,15 @@ impl<'a> LocationVerifier<'a> {
val_to_remove.push(value) val_to_remove.push(value)
} }
} else if lr.is_livein(ebb, &self.func.layout) { } else if lr.is_livein(ebb, &self.func.layout) {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"SingleDest: {} is diverted to {} and live in to {}", "SingleDest: {} is diverted to {} and live in to {}",
value, value,
d.to.display(&self.reginfo), d.to.display(&self.reginfo),
ebb ebb,
); ),
));
} }
} }
if is_after_branch && unique_predecessor { if is_after_branch && unique_predecessor {
@@ -360,26 +366,28 @@ impl<'a> LocationVerifier<'a> {
let lr = &liveness[value]; let lr = &liveness[value];
if let Some(ebb) = ebb { if let Some(ebb) = ebb {
if lr.is_livein(ebb, &self.func.layout) { if lr.is_livein(ebb, &self.func.layout) {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"Table.default: {} is diverted to {} and live in to {}", "Table.default: {} is diverted to {} and live in to {}",
value, value,
d.to.display(&self.reginfo), d.to.display(&self.reginfo),
ebb ebb,
); ),
));
} }
} }
for ebb in self.func.jump_tables[jt].iter() { for ebb in self.func.jump_tables[jt].iter() {
if lr.is_livein(*ebb, &self.func.layout) { if lr.is_livein(*ebb, &self.func.layout) {
return fatal!( return errors.fatal((
errors,
inst, inst,
format!(
"Table.case: {} is diverted to {} and live in to {}", "Table.case: {} is diverted to {} and live in to {}",
value, value,
d.to.display(&self.reginfo), d.to.display(&self.reginfo),
ebb ebb,
); ),
));
} }
} }
} }

763
cranelift/codegen/src/verifier/mod.rs
View File

File diff suppressed because it is too large Load Diff