Fix AddMod and MulMod (#1018)

Signed-off-by: Lucas Steuernagel <lucas.tnagel@gmail.com>

Signed-off-by: Lucas Steuernagel <lucas.tnagel@gmail.com>

src/codegen/expression.rs

@@ -1717,6 +1717,58 @@ fn expr_builtin(
 
             Expression::Builtin(*loc, tys.to_vec(), builtin.into(), vec![buf, offset])
         }
+        ast::Builtin::AddMod | ast::Builtin::MulMod => {
+            let arguments: Vec<Expression> = args
+                .iter()
+                .map(|v| expression(v, cfg, contract_no, func, ns, vartab, opt))
+                .collect();
+
+            let temp = vartab.temp_anonymous(&tys[0]);
+            let zero = Expression::NumberLiteral(*loc, tys[0].clone(), BigInt::zero());
+            let cond =
+                Expression::NotEqual(*loc, Box::new(zero.clone()), Box::new(arguments[2].clone()));
+
+            let true_block = cfg.new_basic_block("builtin_call".to_string());
+            let false_block = cfg.new_basic_block("zero".to_string());
+            let end_if = cfg.new_basic_block("end_if".to_string());
+
+            cfg.add(
+                vartab,
+                Instr::BranchCond {
+                    cond,
+                    true_block,
+                    false_block,
+                },
+            );
+
+            cfg.set_basic_block(true_block);
+            vartab.new_dirty_tracker();
+
+            cfg.add(
+                vartab,
+                Instr::Set {
+                    loc: *loc,
+                    res: temp,
+                    expr: Expression::Builtin(*loc, tys.to_vec(), builtin.into(), arguments),
+                },
+            );
+            cfg.add(vartab, Instr::Branch { block: end_if });
+
+            cfg.set_basic_block(false_block);
+            cfg.add(
+                vartab,
+                Instr::Set {
+                    loc: *loc,
+                    res: temp,
+                    expr: zero,
+                },
+            );
+            cfg.add(vartab, Instr::Branch { block: end_if });
+
+            cfg.set_phis(end_if, vartab.pop_dirty_tracker());
+            cfg.set_basic_block(end_if);
+            Expression::Variable(*loc, tys[0].clone(), temp)
+        }
         _ => {
             let arguments: Vec<Expression> = args
                 .iter()

src/codegen/yul/builtin.rs

@@ -74,32 +74,16 @@ pub(crate) fn process_builtin(
         | YulBuiltInFunction::Shl
         | YulBuiltInFunction::Shr
         | YulBuiltInFunction::Sar
-        | YulBuiltInFunction::Exp => {
-            process_binary_arithmetic(loc, builtin_ty, args, contract_no, ns, vartab, cfg, opt)
+        | YulBuiltInFunction::Exp
+        | YulBuiltInFunction::AddMod
+        | YulBuiltInFunction::MulMod => {
+            process_arithmetic(loc, builtin_ty, args, contract_no, ns, vartab, cfg, opt)
         }
 
         YulBuiltInFunction::Byte => {
             byte_builtin(loc, args, contract_no, ns, cfg, vartab, opt)
         }
 
-        YulBuiltInFunction::AddMod
-        | YulBuiltInFunction::MulMod => {
-            let left = expression(&args[0], contract_no, ns, vartab, cfg, opt);
-            let right = expression(&args[1], contract_no, ns, vartab, cfg, opt);
-            let (left, right) = equalize_types(left, right, ns);
-
-            let main_expr = if matches!(builtin_ty, YulBuiltInFunction::AddMod) {
-                Expression::Add(*loc, left.ty(), false, Box::new(left), Box::new(right))
-            } else {
-                Expression::Multiply(*loc, left.ty(), false, Box::new(left), Box::new(right))
-            };
-
-            let mod_arg = expression(&args[2], contract_no, ns, vartab, cfg, opt);
-            let (mod_left, mod_right) = equalize_types(main_expr, mod_arg, ns);
-            let codegen_expr = Expression::UnsignedModulo(*loc, mod_left.ty(), Box::new(mod_left), Box::new(mod_right.clone()));
-            branch_if_zero(mod_right, codegen_expr, cfg, vartab)
-        }
-
         YulBuiltInFunction::SignExtend
         | YulBuiltInFunction::Keccak256
         | YulBuiltInFunction::Pop
@@ -223,8 +207,8 @@ pub(crate) fn process_builtin(
     }
 }
 
-/// Process arithmetic operations with two arguments
-fn process_binary_arithmetic(
+/// Process arithmetic operations
+fn process_arithmetic(
     loc: &pt::Loc,
     builtin_ty: &YulBuiltInFunction,
     args: &[ast::YulExpression],
@@ -304,6 +288,23 @@ fn process_binary_arithmetic(
             Expression::ShiftRight(*loc, left.ty(), Box::new(right), Box::new(left), true)
         }
 
+        YulBuiltInFunction::AddMod | YulBuiltInFunction::MulMod => {
+            let modulo_operand = expression(&args[2], contract_no, ns, vartab, cfg, opt);
+            let (_, equalized_modulo) = equalize_types(left.clone(), modulo_operand.clone(), ns);
+            let builtin = if builtin_ty == &YulBuiltInFunction::AddMod {
+                Builtin::AddMod
+            } else {
+                Builtin::MulMod
+            };
+            let codegen_expr = Expression::Builtin(
+                *loc,
+                vec![left.ty()],
+                builtin,
+                vec![right, left, equalized_modulo],
+            );
+            branch_if_zero(modulo_operand, codegen_expr, cfg, vartab)
+        }
+
         _ => panic!("This is not a binary arithmetic operation!"),
     }
 }

src/emit/expression.rs

@@ -1607,13 +1607,10 @@ pub(super) fn expression<'a, T: TargetRuntime<'a> + ?Sized>(
             bin.builder.build_return(Some(&ret));
             bin.builder.position_at_end(success_block);
 
-            let quotient = bin
-                .builder
-                .build_load(quotient, "quotient")
-                .into_int_value();
+            let remainder = bin.builder.build_load(rem, "remainder").into_int_value();
 
             bin.builder
-                .build_int_truncate(quotient, res_ty, "quotient")
+                .build_int_truncate(remainder, res_ty, "quotient")
                 .into()
         }
         Expression::Builtin(_, _, Builtin::MulMod, args) => {
@@ -1717,13 +1714,10 @@ pub(super) fn expression<'a, T: TargetRuntime<'a> + ?Sized>(
 
             bin.builder.position_at_end(success_block);
 
-            let quotient = bin
-                .builder
-                .build_load(quotient, "quotient")
-                .into_int_value();
+            let remainder = bin.builder.build_load(rem, "quotient").into_int_value();
 
             bin.builder
-                .build_int_truncate(quotient, res_ty, "quotient")
+                .build_int_truncate(remainder, res_ty, "quotient")
                 .into()
         }
         Expression::Builtin(_, _, hash @ Builtin::Ripemd160, args)

tests/codegen_testcases/yul/common_builtins.sol

@@ -21,7 +21,7 @@ contract testing {
             // CHECK: ty:uint256 %temp.11 = uint256 0
             // CHECK: branch block3
             // CHECK: block2: # else
-            // CHECK: ty:uint256 %temp.11 = (unsigned modulo ((arg #0) + (arg #1)) % (arg #2))
+            // CHECK: ty:uint256 %temp.11 = (builtin AddMod ((arg #1), (arg #0), (arg #2)))
             // CHECK: branch block3
             // CHECK: block3: # endif
             // CHECK: # phis: temp.11
@@ -33,7 +33,7 @@ contract testing {
             // CHECK: ty:uint256 %temp.12 = uint256 0
             // CHECK: branch block6
             // CHECK: block5: # else
-            // CHECK: ty:uint256 %temp.12 = (unsigned modulo ((arg #0) * (arg #1)) % (arg #2))
+            // CHECK: ty:uint256 %temp.12 = (builtin MulMod ((arg #1), (arg #0), (arg #2)))
             // CHECK: branch block6
             // CHECK: block6: # endif
             // CHECK: # phis: temp.12

tests/solana_tests/yul.rs

@@ -358,3 +358,32 @@ contract testing  {
     b_vec.reverse();
     assert_eq!(b_vec, sender.to_vec());
 }
+
+#[test]
+fn addmod_mulmod() {
+    let mut vm = build_solidity(
+        r#"
+    contract foo {
+        function testMod() public pure returns (uint256 a, uint256 b) {
+            assembly {
+                let x := 115792089237316195423570985008687907853269984665640564039457584007913129639935
+                let y := 115792089237316195423570985008687907853269984665640564039457584007913129639935
+
+                a := mulmod(x, 2, 10)
+                b := addmod(y, 2, 10)
+            }
+
+            return (a, b);
+        }
+    }
+        "#,
+    );
+
+    vm.constructor("foo", &[]);
+
+    let returns = vm.function("testMod", &[], &[], None);
+    assert_eq!(
+        returns,
+        vec![Token::Uint(Uint::from(0)), Token::Uint(Uint::from(7)),]
+    );
+}

tests/substrate_tests/builtins.rs

@@ -479,7 +479,7 @@ fn addmod() {
         r##"
         contract x {
             function test() public {
-                assert(addmod(500, 100, 3) == 200);
+                assert(addmod(500, 100, 3) == 0);
             }
         }"##,
     );
@@ -491,12 +491,12 @@ fn addmod() {
         r##"
         contract x {
             function test() public {
-                assert(addmod(500, 100, 0) == 200);
+                assert(addmod(500, 100, 0) == 0);
             }
         }"##,
     );
 
-    runtime.function_expect_failure("test", Vec::new());
+    runtime.function("test", Vec::new());
 
     // bigger numbers (64 bit)
     let mut runtime = build_solidity(
@@ -507,7 +507,7 @@ fn addmod() {
                 assert(addmod(
                     0,
                     134_480_801_439_669_508_040_541_782_812_209_371_611,
-                    16_473_784_705_703_234_153) == 8_163_321_534_310_945_187);
+                    16_473_784_705_703_234_153) == 0);
             }
         }"##,
     );
@@ -523,7 +523,7 @@ fn addmod() {
                 assert(addmod(
                     0,
                     37_927_759_795_988_462_606_362_647_643_228_779_300_269_446_446_871_437_380_583_919_404_728_626_309_579,
-                    148_872_967_607_295_528_830_315_866_466_318_446_379) == 254_765_928_331_839_140_628_748_569_208_536_440_801);
+                    148_872_967_607_295_528_830_315_866_466_318_446_379) == 0);
             }
         }"##,
     );
@@ -538,7 +538,7 @@ fn addmod() {
                 assert(addmod(
                     109802613191917590715814365746623394364442484359636492253827647701845853490667,
                     49050800785888222684575674817707208319566972397745729319314900174750088808217,
-                    233) == 681774308917621516739871418731032629545104964623958032502757716208566275960);
+                    233) == 204);
             }
         }"##,
     );
@@ -552,7 +552,7 @@ fn addmod() {
                 assert(addmod(
                     109802613191917590715814365746623394364442484359636492253827647701845853490667,
                     109802613191917590715814365746623394364442484359636492253827647701845853490667,
-                    2) == 109802613191917590715814365746623394364442484359636492253827647701845853490667);
+                    2) == 0);
             }
         }"##,
     );
@@ -567,7 +567,7 @@ fn mulmod() {
         r##"
         contract x {
             function test() public {
-                assert(mulmod(500, 100, 5) == 10000);
+                assert(mulmod(500, 100, 5) == 0);
             }
         }"##,
     );
@@ -579,19 +579,19 @@ fn mulmod() {
         r##"
         contract x {
             function test() public {
-                assert(mulmod(500, 100, 0) == 200);
+                assert(mulmod(500, 100, 0) == 0);
             }
         }"##,
     );
 
-    runtime.function_expect_failure("test", Vec::new());
+    runtime.function("test", Vec::new());
 
     // bigger numbers
     let mut runtime = build_solidity(
         r##"
         contract x {
             function test() public {
-                assert(mulmod(50000, 10000, 5) == 10000_0000);
+                assert(mulmod(50000, 10000, 5) == 0);
             }
         }"##,
     );
@@ -602,7 +602,7 @@ fn mulmod() {
         r##"
         contract x {
             function test() public {
-                assert(mulmod(18446744073709551616, 18446744073709550403, 1024) == 332306998946228946374486373068439552);
+                assert(mulmod(18446744073709551616, 18446744073709550403, 1024) == 0);
             }
         }"##,
     );
@@ -614,7 +614,7 @@ fn mulmod() {
         r##"
         contract x {
             function test() public {
-                assert(mulmod(170141183460469231731687303715884105728, 170141183460469231731687303715884105728, 170141183460469231731687303715884105728) == 170141183460469231731687303715884105728);
+                assert(mulmod(170141183460469231731687303715884105728, 170141183460469231731687303715884105728, 170141183460469231731687303715884105728) == 0);
             }
         }"##,
     );
@@ -626,7 +626,7 @@ fn mulmod() {
         r##"
         contract x {
             function test() public {
-                assert(mulmod(340282366920938463463374607431768211456, 340282366920938463463374607431768211456, 340282366920938463463374607431768211456) == 340282366920938463463374607431768211456);
+                assert(mulmod(340282366920938463463374607431768211456, 340282366920938463463374607431768211456, 340282366920938463463374607431768211456) == 0);
             }
         }"##,
     );
@@ -641,7 +641,7 @@ fn mulmod() {
                 assert(mulmod(1766847064778384329583297500742918515827483896875618958121606201292619776,
                     1766847064778384329583297500742918515827483896875618958121606201292619776,
                     1766847064778384329583297500742918515827483896875618958121606201292619776)
-                    == 1766847064778384329583297500742918515827483896875618958121606201292619776);
+                    == 0);
             }
         }"##,
     );
@@ -656,7 +656,7 @@ fn mulmod() {
                 assert(mulmod(824364134751099588297822369420176791913922347811791536817152126684405253,
                     824364134751099588297822369420176791913922347811791536817152126684405253,
                     824364134751099588297822369420176791913922347811791536817152126684405253)
-                    == 824364134751099588297822369420176791913922347811791536817152126684405253);
+                    == 0);
             }
         }"##,
     );
@@ -671,7 +671,21 @@ fn mulmod() {
                 assert(mulmod(113477814626329405513123655892059150026234290706112418221315641434319827527851,
                     113477814626329405513123655892059150026234290706112418221315641434319827527851,
                     113477814626329405513123655892059150026234290706112418221315641434319827527851)
-                    == 113477814626329405513123655892059150026234290706112418221315641434319827527851);
+                    == 0);
+            }
+        }"##,
+    );
+
+    runtime.function("test", Vec::new());
+
+    let mut runtime = build_solidity(
+        r##"
+        contract x {
+            function test() public {
+                assert(mulmod(113477814626329405513123655892059150026234290706112418221315641434319827527851,
+                    113477814626329405513123655892059150026234290706112418221315641434319827527841,
+                    233)
+                    == 12);
             }
         }"##,
     );