From 93c852bfcdee36fb661af7fb12ac1dbb8fc69705 Mon Sep 17 00:00:00 2001 From: Andrey Khmuro Date: Thu, 2 Nov 2023 18:12:29 +0100 Subject: [PATCH 1/2] refactor: make shift and rotate instructions always checked --- assembly/src/assembler/instruction/u32_ops.rs | 28 ++- docs/src/user_docs/assembly/u32_operations.md | 8 +- .../operations/u32_ops/bitwise_ops.rs | 193 ++++++++++++++++-- .../integration/operations/u32_ops/mod.rs | 8 + 4 files changed, 206 insertions(+), 31 deletions(-) diff --git a/assembly/src/assembler/instruction/u32_ops.rs b/assembly/src/assembler/instruction/u32_ops.rs index 8675a05b0e..852a4155ee 100644 --- a/assembly/src/assembler/instruction/u32_ops.rs +++ b/assembly/src/assembler/instruction/u32_ops.rs @@ -260,16 +260,33 @@ pub fn u32rotr( match imm { Some(0) => { // if rotation is performed by 0, do nothing (Noop) - span.push_op(Noop); + span.push_ops([Pad, U32assert2(ZERO), Drop]); return Ok(None); } Some(imm) => { validate_param(imm, 1..=MAX_U32_ROTATE_VALUE)?; - span.push_op(Push(Felt::new(1 << (32 - imm)))); + span.push_ops([Push(Felt::new(1 << (32 - imm))), U32assert2(ZERO)]); } None => { - span.push_ops([Push(Felt::new(32)), Swap, U32sub, Drop]); + span.push_ops([ + // Verify both b and a are u32. + U32assert2(ZERO), + // Calculate 32 - b and assert that the shift value b <= 31. + Push(Felt::from(MAX_U32_ROTATE_VALUE)), + Dup1, + U32sub, + Not, + Assert(ZERO), + Incr, + Dup1, + // If 32-b = 32, replace it with 0. + Eqz, + Not, + CSwap, + Drop, + ]); append_pow2_op(span); + span.push_op(Swap); } } span.add_ops([U32mul, Add]) @@ -366,14 +383,15 @@ fn prepare_bitwise( match imm { Some(0) => { // if shift/rotation is performed by 0, do nothing (Noop) - span.push_op(Noop); + span.push_ops([Pad, U32assert2(ZERO), Drop]); } Some(imm) => { validate_param(imm, 1..=MAX_U32_ROTATE_VALUE)?; - span.push_op(Push(Felt::new(1 << imm))); + span.push_ops([Push(Felt::new(1 << imm)), U32assert2(ZERO)]); } None => { append_pow2_op(span); + span.push_op(U32assert2(ZERO)); } } Ok(()) diff --git a/docs/src/user_docs/assembly/u32_operations.md b/docs/src/user_docs/assembly/u32_operations.md index ec7ed79658..681d1e4f2f 100644 --- a/docs/src/user_docs/assembly/u32_operations.md +++ b/docs/src/user_docs/assembly/u32_operations.md @@ -50,10 +50,10 @@ If the error code is omitted, the default value of $0$ is assumed. | u32or
- *(6 cycle)s* | [b, a, ...] | [c, ...] | Computes $c$ as a bitwise `OR` of binary representations of $a$ and $b$.
Fails if $max(a,b) \ge 2^{32}$ | | u32xor
- *(1 cycle)* | [b, a, ...] | [c, ...] | Computes $c$ as a bitwise `XOR` of binary representations of $a$ and $b$.
Fails if $max(a,b) \ge 2^{32}$ | | u32not
- *(5 cycles)* | [a, ...] | [b, ...] | Computes $b$ as a bitwise `NOT` of binary representation of $a$.
Fails if $a \ge 2^{32}$ | -| u32shl
- *(40 cycles)*
u32shl.*b*
- *(3 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow (a \cdot 2^b) \mod 2^{32}$
Undefined if $a \ge 2^{32}$ or $b > 31$ | -| u32shr
- *(40 cycles)*
u32shr.*b*
- *(3 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow \lfloor a/2^b \rfloor$
Undefined if $a \ge 2^{32}$ or $b > 31$ | -| u32rotl
- *(40 cycles)*
u32rotl.*b*
- *(3 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the left by $b$ bits.
Undefined if $a \ge 2^{32}$ or $b > 31$ | -| u32rotr
- *(44 cycles)*
u32rotr.*b*
- *(3 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the right by $b$ bits.
Undefined if $a \ge 2^{32}$ or $b > 31$ | +| u32shl
- *(47 cycles)*
u32shl.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow (a \cdot 2^b) \mod 2^{32}$
Fails if $a \ge 2^{32}$ or $b > 31$ | +| u32shr
- *(47 cycles)*
u32shr.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow \lfloor a/2^b \rfloor$
Fails if $a \ge 2^{32}$ or $b > 31$ | +| u32rotl
- *(47 cycles)*
u32rotl.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the left by $b$ bits.
Fails if $a \ge 2^{32}$ or $b > 31$ | +| u32rotr
- *(59 cycles)*
u32rotr.*b*
- *(6 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the right by $b$ bits.
Fails if $a \ge 2^{32}$ or $b > 31$ | | u32popcnt
- *(33 cycles)* | [a, ...] | [b, ...] | Computes $b$ by counting the number of set bits in $a$ (hamming weight of $a$).
Undefined if $a \ge 2^{32}$ | ### Comparison operations diff --git a/miden/tests/integration/operations/u32_ops/bitwise_ops.rs b/miden/tests/integration/operations/u32_ops/bitwise_ops.rs index e75be1047b..8cefe01ca2 100644 --- a/miden/tests/integration/operations/u32_ops/bitwise_ops.rs +++ b/miden/tests/integration/operations/u32_ops/bitwise_ops.rs @@ -1,4 +1,4 @@ -use super::test_input_out_of_bounds; +use super::{test_input_out_of_bounds, test_param_out_of_bounds}; use test_utils::{build_op_test, proptest::prelude::*, rand::rand_value, TestError, U32_BOUND}; // U32 OPERATIONS TESTS - MANUAL - BITWISE OPERATIONS @@ -193,10 +193,20 @@ fn u32shl() { let test = build_op_test!(asm_op, &[a as u64, b as u64]); test.expect_stack(&[a.wrapping_shl(b) as u64]); +} - // --- test out of bounds input (should not fail) -------------------------------------------- +#[test] +fn u32shl_fail() { + let asm_op = "u32shl"; + + // should fail if a >= 2^32 let test = build_op_test!(asm_op, &[U32_BOUND, 1]); - assert!(test.execute().is_ok()); + test.expect_error(TestError::ExecutionError("NotU32Value")); + + // should fail if b >= 32 + let test = build_op_test!(asm_op, &[1, 32]); + // if b >= 32, 2^b >= 2^32 or not a u32 + test.expect_error(TestError::ExecutionError("NotU32Value")); } #[test] @@ -225,17 +235,20 @@ fn u32shl_b() { let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); test.expect_stack(&[a.wrapping_shl(b) as u64]); - // // --- test random values --------------------------------------------------------------------- - // let a = rand_value::(); - // let b = rand_value::() % 32; + // --- test random values --------------------------------------------------------------------- + let a = rand_value::(); + let b = rand_value::() % 32; + + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[a.wrapping_shl(b) as u64]); +} - // let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); - // test.expect_stack(&[a.wrapping_shl(b) as u64]); +#[test] +fn u32shl_b_fail() { + let op_base = "u32shl"; - // // --- test out of bounds input (should not fail) -------------------------------------------- - // let b = 1; - // let test = build_op_test!(get_asm_op(b).as_str(), &[U32_BOUND]); - // assert!(test.execute().is_ok()); + test_input_out_of_bounds(format!("{}.{}", op_base, 1).as_str()); + test_param_out_of_bounds(op_base, 32); } #[test] @@ -269,10 +282,19 @@ fn u32shr() { let test = build_op_test!(asm_op, &[a as u64, b as u64]); test.expect_stack(&[a.wrapping_shr(b) as u64]); +} - // --- test out of bounds inputs (should not fail) -------------------------------------------- +#[test] +fn u32shr_fail() { + let asm_op = "u32shr"; + + // should fail if a >= 2^32 let test = build_op_test!(asm_op, &[U32_BOUND, 1]); - assert!(test.execute().is_ok()); + test.expect_error(TestError::ExecutionError("NotU32Value")); + + // should fail if b >= 32 + let test = build_op_test!(asm_op, &[1, 32]); + test.expect_error(TestError::ExecutionError("NotU32Value")); } #[test] @@ -307,11 +329,14 @@ fn u32shr_b() { let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); test.expect_stack(&[a.wrapping_shr(b) as u64]); +} - // --- test out of bounds inputs (should not fail) -------------------------------------------- - let b = 1; - let test = build_op_test!(get_asm_op(b).as_str(), &[U32_BOUND]); - assert!(test.execute().is_ok()); +#[test] +fn u32shr_b_fail() { + let op_base = "u32shr"; + + test_input_out_of_bounds(format!("{}.{}", op_base, 1).as_str()); + test_param_out_of_bounds(op_base, 32); } #[test] @@ -356,10 +381,72 @@ fn u32rotl() { let test = build_op_test!(asm_op, &[a as u64, b as u64]); test.expect_stack(&[a.rotate_left(b) as u64]); +} + +#[test] +fn u32rotl_fail() { + let asm_op = "u32rotl"; - // --- test out of bounds inputs (should not fail) -------------------------------------------- + // should fail if a >= 2^32 let test = build_op_test!(asm_op, &[U32_BOUND, 1]); - assert!(test.execute().is_ok()); + test.expect_error(TestError::ExecutionError("NotU32Value")); + + // should fail if b >= 32 + let test = build_op_test!(asm_op, &[1, 32]); + test.expect_error(TestError::ExecutionError("NotU32Value")); +} + +#[test] +fn u32rotl_b() { + // Computes c by rotating a 32-bit representation of a to the left by b bits. + let op_base = "u32rotl"; + let get_asm_op = |b: u32| format!("{op_base}.{b}"); + + // --- test simple case ----------------------------------------------------------------------- + let a = 1_u32; + let b = 1_u32; + let test = build_op_test!(get_asm_op(b).as_str(), &[5, a as u64]); + test.expect_stack(&[2, 5]); + + // --- test simple wraparound case with large a ----------------------------------------------- + let a = (1_u64 << 31) as u32; + let b: u32 = 1; + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[1]); + + // --- test simple case wraparound case with max b -------------------------------------------- + let a = 2_u32; + let b: u32 = 31; + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[1]); + + // --- no change when a is max value (all 1s) ------------------------------------------------- + let a = (U32_BOUND - 1) as u32; + let b = 2; + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[a as u64]); + + // --- test b = 0 --------------------------------------------------------------------------- + let a = rand_value::(); + let b = 0; + + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[a.rotate_left(b) as u64]); + + // --- test random values --------------------------------------------------------------------- + let a = rand_value::(); + let b = rand_value::() % 32; + + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[a.rotate_left(b) as u64]); +} + +#[test] +fn u32rotl_fail_b() { + let op_base = "u32rotl"; + + test_input_out_of_bounds(format!("{}.{}", op_base, 1).as_str()); + test_param_out_of_bounds(op_base, 32); } #[test] @@ -404,10 +491,72 @@ fn u32rotr() { let test = build_op_test!(asm_op, &[a as u64, b as u64]); test.expect_stack(&[a.rotate_right(b) as u64]); +} + +#[test] +fn u32rotr_fail() { + let asm_op = "u32rotr"; - // --- test out of bounds inputs (should not fail) -------------------------------------------- + // should fail if a >= 2^32 let test = build_op_test!(asm_op, &[U32_BOUND, 1]); - assert!(test.execute().is_ok()); + test.expect_error(TestError::ExecutionError("NotU32Value")); + + // should fail if b >= 32 + let test = build_op_test!(asm_op, &[1, 32]); + test.expect_error(TestError::ExecutionError("FailedAssertion")); +} + +#[test] +fn u32rotr_b() { + // Computes c by rotating a 32-bit representation of a to the right by b bits. + let op_base = "u32rotr"; + let get_asm_op = |b: u32| format!("{op_base}.{b}"); + + // --- test simple case ----------------------------------------------------------------------- + let a = 2_u32; + let b = 1_u32; + let test = build_op_test!(get_asm_op(b).as_str(), &[5, a as u64]); + test.expect_stack(&[1, 5]); + + // --- test simple wraparound case with small a ----------------------------------------------- + let a = 1_u32; + let b = 1_u32; + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[U32_BOUND >> 1]); + + // --- test simple case wraparound case with max b -------------------------------------------- + let a = 1_u32; + let b: u32 = 31; + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[2]); + + // --- no change when a is max value (all 1s) ------------------------------------------------- + let a = (U32_BOUND - 1) as u32; + let b = 2; + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[a as u64]); + + // --- test b = 0 --------------------------------------------------------------------------- + let a = rand_value::(); + let b = 0; + + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[a.rotate_right(b) as u64]); + + // --- test random values --------------------------------------------------------------------- + let a = rand_value::(); + let b = rand_value::() % 32; + + let test = build_op_test!(get_asm_op(b).as_str(), &[a as u64]); + test.expect_stack(&[a.rotate_right(b) as u64]); +} + +#[test] +fn u32rotr_b_fail() { + let op_base = "u32rotr"; + + test_input_out_of_bounds(format!("{}.{}", op_base, 1).as_str()); + test_param_out_of_bounds(op_base, 32); } #[test] diff --git a/miden/tests/integration/operations/u32_ops/mod.rs b/miden/tests/integration/operations/u32_ops/mod.rs index 3d1746f1c2..397ae1b95b 100644 --- a/miden/tests/integration/operations/u32_ops/mod.rs +++ b/miden/tests/integration/operations/u32_ops/mod.rs @@ -30,6 +30,14 @@ pub fn test_inputs_out_of_bounds(asm_op: &str, input_count: usize) { } } +/// This helper function tests a provided assembly operation which takes a single parameter +/// to ensure that it fails when that parameter is over the maximum allowed value (out of bounds). +pub fn test_param_out_of_bounds(asm_op_base: &str, gt_max_value: u64) { + let asm_op = format!("{asm_op_base}.{gt_max_value}"); + let test = build_op_test!(&asm_op); + test.expect_error(TestError::AssemblyError("parameter")); +} + /// This helper function tests that when the given u32 assembly instruction is executed on /// out-of-bounds inputs it does not fail. Each input is tested independently. pub fn test_unchecked_execution(asm_op: &str, input_count: usize) { From a71e75c312ac3c8ec3d86ee38afc76e2cd9b6262 Mon Sep 17 00:00:00 2001 From: Andrey Khmuro Date: Fri, 3 Nov 2023 19:41:08 +0100 Subject: [PATCH 2/2] docs: update cycle count for shift and rot instructions --- assembly/src/assembler/instruction/u32_ops.rs | 16 ++++++++-------- docs/src/user_docs/assembly/u32_operations.md | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/assembly/src/assembler/instruction/u32_ops.rs b/assembly/src/assembler/instruction/u32_ops.rs index 852a4155ee..e18e3962c9 100644 --- a/assembly/src/assembler/instruction/u32_ops.rs +++ b/assembly/src/assembler/instruction/u32_ops.rs @@ -197,8 +197,8 @@ pub fn u32not(span: &mut SpanBuilder) -> Result, AssemblyError /// the value to be shifted and splitting the result. /// /// VM cycles per mode: -/// - u32shl: 18 cycles -/// - u32shl.b: 3 cycles +/// - u32shl: 19 cycles +/// - u32shl.b: 4 cycles pub fn u32shl(span: &mut SpanBuilder, imm: Option) -> Result, AssemblyError> { prepare_bitwise::(span, imm)?; if imm != Some(0) { @@ -214,8 +214,8 @@ pub fn u32shl(span: &mut SpanBuilder, imm: Option) -> Result) -> Result, AssemblyError> { prepare_bitwise::(span, imm)?; if imm != Some(0) { @@ -231,8 +231,8 @@ pub fn u32shr(span: &mut SpanBuilder, imm: Option) -> Result, @@ -251,8 +251,8 @@ pub fn u32rotl( /// b is the shift amount, then adding the overflow limb to the shifted limb. /// /// VM cycles per mode: -/// - u32rotr: 22 cycles -/// - u32rotr.b: 3 cycles +/// - u32rotr: 31 cycles +/// - u32rotr.b: 4 cycles pub fn u32rotr( span: &mut SpanBuilder, imm: Option, diff --git a/docs/src/user_docs/assembly/u32_operations.md b/docs/src/user_docs/assembly/u32_operations.md index 681d1e4f2f..712c14a08f 100644 --- a/docs/src/user_docs/assembly/u32_operations.md +++ b/docs/src/user_docs/assembly/u32_operations.md @@ -50,10 +50,10 @@ If the error code is omitted, the default value of $0$ is assumed. | u32or
- *(6 cycle)s* | [b, a, ...] | [c, ...] | Computes $c$ as a bitwise `OR` of binary representations of $a$ and $b$.
Fails if $max(a,b) \ge 2^{32}$ | | u32xor
- *(1 cycle)* | [b, a, ...] | [c, ...] | Computes $c$ as a bitwise `XOR` of binary representations of $a$ and $b$.
Fails if $max(a,b) \ge 2^{32}$ | | u32not
- *(5 cycles)* | [a, ...] | [b, ...] | Computes $b$ as a bitwise `NOT` of binary representation of $a$.
Fails if $a \ge 2^{32}$ | -| u32shl
- *(47 cycles)*
u32shl.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow (a \cdot 2^b) \mod 2^{32}$
Fails if $a \ge 2^{32}$ or $b > 31$ | -| u32shr
- *(47 cycles)*
u32shr.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow \lfloor a/2^b \rfloor$
Fails if $a \ge 2^{32}$ or $b > 31$ | -| u32rotl
- *(47 cycles)*
u32rotl.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the left by $b$ bits.
Fails if $a \ge 2^{32}$ or $b > 31$ | -| u32rotr
- *(59 cycles)*
u32rotr.*b*
- *(6 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the right by $b$ bits.
Fails if $a \ge 2^{32}$ or $b > 31$ | +| u32shl
- *(19 cycles)*
u32shl.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow (a \cdot 2^b) \mod 2^{32}$
Fails if $a \ge 2^{32}$ or $b > 31$ | +| u32shr
- *(19 cycles)*
u32shr.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | $c \leftarrow \lfloor a/2^b \rfloor$
Fails if $a \ge 2^{32}$ or $b > 31$ | +| u32rotl
- *(19 cycles)*
u32rotl.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the left by $b$ bits.
Fails if $a \ge 2^{32}$ or $b > 31$ | +| u32rotr
- *(31 cycles)*
u32rotr.*b*
- *(4 cycles)* | [b, a, ...] | [c, ...] | Computes $c$ by rotating a 32-bit representation of $a$ to the right by $b$ bits.
Fails if $a \ge 2^{32}$ or $b > 31$ | | u32popcnt
- *(33 cycles)* | [a, ...] | [b, ...] | Computes $b$ by counting the number of set bits in $a$ (hamming weight of $a$).
Undefined if $a \ge 2^{32}$ | ### Comparison operations