diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 66e435ae6b..2a03afbb2c 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -55,6 +55,34 @@ jobs: - name: Check workspace inheritance run: cargo workspace-inheritance-check --promotion-threshold 2 --promotion-failure + masm-lint: + name: masm-lint on ubuntu-latest + runs-on: ubuntu-latest + env: + CARGO_NET_GIT_FETCH_WITH_CLI: true + MASM_LSP_REF: 79f108aae4edcc6427d2fef9c40c1b28afe28397 + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + persist-credentials: false + - name: Cleanup large tools for build space + uses: ./.github/actions/cleanup-runner + - name: Install rust + run: rustup update --no-self-update + - name: Install masm-lint + run: > + cargo install + --git https://github.com/huitseeker/masm-lsp + --rev "$MASM_LSP_REF" + masm-lint + --bin masm-lint + --debug + - name: Lint MASM core library + run: > + masm-lint --no-color + --library miden::core=crates/lib/core/asm + crates/lib/core/asm + clippy: name: clippy nightly on ubuntu-latest runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index a62d41955d..329114478e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ #### Changes - [BREAKING] Refactored MAST forest serialization around fixed-layout full, stripped, and hashless sections, and bumped the MAST wire format to `0.0.3` ([#2765](https://github.com/0xMiden/miden-vm/pull/2765)). +- Added experimental `masm-lint` CI for the core MASM library, and fixed invalid signatures and missing `u32assert` checks it surfaced ([#3052](https://github.com/0xMiden/miden-vm/pull/3052)). - Documented sortedness precondition more prominently for sorted array operations ([#2832](https://github.com/0xMiden/miden-vm/pull/2832)). - [BREAKING] Updated the Miden crypto stack to `miden-crypto` and `miden-lifted-stark` v0.24, and switched digest-ordering code to `Word`'s native lexicographic ordering ([#3039](https://github.com/0xMiden/miden-vm/pull/3039)). - Borrowed operation slices in basic-block batching helpers to avoid cloning in the fingerprinting path ([#2994](https://github.com/0xMiden/miden-vm/pull/2994)). diff --git a/core/src/mast/debuginfo/debug_var_storage.rs b/core/src/mast/debuginfo/debug_var_storage.rs index b948fc2e1d..735374f28f 100644 --- a/core/src/mast/debuginfo/debug_var_storage.rs +++ b/core/src/mast/debuginfo/debug_var_storage.rs @@ -293,8 +293,7 @@ impl OpToDebugVarIds { .ok_or_else(|| "node_indptr_for_op_idx is unexpectedly empty".to_string())?; if last_node_ptr > max_node_ptr { return Err(format!( - "node_indptr_for_op_idx end {} exceeds op_indptr bounds {}", - last_node_ptr, max_node_ptr + "node_indptr_for_op_idx end {last_node_ptr} exceeds op_indptr bounds {max_node_ptr}" )); } diff --git a/core/src/mast/debuginfo/decorator_storage.rs b/core/src/mast/debuginfo/decorator_storage.rs index a2cf84866d..a0f9db2ceb 100644 --- a/core/src/mast/debuginfo/decorator_storage.rs +++ b/core/src/mast/debuginfo/decorator_storage.rs @@ -320,8 +320,7 @@ impl OpToDecoratorIds { .ok_or_else(|| "node_indptr_for_op_idx is unexpectedly empty".to_string())?; if last_node_ptr > max_node_ptr { return Err(format!( - "node_indptr_for_op_idx end {} exceeds op_indptr bounds {}", - last_node_ptr, max_node_ptr + "node_indptr_for_op_idx end {last_node_ptr} exceeds op_indptr bounds {max_node_ptr}" )); } diff --git a/crates/lib/core/asm/collections/sorted_array.masm b/crates/lib/core/asm/collections/sorted_array.masm index c83bea01ef..e93c8f2fb4 100644 --- a/crates/lib/core/asm/collections/sorted_array.masm +++ b/crates/lib/core/asm/collections/sorted_array.masm @@ -41,7 +41,7 @@ pub proc find_word #=> [maybe_value_ptr, VALUE, start_ptr, end_ptr] # dereference maybe_value_ptr (8 cycles) - dup movdn.5 padw movup.4 mem_loadw_le + dup u32assert.err="maybe_value_ptr is not u32" movdn.5 padw movup.4 mem_loadw_le #=> [MAYBE_VALUE, VALUE, maybe_value_ptr, start_ptr, end_ptr] # check if it matches the requested VALUE (15 cycles) @@ -78,7 +78,7 @@ pub proc find_word #=> [VALUE, maybe_value_ptr, start_ptr, end_ptr] # dereference maybe_value_ptr (7 cycles) - dup.4 padw movup.4 mem_loadw_le + dup.4 u32assert.err="start_ptr is not u32" padw movup.4 mem_loadw_le #=> [FIRST_WORD, VALUE, maybe_value_ptr = start_ptr, start_ptr, end_ptr] # there was no match, the first array element must be larger than VALUE @@ -99,7 +99,7 @@ pub proc find_word #=> [maybe_value_ptr = end_ptr, VALUE, start_ptr, end_ptr] # fetch the last word in the array (10 cycles) - dup movdn.5 sub.4 padw movup.4 mem_loadw_le + dup movdn.5 sub.4 u32assert.err="end_ptr-4 is not u32" padw movup.4 mem_loadw_le #=> [LAST_WORD, VALUE, maybe_value_ptr = end_ptr, start_ptr, end_ptr] # there was no match, the last array element must be smaller than VALUE @@ -122,7 +122,7 @@ pub proc find_word #=> [VALUE, VALUE, maybe_value_ptr, start_ptr, end_ptr] # dereference maybe_value_ptr (8 cycles) - padw dup.12 mem_loadw_le + padw dup.12 u32assert.err="maybe_value_ptr is not u32" mem_loadw_le #=> [MAYBE_VALUE, VALUE, VALUE, maybe_value_ptr, start_ptr, end_ptr] # there was no match, MAYBE_VALUE must be larger than VALUE (123 cycles) @@ -130,7 +130,7 @@ pub proc find_word assert.err="lowerbound_array invariant: *maybe_value_ptr not greater than VALUE" # dereference `maybe_value_ptr-4` (11 cycles) - padw dup.8 sub.4 mem_loadw_le + padw dup.8 sub.4 u32assert.err="maybe_value_ptr-4 is not u32" mem_loadw_le #=> [MAYBE_VALUE_PREV, VALUE, maybe_value_ptr, start_ptr, end_ptr] # there was no match, MAYBE_VALUE_PREV must be smaller than VALUE (119 cycles) @@ -254,11 +254,12 @@ proc find_partial_key_value #=> [maybe_key_ptr, KEY, start_ptr, end_ptr] # make sure maybe_key_ptr is pointing to a key, not value (10 cycles) - dup dup.6 sub u32mod.8 assertz.err="lowerbound_key_value invariant: key_ptr must be double-word aligned with start_ptr" + dup dup.6 sub u32assert.err="maybe_key_ptr-start_ptr is not u32" + u32mod.8 assertz.err="lowerbound_key_value invariant: key_ptr must be double-word aligned with start_ptr" #=> [maybe_key_ptr, KEY, start_ptr, end_ptr] # dereference maybe_key_ptr (22 cycles) - dup movdn.5 + dup u32assert.err="maybe_key_ptr is not u32" movdn.5 #=> [maybe_key_ptr, KEY, maybe_key_ptr, start_ptr, end_ptr] loc_load.0 exec.load_key @@ -293,7 +294,7 @@ proc find_partial_key_value #=> [maybe_key_ptr = start_ptr, KEY, start_ptr, end_ptr] # load key (22 cycles) - dup movdn.5 + dup u32assert.err="start_ptr is not u32" movdn.5 #=> [maybe_key_ptr, KEY, maybe_key_ptr, start_ptr, end_ptr] loc_load.0 exec.load_key @@ -317,7 +318,7 @@ proc find_partial_key_value #=> [maybe_key_ptr = end_ptr, KEY, start_ptr, end_ptr] # fetch the last key in the array (23 cycles) - dup movdn.5 sub.8 + dup movdn.5 sub.8 u32assert.err="end_ptr-8 is not u32" #=> [maybe_key_ptr-8, KEY, maybe_key_ptr, start_ptr, end_ptr] loc_load.0 exec.load_key @@ -400,4 +401,4 @@ proc load_key movup.4 drop # => [w0', w1', w2, w3] -end \ No newline at end of file +end diff --git a/crates/lib/core/asm/math/u256.masm b/crates/lib/core/asm/math/u256.masm index 415c6c3afd..1a483127df 100644 --- a/crates/lib/core/asm/math/u256.masm +++ b/crates/lib/core/asm/math/u256.masm @@ -295,7 +295,7 @@ end #! Returns 1 if the top u256 value is zero; lower stack values are preserved. #! Stack transition looks as follows: #! [a0, a1, a2, a3, a4, a5, a6, a7, ...] -> [is_zero, ...]. -pub proc eqz(rhs: u256, lhs: u256) -> i1 +pub proc eqz(a: u256) -> i1 eq.0 repeat.7 swap diff --git a/crates/lib/core/asm/pcs/fri/frie2f4.masm b/crates/lib/core/asm/pcs/fri/frie2f4.masm index b6773b104a..0606ec8675 100644 --- a/crates/lib/core/asm/pcs/fri/frie2f4.masm +++ b/crates/lib/core/asm/pcs/fri/frie2f4.masm @@ -14,6 +14,7 @@ use miden::core::pcs::fri::helper @locals(16) pub proc preprocess adv_push + u32assert # => [num_queries, g, ...] exec.constants::fri_com_ptr # => [layer_ptr, num_queries, g, ...] @@ -45,6 +46,7 @@ pub proc preprocess #=> [X, layer_ptr, layer_ptr, g] adv_push + u32assert mul.2 sub.1 movdn.4 @@ -72,6 +74,7 @@ pub proc preprocess #=> [X, remainder_poly_ptr, remainder_poly_ptr, layer_ptr, g] adv_push + u32assert dup mul.2 exec.constants::set_remainder_poly_size sub.1 diff --git a/crates/lib/core/asm/sys/mod.masm b/crates/lib/core/asm/sys/mod.masm index f34f819685..80674fa36f 100644 --- a/crates/lib/core/asm/sys/mod.masm +++ b/crates/lib/core/asm/sys/mod.masm @@ -31,7 +31,7 @@ pub proc truncate_stack() end #! Drop 16 values from the stack. -pub proc drop_stack_top() +pub proc drop_stack_top(top: word, upper_mid: word, lower_mid: word, bottom: word) dropw dropw dropw dropw end diff --git a/crates/lib/core/tests/pcs/fri/mod.rs b/crates/lib/core/tests/pcs/fri/mod.rs index 64abb19587..08bcb00c0d 100644 --- a/crates/lib/core/tests/pcs/fri/mod.rs +++ b/crates/lib/core/tests/pcs/fri/mod.rs @@ -8,6 +8,14 @@ pub(crate) mod verifier_fri_e2f4; use miden_core::Word; pub use verifier_fri_e2f4::*; +const PREPROCESS_SOURCE: &str = " + use miden::core::pcs::fri::frie2f4 + + begin + exec.frie2f4::preprocess + end + "; + #[test] fn fri_fold4_ext2_remainder64() { let source = " @@ -104,6 +112,79 @@ fn fri_fold4_ext2_remainder128() { test.expect_stack(&[]); } +#[test] +fn fri_preprocess_rejects_non_u32_num_queries() { + let (domain_generator, mut advice_stack, num_layers_index, remainder_length_index) = + prepare_preprocess_inputs(14); + let invalid_u32 = u64::from(u32::MAX) + 1; + + advice_stack[0] = invalid_u32; + // Keep later counters valid so this test isolates the first assertion. + advice_stack[num_layers_index] = 1; + advice_stack[remainder_length_index] = 1; + + let test = build_test!(PREPROCESS_SOURCE, &[domain_generator], &advice_stack); + expect_assert_error_message!(test); +} + +#[test] +fn fri_preprocess_rejects_non_u32_num_layers() { + let (domain_generator, mut advice_stack, num_layers_index, remainder_length_index) = + prepare_preprocess_inputs(14); + let invalid_u32 = u64::from(u32::MAX) + 1; + + advice_stack[num_layers_index] = invalid_u32; + advice_stack[remainder_length_index] = 1; + + let test = build_test!(PREPROCESS_SOURCE, &[domain_generator], &advice_stack); + expect_assert_error_message!(test); +} + +#[test] +fn fri_preprocess_rejects_non_u32_remainder_length() { + let (domain_generator, mut advice_stack, _, remainder_length_index) = + prepare_preprocess_inputs(14); + let invalid_u32 = u64::from(u32::MAX) + 1; + + advice_stack[remainder_length_index] = invalid_u32; + + let test = build_test!(PREPROCESS_SOURCE, &[domain_generator], &advice_stack); + expect_assert_error_message!(test); +} + +fn prepare_preprocess_inputs(trace_len_e: usize) -> (u64, Vec, usize, usize) { + let blowup_exp = 3; + let depth = trace_len_e + blowup_exp; + let domain_size = 1 << depth; + + let FriResult { + positions, + alphas, + commitments, + remainder, + num_queries, + .. + } = fri_prove_verify_fold4_ext2(trace_len_e).unwrap(); + + let num_layers = (commitments.len() / 4) - 1; + let num_layers_index = 1 + positions.len(); + let remainder_length_index = num_layers_index + 1 + (num_layers * 8); + + let advice_stack = prepare_advice_stack( + depth, + domain_size, + num_queries, + positions, + alphas, + commitments, + remainder, + ); + + let domain_generator = Felt::get_root_of_unity(domain_size.ilog2()).as_canonical_u64(); + + (domain_generator, advice_stack, num_layers_index, remainder_length_index) +} + fn prepare_advice_stack( depth: usize, domain_size: u32,