From ef6c8f32263fc78f38af5f68018f7359ae570467 Mon Sep 17 00:00:00 2001
From: Chris Fallin <chris@cfallin.org>
Date: Sat, 4 Dec 2021 23:30:30 -0800
Subject: [PATCH] Fix fuzzbug: add checker metadata for new vreg on
 multi-fixed-reg fixup move.

When an instruction uses the same vreg constrained to multiple different
fixed registers, the allocator converts all but one of the fixed
constraints to `Any` and then records a special fixup move that copies
the value to the other fixed registers just before the instruction. This
allows the allocator to maintain the invariant that a value lives in
only one place at a time throughout most of its logic, and constrains
the complexity-fallout of this corner case to just a special last-minute
edit.

Unfortunately some recent CPU time thrown at the fuzzer has uncovered
a subtle interaction with the redundant move eliminator that confuses
the checker.

Specifically, when the correct value is *already* in the second
constrained fixed reg, because of an unrelated other move (e.g. because
of a blockparam or other vreg moved from the original), the redundant
move eliminator can delete the fixup move without telling the checker
that it has done so.

Such an optimization is perfectly valid, and the generated code is
correct; but the checker thinks that some other vreg (the one that was
copied from the original) is in the second preg, and panics.

The fix is to use the mechanism that indicates "this move defines a new
vreg" (emitting a `defalloc` checker-instruction) to force the checker
to understand that after the fixup move, the given preg actually
contains the appropriate vreg.
---
 src/ion/data_structures.rs | 2 +-
 src/ion/liveranges.rs      | 3 ++-
 src/ion/moves.rs           | 9 +++++----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs
index 711755e..748d846 100644
--- a/src/ion/data_structures.rs
+++ b/src/ion/data_structures.rs
@@ -331,7 +331,7 @@ pub struct Env<'a, F: Function> {
     // was to the approprate PReg.
     //
     // (progpoint, copy-from-preg, copy-to-preg, to-slot)
-    pub multi_fixed_reg_fixups: Vec<(ProgPoint, PRegIndex, PRegIndex, usize)>,
+    pub multi_fixed_reg_fixups: Vec<(ProgPoint, PRegIndex, PRegIndex, VRegIndex, usize)>,
 
     pub inserted_moves: Vec<InsertedMove>,
 
diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs
index 4637318..de100c9 100644
--- a/src/ion/liveranges.rs
+++ b/src/ion/liveranges.rs
@@ -1167,6 +1167,7 @@ impl<'a, F: Function> Env<'a, F> {
                     ProgPoint,
                     PRegIndex,
                     PRegIndex,
+                    VRegIndex,
                     usize,
                 )>| {
                     if last_point.is_some() && Some(pos) != last_point {
@@ -1189,7 +1190,7 @@ impl<'a, F: Function> Env<'a, F> {
                             let orig_preg = first_preg[idx];
                             if orig_preg != preg_idx {
                                 log::trace!(" -> duplicate; switching to constraint Reg");
-                                fixups.push((pos, orig_preg, preg_idx, slot));
+                                fixups.push((pos, orig_preg, preg_idx, vreg_idx, slot));
                                 *op = Operand::new(
                                     op.vreg(),
                                     OperandConstraint::Reg,
diff --git a/src/ion/moves.rs b/src/ion/moves.rs
index f382821..2b562f7 100644
--- a/src/ion/moves.rs
+++ b/src/ion/moves.rs
@@ -685,21 +685,22 @@ impl<'a, F: Function> Env<'a, F> {
         }
 
         // Handle multi-fixed-reg constraints by copying.
-        for (progpoint, from_preg, to_preg, slot) in
+        for (progpoint, from_preg, to_preg, to_vreg, slot) in
             std::mem::replace(&mut self.multi_fixed_reg_fixups, vec![])
         {
             log::trace!(
-                "multi-fixed-move constraint at {:?} from p{} to p{}",
+                "multi-fixed-move constraint at {:?} from p{} to p{} for v{}",
                 progpoint,
                 from_preg.index(),
-                to_preg.index()
+                to_preg.index(),
+                to_vreg.index(),
             );
             self.insert_move(
                 progpoint,
                 InsertMovePrio::MultiFixedReg,
                 Allocation::reg(self.pregs[from_preg.index()].reg),
                 Allocation::reg(self.pregs[to_preg.index()].reg),
-                None,
+                Some(self.vreg_regs[to_vreg.index()]),
             );
             self.set_alloc(
                 progpoint.inst(),