fix: Prevent hoisting binary instructions which can overflow

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -386,12 +386,25 @@
             // These can fail.
             Constrain(..) | RangeCheck { .. } => true,
 
             // This should never be side-effectful
             MakeArray { .. } => false,
 
+            // Some binary math can overflow or underflow
+            Binary(binary) => match binary.operator {
+                BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul | BinaryOp::Div | BinaryOp::Mod => {
+                    true
+                }
+                BinaryOp::Eq
+                | BinaryOp::Lt
+                | BinaryOp::And
+                | BinaryOp::Or
+                | BinaryOp::Xor
+                | BinaryOp::Shl
+                | BinaryOp::Shr => false,
+            },
+
             // These can have different behavior depending on the EnableSideEffectsIf context.
-            Binary(_)
-            | Cast(_, _)
+            Cast(_, _)
             | Not(_)
             | Truncate { .. }
             | IfElse { .. }
@@ -435,7 +448,7 @@
             // We can deduplicate these instructions if we know the predicate is also the same.
             Constrain(..) | RangeCheck { .. } => deduplicate_with_predicate,
 
             // This should never be side-effectful
             MakeArray { .. } => true,
 
             // These can have different behavior depending on the EnableSideEffectsIf context.

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

@@ -1187,15 +1187,15 @@ mod test {
                 v2 = lt u32 1000, v0
                 jmpif v2 then: b1, else: b2
               b1():
-                v4 = add v0, u32 1
+                v4 = shl v0, u32 1
                 v5 = lt v0, v4
                 constrain v5 == u1 1
                 jmp b2()
               b2():
                 v7 = lt u32 1000, v0
                 jmpif v7 then: b3, else: b4
               b3():
-                v8 = add v0, u32 1
+                v8 = shl v0, u32 1
                 v9 = lt v0, v8
                 constrain v9 == u1 1
                 jmp b4()
@@ -1213,10 +1213,10 @@ mod test {
             brillig(inline) fn main f0 {
               b0(v0: u32):
                 v2 = lt u32 1000, v0
-                v4 = add v0, u32 1
+                v4 = shl v0, u32 1
                 jmpif v2 then: b1, else: b2
               b1():
-                v5 = add v0, u32 1
+                v5 = shl v0, u32 1
                 v6 = lt v0, v5
                 constrain v6 == u1 1
                 jmp b2()
@@ -1457,6 +1457,32 @@ mod test {
         assert_normalized_ssa_equals(ssa, src);
     }
 
+    #[test]
+    fn does_not_hoist_sub_to_common_ancestor() {
+        let src = "
+            acir(inline) fn main f0 {
+              b0(v0: u32):
+                v2 = eq v0, u32 0
+                jmpif v2 then: b4, else: b1
+              b4():
+                v5 = sub v0, u32 1
+                jmp b5()
+              b5():
+                return
+              b1():
+                jmpif v0 then: b3, else: b2
+              b3():
+                v4 = sub v0, u32 1 // We can't hoist this because v0 is zero here and it will lead to an underflow
+                jmp b5()
+              b2():
+                jmp b5()
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+        let ssa = ssa.fold_constants_using_constraints();
+        assert_normalized_ssa_equals(ssa, src);
+    }
+
     #[test]
     fn deduplicates_side_effecting_intrinsics() {
         let src = "