From 82714dd977324ef35944a4debe2d7d25f6b3284d Mon Sep 17 00:00:00 2001 From: Justin Traglia <95511699+jtraglia@users.noreply.github.com> Date: Sat, 30 Mar 2024 05:19:52 -0500 Subject: [PATCH] nit: move div by zero check from smod to i256_mod (#1248) --- .../src/instructions/arithmetic.rs | 4 +- crates/interpreter/src/instructions/i256.rs | 140 +++++++++++++----- 2 files changed, 106 insertions(+), 38 deletions(-) diff --git a/crates/interpreter/src/instructions/arithmetic.rs b/crates/interpreter/src/instructions/arithmetic.rs index e982ccd4710..583299d58cb 100644 --- a/crates/interpreter/src/instructions/arithmetic.rs +++ b/crates/interpreter/src/instructions/arithmetic.rs @@ -48,9 +48,7 @@ pub fn rem(interpreter: &mut Interpreter, _host: &mut H) { pub fn smod(interpreter: &mut Interpreter, _host: &mut H) { gas!(interpreter, gas::LOW); pop_top!(interpreter, op1, op2); - if *op2 != U256::ZERO { - *op2 = i256_mod(op1, *op2) - } + *op2 = i256_mod(op1, *op2) } pub fn addmod(interpreter: &mut Interpreter, _host: &mut H) { diff --git a/crates/interpreter/src/instructions/i256.rs b/crates/interpreter/src/instructions/i256.rs index a26ae69ad46..eaccbab7051 100644 --- a/crates/interpreter/src/instructions/i256.rs +++ b/crates/interpreter/src/instructions/i256.rs @@ -77,7 +77,7 @@ pub fn i256_div(mut first: U256, mut second: U256) -> U256 { } let first_sign = i256_sign_compl(&mut first); - if first_sign == Sign::Minus && first == MIN_NEGATIVE_VALUE && second == U256::from(1) { + if first == MIN_NEGATIVE_VALUE && second == U256::from(1) { return two_compl(MIN_NEGATIVE_VALUE); } @@ -105,9 +105,11 @@ pub fn i256_mod(mut first: U256, mut second: U256) -> U256 { return U256::ZERO; } - let _ = i256_sign_compl(&mut second); + let second_sign = i256_sign_compl(&mut second); + if second_sign == Sign::Zero { + return U256::ZERO; + } - // necessary overflow checks are done above, perform the operation let mut r = first % second; // set sign bit to zero @@ -125,6 +127,58 @@ mod tests { use super::*; use core::num::Wrapping; + const ZERO: U256 = U256::ZERO; + const ONE: U256 = U256::from_limbs([ + 0x0000000000000001, + 0x0000000000000000, + 0x0000000000000000, + 0x0000000000000000, + ]); + const TWO: U256 = U256::from_limbs([ + 0x0000000000000002, + 0x0000000000000000, + 0x0000000000000000, + 0x0000000000000000, + ]); + const THREE: U256 = U256::from_limbs([ + 0x0000000000000003, + 0x0000000000000000, + 0x0000000000000000, + 0x0000000000000000, + ]); + const FOUR: U256 = U256::from_limbs([ + 0x0000000000000004, + 0x0000000000000000, + 0x0000000000000000, + 0x0000000000000000, + ]); + + const NEG_ONE: U256 = U256::from_limbs([ + 0xffffffffffffffff, + 0xffffffffffffffff, + 0xffffffffffffffff, + 0xffffffffffffffff, + ]); + const NEG_TWO: U256 = U256::from_limbs([ + 0xfffffffffffffffe, + 0xffffffffffffffff, + 0xffffffffffffffff, + 0xffffffffffffffff, + ]); + const NEG_THREE: U256 = U256::from_limbs([ + 0xfffffffffffffffd, + 0xffffffffffffffff, + 0xffffffffffffffff, + 0xffffffffffffffff, + ]); + + const I256_MAX: U256 = U256::from_limbs([ + 0xffffffffffffffff, + 0xffffffffffffffff, + 0xffffffffffffffff, + 0x7fffffffffffffff, + ]); + #[test] fn div_i256() { // Sanity checks based on i8. Notice that we need to use `Wrapping` here because @@ -133,70 +187,86 @@ mod tests { assert_eq!(i8::MAX / -1, -i8::MAX); // Now the same calculations based on i256 - let one = U256::from(1); - let one_hundred = U256::from(100); let fifty = U256::from(50); - let two = U256::from(2); - let neg_one_hundred = U256::from(100); - let minus_one = U256::from(1); - let max_value = U256::from(2).pow(U256::from(255)) - U256::from(1); - let neg_max_value = U256::from(2).pow(U256::from(255)) - U256::from(1); - - assert_eq!(i256_div(MIN_NEGATIVE_VALUE, minus_one), MIN_NEGATIVE_VALUE); - assert_eq!(i256_div(MIN_NEGATIVE_VALUE, one), MIN_NEGATIVE_VALUE); - assert_eq!(i256_div(max_value, one), max_value); - assert_eq!(i256_div(max_value, minus_one), neg_max_value); - assert_eq!(i256_div(one_hundred, minus_one), neg_one_hundred); - assert_eq!(i256_div(one_hundred, two), fifty); + let one_hundred = U256::from(100); + + assert_eq!(i256_div(MIN_NEGATIVE_VALUE, NEG_ONE), MIN_NEGATIVE_VALUE); + assert_eq!(i256_div(MIN_NEGATIVE_VALUE, ONE), MIN_NEGATIVE_VALUE); + assert_eq!(i256_div(I256_MAX, ONE), I256_MAX); + assert_eq!(i256_div(I256_MAX, NEG_ONE), NEG_ONE * I256_MAX); + assert_eq!(i256_div(one_hundred, NEG_ONE), NEG_ONE * one_hundred); + assert_eq!(i256_div(one_hundred, TWO), fifty); } #[test] fn test_i256_sign() { - assert_eq!(i256_sign(&U256::ZERO), Sign::Zero); - assert_eq!(i256_sign(&U256::from(1)), Sign::Plus); + assert_eq!(i256_sign(&ZERO), Sign::Zero); + assert_eq!(i256_sign(&ONE), Sign::Plus); assert_eq!(i256_sign(&MIN_NEGATIVE_VALUE), Sign::Minus); } #[test] fn test_i256_sign_compl() { - let mut positive = U256::from(1); + let mut zero = ZERO; + let mut positive = ONE; let mut negative = MIN_NEGATIVE_VALUE; + assert_eq!(i256_sign_compl(&mut zero), Sign::Zero); assert_eq!(i256_sign_compl(&mut positive), Sign::Plus); assert_eq!(i256_sign_compl(&mut negative), Sign::Minus); } #[test] fn test_two_compl() { - let value = U256::from(1); - assert_eq!(two_compl(value), U256::MAX); + assert_eq!(two_compl(ZERO), ZERO); + assert_eq!(two_compl(ONE), NEG_ONE); + assert_eq!(two_compl(NEG_ONE), ONE); + assert_eq!(two_compl(TWO), NEG_TWO); + assert_eq!(two_compl(NEG_TWO), TWO); + + // Two's complement of the min value is itself. + assert_eq!(two_compl(MIN_NEGATIVE_VALUE), MIN_NEGATIVE_VALUE); } #[test] fn test_two_compl_mut() { - let mut value = U256::from(1); + let mut value = ONE; two_compl_mut(&mut value); - assert_eq!(value, U256::MAX); + assert_eq!(value, NEG_ONE); } #[test] fn test_i256_cmp() { - assert_eq!(i256_cmp(&U256::from(1), &U256::from(2)), Ordering::Less); - assert_eq!(i256_cmp(&U256::from(2), &U256::from(2)), Ordering::Equal); - assert_eq!(i256_cmp(&U256::from(3), &U256::from(2)), Ordering::Greater); + assert_eq!(i256_cmp(&ONE, &TWO), Ordering::Less); + assert_eq!(i256_cmp(&TWO, &TWO), Ordering::Equal); + assert_eq!(i256_cmp(&THREE, &TWO), Ordering::Greater); + assert_eq!(i256_cmp(&NEG_ONE, &NEG_ONE), Ordering::Equal); + assert_eq!(i256_cmp(&NEG_ONE, &NEG_TWO), Ordering::Greater); + assert_eq!(i256_cmp(&NEG_ONE, &ZERO), Ordering::Less); + assert_eq!(i256_cmp(&NEG_TWO, &TWO), Ordering::Less); } #[test] fn test_i256_div() { - let one = U256::from(1); - let two = U256::from(2); - assert_eq!(i256_div(MIN_NEGATIVE_VALUE, one), MIN_NEGATIVE_VALUE); - assert_eq!(i256_div(U256::from(4), two), U256::from(2)); + assert_eq!(i256_div(ONE, ZERO), ZERO); + assert_eq!(i256_div(ZERO, ONE), ZERO); + assert_eq!(i256_div(ZERO, NEG_ONE), ZERO); + assert_eq!(i256_div(MIN_NEGATIVE_VALUE, ONE), MIN_NEGATIVE_VALUE); + assert_eq!(i256_div(FOUR, TWO), TWO); + assert_eq!(i256_div(MIN_NEGATIVE_VALUE, MIN_NEGATIVE_VALUE), ONE); + assert_eq!(i256_div(TWO, NEG_ONE), NEG_TWO); + assert_eq!(i256_div(NEG_TWO, NEG_ONE), TWO); } #[test] fn test_i256_mod() { - let one = U256::from(1); - let two = U256::from(2); - assert_eq!(i256_mod(U256::from(4), two), U256::ZERO); - assert_eq!(i256_mod(U256::from(3), two), one); + assert_eq!(i256_mod(ZERO, ONE), ZERO); + assert_eq!(i256_mod(ONE, ZERO), ZERO); + assert_eq!(i256_mod(FOUR, TWO), ZERO); + assert_eq!(i256_mod(THREE, TWO), ONE); + assert_eq!(i256_mod(MIN_NEGATIVE_VALUE, ONE), ZERO); + assert_eq!(i256_mod(TWO, TWO), ZERO); + assert_eq!(i256_mod(TWO, THREE), TWO); + assert_eq!(i256_mod(NEG_TWO, THREE), NEG_TWO); + assert_eq!(i256_mod(TWO, NEG_THREE), TWO); + assert_eq!(i256_mod(NEG_TWO, NEG_THREE), NEG_TWO); } }