diff --git a/fuzz/fuzz_targets/fastalloc_checker.rs b/fuzz/fuzz_targets/fastalloc_checker.rs index b099c27a..b14078a7 100644 --- a/fuzz/fuzz_targets/fastalloc_checker.rs +++ b/fuzz/fuzz_targets/fastalloc_checker.rs @@ -41,5 +41,6 @@ fuzz_target!(|testcase: TestCase| { let mut checker = Checker::new(&func, &env); checker.prepare(&out); + checker.init_debug_locations(&out); checker.run().expect("checker failed"); }); diff --git a/fuzz/fuzz_targets/ion_checker.rs b/fuzz/fuzz_targets/ion_checker.rs index af41b55e..1ee2bcd3 100644 --- a/fuzz/fuzz_targets/ion_checker.rs +++ b/fuzz/fuzz_targets/ion_checker.rs @@ -48,6 +48,7 @@ fuzz_target!(|testcase: TestCase| { let mut checker = Checker::new(&func, &env); checker.prepare(&ctx.borrow().output); + checker.init_debug_locations(&ctx.borrow().output); checker.run().expect("checker failed"); }); }); diff --git a/src/checker.rs b/src/checker.rs index d67dc2da..832d0699 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -98,7 +98,7 @@ use crate::{ Allocation, AllocationKind, Block, Edit, Function, FxHashMap, FxHashSet, Inst, InstOrEdit, InstPosition, MachineEnv, Operand, OperandConstraint, OperandKind, OperandPos, Output, PReg, - PRegSet, VReg, + PRegSet, ProgPoint, VReg, }; use alloc::vec::Vec; use alloc::{format, vec}; @@ -110,11 +110,11 @@ use smallvec::{smallvec, SmallVec}; /// A set of errors detected by the regalloc checker. #[derive(Clone, Debug)] pub struct CheckerErrors { - errors: Vec, + pub errors: Vec, } /// A single error detected by the regalloc checker. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum CheckerError { MissingAllocation { inst: Inst, @@ -166,6 +166,13 @@ pub enum CheckerError { into: Allocation, from: Allocation, }, + ExpectedValueForDebug { + point: ProgPoint, + alloc: Allocation, + vreg: VReg, + found: CheckerValue, + label: u32, + }, } /// Abstract state for an allocation. @@ -174,7 +181,7 @@ pub enum CheckerError { /// universe-set as top and empty set as bottom lattice element. The /// meet-function is thus set intersection. #[derive(Clone, Debug, PartialEq, Eq)] -enum CheckerValue { +pub enum CheckerValue { /// The lattice top-value: this value could be equivalent to any /// vreg (i.e., the universe set). Universe, @@ -690,6 +697,128 @@ pub(crate) enum CheckerInst { }, } +#[derive(Debug)] +struct DebugLocationEntry { + vreg: VReg, + alloc: Allocation, + label: u32, +} + +#[derive(Debug)] +struct DebugLocations { + expected_vreg_locations: FxHashMap<(VReg, (ProgPoint, ProgPoint), u32), Allocation>, +} + +impl DebugLocations { + fn ranges_overlaps( + (start_point, end_point): (ProgPoint, ProgPoint), + (start_inst, end_inst): (Inst, Inst), + ) -> Option<(ProgPoint, ProgPoint)> { + if end_inst <= start_point.inst() || start_inst >= end_point.inst() { + None + } else { + let point0 = if start_point.inst() >= start_inst { + start_point + } else { + ProgPoint::before(start_inst) + }; + let point1 = if end_point.inst() < end_inst { + end_point + } else { + ProgPoint::before(end_inst) + }; + Some((point0, point1)) + } + } + + fn new(f: &F, output: &Output) -> Self { + let mut expected_vreg_locations = FxHashMap::default(); + for (label, start_point, end_point, alloc) in &output.debug_locations { + for (vreg, start_inst, end_inst, in_label) in f.debug_value_labels() { + if in_label != label { + continue; + } + if let Some(range) = + Self::ranges_overlaps((*start_point, *end_point), (*start_inst, *end_inst)) + { + expected_vreg_locations.insert((*vreg, range, *label), *alloc); + } + } + } + Self { + expected_vreg_locations, + } + } + + fn points_covers_inst( + &self, + inst: Inst, + start_point: ProgPoint, + end_point: ProgPoint, + ) -> (bool, bool) { + let start_inst = start_point.inst(); + let end_inst = end_point.inst(); + if inst > start_inst && inst < end_inst { + return (true, true); + } + if inst == start_inst && start_point.pos() == InstPosition::Before { + return (true, true); + } + // Don't check for the case where inst == start and pos == after + // because it may be edit instructions after inst that are responsible + // for moving the vreg into the expected allocation. + if inst == end_inst && end_point.pos() == InstPosition::After { + return (true, false); + } + (false, false) + } + + fn entries_covering(&self, inst: Inst) -> Vec<(bool, bool, DebugLocationEntry)> { + let mut entries = vec![]; + for entry in self.expected_vreg_locations.keys().copied() { + let (vreg, (start_point, end_point), label) = entry; + let (before, after) = self.points_covers_inst(inst, start_point, end_point); + if before || after { + entries.push(( + before, + after, + DebugLocationEntry { + vreg, + alloc: self.expected_vreg_locations[&entry], + label, + }, + )); + } + } + entries + } + + fn check_locations_covering_inst( + &self, + inst: Inst, + pos: InstPosition, + state: &CheckerState, + errors: &mut Vec, + ) { + for (before, after, entry) in self.entries_covering(inst) { + if before && pos == InstPosition::Before || pos == InstPosition::After && after { + let default_val = Default::default(); + let val = state.get_value(&entry.alloc).unwrap_or(&default_val); + match val { + CheckerValue::VRegs(vregs) if vregs.contains(&entry.vreg) => (), + _ => errors.push(CheckerError::ExpectedValueForDebug { + point: ProgPoint::new(inst, pos), + alloc: entry.alloc, + vreg: entry.vreg, + found: val.clone(), + label: entry.label, + }), + }; + } + } + } +} + #[derive(Debug)] pub struct Checker<'a, F: Function> { f: &'a F, @@ -698,6 +827,7 @@ pub struct Checker<'a, F: Function> { edge_insts: FxHashMap<(Block, Block), Vec>, machine_env: &'a MachineEnv, stack_pregs: PRegSet, + debug_locations: Option, } impl<'a, F: Function> Checker<'a, F> { @@ -733,6 +863,7 @@ impl<'a, F: Function> Checker<'a, F> { edge_insts, machine_env, stack_pregs, + debug_locations: None, } } @@ -756,6 +887,10 @@ impl<'a, F: Function> Checker<'a, F> { } } + pub fn init_debug_locations(&mut self, out: &Output) { + self.debug_locations = Some(DebugLocations::new(self.f, out)); + } + /// For each original instruction, create an `Op`. fn handle_inst(&mut self, block: Block, inst: Inst, out: &Output) { // Skip normal checks if this is a branch: the blockparams do @@ -888,6 +1023,21 @@ impl<'a, F: Function> Checker<'a, F> { for (block, input) in &self.bb_in { let mut state = input.clone(); for inst in self.bb_insts.get(block).unwrap() { + let orig_inst = match inst { + CheckerInst::Op { inst, .. } => Some(inst), + _ => None, + }; + + if let (Some(debug_locations), Some(orig_inst)) = (&self.debug_locations, orig_inst) + { + debug_locations.check_locations_covering_inst( + *orig_inst, + InstPosition::Before, + &state, + &mut errors, + ); + } + if let Err(e) = state.check(InstPosition::Before, inst, self) { trace!("Checker error: {:?}", e); errors.push(e); @@ -897,6 +1047,16 @@ impl<'a, F: Function> Checker<'a, F> { trace!("Checker error: {:?}", e); errors.push(e); } + + if let (Some(debug_locations), Some(orig_inst)) = (&self.debug_locations, orig_inst) + { + debug_locations.check_locations_covering_inst( + *orig_inst, + InstPosition::After, + &state, + &mut errors, + ); + } } } diff --git a/src/fastalloc/mod.rs b/src/fastalloc/mod.rs index c8d41f6c..514923aa 100644 --- a/src/fastalloc/mod.rs +++ b/src/fastalloc/mod.rs @@ -1209,11 +1209,35 @@ impl<'a, F: Function> Env<'a, F> { Ok(()) } + fn point_range_intersect( + &self, + (start_inst, end_inst): (Inst, Inst), + (point_start, point_end): (ProgPoint, ProgPoint), + ) -> Option<(ProgPoint, ProgPoint)> { + if end_inst <= point_start.inst() || start_inst >= point_end.inst() { + None + } else { + let point0 = if point_start.inst() >= start_inst { + point_start + } else { + ProgPoint::new(start_inst, InstPosition::Before) + }; + let point1 = if point_end.inst() < end_inst { + point_end + } else { + ProgPoint::new(end_inst, InstPosition::Before) + }; + Some((point0, point1)) + } + } + fn build_debug_info(&mut self) { trace!("Building debug location info"); for &(vreg, start, end, label) in self.func.debug_value_labels() { let (point_start, point_end, alloc) = self.vreg_to_live_inst_range[vreg.vreg()]; - if point_start.inst() <= start && end <= point_end.inst().next() { + if let Some((point_start, point_end)) = + self.point_range_intersect((start, end), (point_start, point_end)) + { self.debug_locations .push((label, point_start, point_end, alloc)); } diff --git a/src/fastalloc/tests.rs b/src/fastalloc/tests.rs index 41be3771..7da3a74e 100644 --- a/src/fastalloc/tests.rs +++ b/src/fastalloc/tests.rs @@ -1,8 +1,9 @@ +use crate::checker::{Checker, CheckerError, CheckerValue}; use crate::OperandConstraint::{self, *}; use crate::OperandKind::{self, *}; use crate::{ - run, Algorithm, Allocation, Block, Function, Inst, InstRange, MachineEnv, Operand, OperandPos, - PReg, PRegSet, ProgPoint, RegClass, RegallocOptions, VReg, + run, Algorithm, Allocation, Block, Function, FxHashSet, Inst, InstRange, MachineEnv, Operand, + OperandPos, PReg, PRegSet, ProgPoint, RegClass, RegallocOptions, VReg, }; use alloc::vec; use alloc::vec::Vec; @@ -102,10 +103,118 @@ fn test_debug_locations2() { ); assert_eq!(result.debug_locations[1].0, 23); assert_eq!(result.debug_locations[1].1, ProgPoint::after(i(2))); - assert_eq!(result.debug_locations[1].2, ProgPoint::after(i(4))); + assert_eq!(result.debug_locations[1].2, ProgPoint::before(i(3))); assert!(matches!(result.debug_locations[1].3.as_stack(), Some(_))); } +#[test] +fn test_debug_locations_check1() { + let mach_env = mach_env(10); + let mut options = RegallocOptions::default(); + options.validate_ssa = true; + options.algorithm = Algorithm::Fastalloc; + let mut f = RealFunction::new(vec![BlockBuildInfo { + insts: vec![ + /* 0. */ vec![op(Def, 0, FixedReg(p(0)))], + /* 1. */ + vec![ + op(Def, 1, FixedReg(p(0))), + op(Use, 0, FixedReg(p(0))), + op(Use, 0, Reg), + ], + /* 2. */ + vec![ + op(Def, 2, FixedReg(p(8))), + op(Use, 0, FixedReg(p(2))), + op(Use, 1, FixedReg(p(0))), + ], + /* 3. */ vec![op(Def, 3, FixedReg(p(9))), op(Use, 0, FixedReg(p(9)))], + ], + }]); + f.debug_value_labels = vec![ + (v(0), i(0), i(4), 32), + (v(2), i(2), i(4), 70), + (v(2), i(2), i(4), 71), + (v(3), i(3), i(4), 34), + ]; + let mut result = run(&f, &mach_env, &options).unwrap(); + /* + The correct debug_locations output + vec![ + ( + 32, + ProgPoint::after(i(0)), + ProgPoint::after(i(3)), + alloc(p(9)) + ), + ( + 34, + ProgPoint::after(i(3)), + ProgPoint::before(i(4)), + alloc(p(9)) + ), + ( + 70, + ProgPoint::after(i(2)), + ProgPoint::before(i(3)), + alloc(p(8)) + ), + ( + 71, + ProgPoint::after(i(2)), + ProgPoint::before(i(3)), + alloc(p(8)) + ), + ] + */ + result.debug_locations.pop(); + /* + Replacing the last entry with this should result in a checker error + */ + result.debug_locations.push(( + 71, + ProgPoint::after(i(2)), + ProgPoint::before(i(4)), + alloc(p(9)), + )); + let mut checker = Checker::new(&f, &mach_env); + checker.prepare(&result); + checker.init_debug_locations(&result); + let checker_result = checker.run(); + assert!(checker_result.is_err()); + let mut found_0 = FxHashSet::default(); + let mut found_3 = FxHashSet::default(); + found_0.insert(v(0)); + found_3.insert(v(3)); + assert_eq!( + checker_result.unwrap_err().errors, + vec![ + // Before inst 3, we're expecting v2 to be in p9, + // because the entry in debug_locations labeled 71 + // indicates that it should. + // Instead, we found v0 + CheckerError::ExpectedValueForDebug { + point: ProgPoint::before(i(3)), + alloc: alloc(p(9)), + vreg: v(2), + found: CheckerValue::VRegs(found_0), + label: 71, + }, + // After inst 3, we're expecting v2 to be in p9, + // because the entry in debug_locations labeled 71 + // indicates that it should. + // Instead, we found v3 + CheckerError::ExpectedValueForDebug { + point: ProgPoint::after(i(3)), + alloc: alloc(p(9)), + vreg: v(2), + found: CheckerValue::VRegs(found_3), + label: 71, + }, + ] + ); +} + impl RealFunction { fn new(blocks: Vec) -> Self { assert!(blocks.len() <= 2, "Just for testing purposes"); diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index 69ce7262..ac9d3a58 100644 --- a/src/fuzzing/func.rs +++ b/src/fuzzing/func.rs @@ -4,8 +4,8 @@ */ use crate::{ - domtree, postorder, Allocation, Block, Function, Inst, InstRange, MachineEnv, Operand, - OperandConstraint, OperandKind, OperandPos, PReg, PRegSet, RegClass, VReg, + domtree, postorder, Allocation, Block, Function, FxHashMap, Inst, InstRange, MachineEnv, + Operand, OperandConstraint, OperandKind, OperandPos, PReg, PRegSet, RegClass, VReg, }; use alloc::vec::Vec; @@ -300,6 +300,7 @@ impl Func { builder.add_block(); } let num_blocks = builder.f.blocks.len(); + let mut label_free_range_start = FxHashMap::default(); // Generate a CFG. Create a "spine" of either single blocks, // with links to the next; or fork patterns, with the left @@ -358,20 +359,20 @@ impl Func { } if bool::arbitrary(u)? { let assumed_end_inst = 10 * num_blocks; - let mut start = u.int_in_range::(0..=assumed_end_inst)?; for _ in 0..10 { - if start >= assumed_end_inst { - break; - } - let end = u.int_in_range::(start..=assumed_end_inst)?; let label = u.int_in_range::(0..=100)?; + let free_range_start = label_free_range_start.entry(label).or_insert(0); + if *free_range_start >= assumed_end_inst { + continue; + } + let end = u.int_in_range(*free_range_start..=assumed_end_inst)?; builder.f.debug_value_labels.push(( vreg, - Inst::new(start), + Inst::new(*free_range_start), Inst::new(end), label, )); - start = end; + *free_range_start = end; } } } @@ -619,6 +620,7 @@ impl core::fmt::Debug for Func { } } } + write!(f, " debug_value_labels: {:?}", self.debug_value_labels)?; write!(f, "}}\n")?; Ok(()) }