Skip to content

Commit 6d0b628

Browse files
authored
fix: fix unbounded trace row generation for rv32im_hintbuffer (#2289)
Closes INT-4830
1 parent e017615 commit 6d0b628

File tree

18 files changed

+327
-84
lines changed

18 files changed

+327
-84
lines changed

crates/circuits/primitives/cuda/include/primitives/constants.h

Lines changed: 55 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3,91 +3,97 @@
33
#include <cstddef>
44

55
namespace riscv {
6-
static const size_t RV32_REGISTER_NUM_LIMBS = 4;
7-
static const size_t RV32_CELL_BITS = 8;
8-
static const size_t RV_J_TYPE_IMM_BITS = 21;
6+
inline constexpr size_t RV32_REGISTER_NUM_LIMBS = 4;
7+
inline constexpr size_t RV32_CELL_BITS = 8;
8+
inline constexpr size_t RV_J_TYPE_IMM_BITS = 21;
99

10-
static const size_t RV32_IMM_AS = 0;
10+
inline constexpr size_t RV32_IMM_AS = 0;
1111
} // namespace riscv
1212

1313
namespace program {
14-
static const size_t PC_BITS = 30;
15-
static const size_t DEFAULT_PC_STEP = 4;
14+
inline constexpr size_t PC_BITS = 30;
15+
inline constexpr size_t DEFAULT_PC_STEP = 4;
1616
} // namespace program
1717

1818
namespace native {
19-
static const size_t AS_IMMEDIATE = 0;
20-
static const size_t AS_NATIVE = 4;
21-
static const size_t EXT_DEG = 4;
22-
static const size_t BETA = 11;
19+
inline constexpr size_t AS_IMMEDIATE = 0;
20+
inline constexpr size_t AS_NATIVE = 4;
21+
inline constexpr size_t EXT_DEG = 4;
22+
inline constexpr size_t BETA = 11;
2323
} // namespace native
2424

2525
namespace poseidon2 {
26-
static const size_t CHUNK = 8;
26+
inline constexpr size_t CHUNK = 8;
2727
} // namespace poseidon2
2828

2929
namespace p3_keccak_air {
30-
static const size_t NUM_ROUNDS = 24;
31-
static const size_t BITS_PER_LIMB = 16;
32-
static const size_t U64_LIMBS = 64 / BITS_PER_LIMB;
33-
static const size_t RATE_BITS = 1088;
34-
static const size_t RATE_LIMBS = RATE_BITS / BITS_PER_LIMB;
30+
inline constexpr size_t NUM_ROUNDS = 24;
31+
inline constexpr size_t BITS_PER_LIMB = 16;
32+
inline constexpr size_t U64_LIMBS = 64 / BITS_PER_LIMB;
33+
inline constexpr size_t RATE_BITS = 1088;
34+
inline constexpr size_t RATE_LIMBS = RATE_BITS / BITS_PER_LIMB;
3535
} // namespace p3_keccak_air
3636

3737
namespace keccak256 {
3838
/// Total number of sponge bytes: number of rate bytes + number of capacity bytes.
39-
static const size_t KECCAK_WIDTH_BYTES = 200;
39+
inline constexpr size_t KECCAK_WIDTH_BYTES = 200;
4040
/// Total number of 16-bit limbs in the sponge.
41-
static const size_t KECCAK_WIDTH_U16S = KECCAK_WIDTH_BYTES / 2;
41+
inline constexpr size_t KECCAK_WIDTH_U16S = KECCAK_WIDTH_BYTES / 2;
4242
/// Number of rate bytes.
43-
static const size_t KECCAK_RATE_BYTES = 136;
43+
inline constexpr size_t KECCAK_RATE_BYTES = 136;
4444
/// Number of 16-bit rate limbs.
45-
static const size_t KECCAK_RATE_U16S = KECCAK_RATE_BYTES / 2;
45+
inline constexpr size_t KECCAK_RATE_U16S = KECCAK_RATE_BYTES / 2;
4646
/// Number of absorb rounds, equal to rate in u64s.
47-
static const size_t NUM_ABSORB_ROUNDS = KECCAK_RATE_BYTES / 8;
47+
inline constexpr size_t NUM_ABSORB_ROUNDS = KECCAK_RATE_BYTES / 8;
4848
/// Number of capacity bytes.
49-
static const size_t KECCAK_CAPACITY_BYTES = 64;
49+
inline constexpr size_t KECCAK_CAPACITY_BYTES = 64;
5050
/// Number of 16-bit capacity limbs.
51-
static const size_t KECCAK_CAPACITY_U16S = KECCAK_CAPACITY_BYTES / 2;
51+
inline constexpr size_t KECCAK_CAPACITY_U16S = KECCAK_CAPACITY_BYTES / 2;
5252
/// Number of output digest bytes used during the squeezing phase.
53-
static const size_t KECCAK_DIGEST_BYTES = 32;
53+
inline constexpr size_t KECCAK_DIGEST_BYTES = 32;
5454
/// Number of 64-bit digest limbs.
55-
static const size_t KECCAK_DIGEST_U64S = KECCAK_DIGEST_BYTES / 8;
55+
inline constexpr size_t KECCAK_DIGEST_U64S = KECCAK_DIGEST_BYTES / 8;
5656

5757
// ==== Constants for register/memory adapter ====
5858
/// Register reads to get dst, src, len
59-
static const size_t KECCAK_REGISTER_READS = 3;
59+
inline constexpr size_t KECCAK_REGISTER_READS = 3;
6060
/// Number of cells to read/write in a single memory access
61-
static const size_t KECCAK_WORD_SIZE = 4;
61+
inline constexpr size_t KECCAK_WORD_SIZE = 4;
6262
/// Memory reads for absorb per row
63-
static const size_t KECCAK_ABSORB_READS = KECCAK_RATE_BYTES / KECCAK_WORD_SIZE;
63+
inline constexpr size_t KECCAK_ABSORB_READS = KECCAK_RATE_BYTES / KECCAK_WORD_SIZE;
6464
/// Memory writes for digest per row
65-
static const size_t KECCAK_DIGEST_WRITES = KECCAK_DIGEST_BYTES / KECCAK_WORD_SIZE;
65+
inline constexpr size_t KECCAK_DIGEST_WRITES = KECCAK_DIGEST_BYTES / KECCAK_WORD_SIZE;
6666
/// keccakf parameters
67-
static const size_t KECCAK_ROUND = 24;
68-
static const size_t KECCAK_STATE_SIZE = 25;
69-
static const size_t KECCAK_Q_SIZE = 192;
67+
inline constexpr size_t KECCAK_ROUND = 24;
68+
inline constexpr size_t KECCAK_STATE_SIZE = 25;
69+
inline constexpr size_t KECCAK_Q_SIZE = 192;
7070
/// From memory config
71-
static const size_t KECCAK_POINTER_MAX_BITS = 29;
71+
inline constexpr size_t KECCAK_POINTER_MAX_BITS = 29;
7272
} // namespace keccak256
7373

7474
namespace mod_builder {
75-
static const size_t MAX_LIMBS = 97;
75+
inline constexpr size_t MAX_LIMBS = 97;
7676
} // namespace mod_builder
7777

7878
namespace sha256 {
79-
static const size_t SHA256_BLOCK_BITS = 512;
80-
static const size_t SHA256_BLOCK_U8S = 64;
81-
static const size_t SHA256_BLOCK_WORDS = 16;
82-
static const size_t SHA256_WORD_U8S = 4;
83-
static const size_t SHA256_WORD_BITS = 32;
84-
static const size_t SHA256_WORD_U16S = 2;
85-
static const size_t SHA256_HASH_WORDS = 8;
86-
static const size_t SHA256_NUM_READ_ROWS = 4;
87-
static const size_t SHA256_ROWS_PER_BLOCK = 17;
88-
static const size_t SHA256_ROUNDS_PER_ROW = 4;
89-
static const size_t SHA256_ROW_VAR_CNT = 5;
90-
static const size_t SHA256_REGISTER_READS = 3;
91-
static const size_t SHA256_READ_SIZE = 16;
92-
static const size_t SHA256_WRITE_SIZE = 32;
93-
} // namespace sha256
79+
inline constexpr size_t SHA256_BLOCK_BITS = 512;
80+
inline constexpr size_t SHA256_BLOCK_U8S = 64;
81+
inline constexpr size_t SHA256_BLOCK_WORDS = 16;
82+
inline constexpr size_t SHA256_WORD_U8S = 4;
83+
inline constexpr size_t SHA256_WORD_BITS = 32;
84+
inline constexpr size_t SHA256_WORD_U16S = 2;
85+
inline constexpr size_t SHA256_HASH_WORDS = 8;
86+
inline constexpr size_t SHA256_NUM_READ_ROWS = 4;
87+
inline constexpr size_t SHA256_ROWS_PER_BLOCK = 17;
88+
inline constexpr size_t SHA256_ROUNDS_PER_ROW = 4;
89+
inline constexpr size_t SHA256_ROW_VAR_CNT = 5;
90+
inline constexpr size_t SHA256_REGISTER_READS = 3;
91+
inline constexpr size_t SHA256_READ_SIZE = 16;
92+
inline constexpr size_t SHA256_WRITE_SIZE = 32;
93+
} // namespace sha256
94+
95+
namespace hintstore {
96+
// Must match MAX_HINT_BUFFER_WORDS_BITS in openvm_rv32im_guest::lib.rs
97+
inline constexpr size_t MAX_HINT_BUFFER_WORDS_BITS = 18;
98+
inline constexpr size_t MAX_HINT_BUFFER_WORDS = (1 << MAX_HINT_BUFFER_WORDS_BITS) - 1;
99+
} // namespace hintstore

crates/toolchain/openvm/src/io/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use core::alloc::Layout;
66
use core::fmt::Write;
77

88
#[cfg(target_os = "zkvm")]
9-
use openvm_rv32im_guest::{hint_buffer_u32, hint_input, hint_store_u32};
9+
use openvm_rv32im_guest::{hint_buffer_chunked, hint_input, hint_store_u32};
1010
use serde::de::DeserializeOwned;
1111

1212
#[cfg(not(target_os = "zkvm"))]
@@ -83,7 +83,7 @@ pub(crate) fn read_vec_by_len(len: usize) -> Vec<u8> {
8383
// The heap-embedded-alloc uses linked list allocator, which has a minimum alignment of
8484
// `sizeof(usize) * 2 = 8` on 32-bit architectures: https://github.com/rust-osdev/linked-list-allocator/blob/b5caf3271259ddda60927752fa26527e0ccd2d56/src/hole.rs#L429
8585
let mut bytes = Vec::with_capacity(capacity);
86-
hint_buffer_u32!(bytes.as_mut_ptr(), num_words);
86+
hint_buffer_chunked(bytes.as_mut_ptr(), num_words as usize);
8787
// SAFETY: We populate a `Vec<u8>` by hintstore-ing `num_words` 4 byte words. We set the
8888
// length to `len` and don't care about the extra `capacity - len` bytes stored.
8989
unsafe {

crates/toolchain/openvm/src/io/read.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use core::mem::MaybeUninit;
22

33
use openvm_platform::WORD_SIZE;
44
#[cfg(target_os = "zkvm")]
5-
use openvm_rv32im_guest::hint_buffer_u32;
5+
use openvm_rv32im_guest::hint_buffer_chunked;
66

77
use super::hint_store_word;
88
use crate::serde::WordRead;
@@ -31,7 +31,7 @@ impl WordRead for Reader {
3131
let num_words = words.len();
3232
if let Some(new_remaining) = self.bytes_remaining.checked_sub(num_words * WORD_SIZE) {
3333
#[cfg(target_os = "zkvm")]
34-
hint_buffer_u32!(words.as_mut_ptr(), words.len());
34+
hint_buffer_chunked(words.as_mut_ptr() as *mut u8, words.len());
3535
#[cfg(not(target_os = "zkvm"))]
3636
{
3737
for w in words.iter_mut() {
@@ -51,7 +51,7 @@ impl WordRead for Reader {
5151
}
5252
let mut num_padded_bytes = bytes.len();
5353
#[cfg(target_os = "zkvm")]
54-
hint_buffer_u32!(bytes as *mut [u8] as *mut u32, num_padded_bytes / WORD_SIZE);
54+
hint_buffer_chunked(bytes.as_mut_ptr(), num_padded_bytes / WORD_SIZE);
5555
#[cfg(not(target_os = "zkvm"))]
5656
{
5757
let mut words = bytes.chunks_exact_mut(WORD_SIZE);

crates/toolchain/openvm/src/pal_abi.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
/// system operations in the same way: there is no operating system and even the standard
66
/// library should be directly handled with intrinsics.
77
use openvm_platform::{fileno::*, memory::sys_alloc_aligned, rust_rt::terminate, WORD_SIZE};
8-
use openvm_rv32im_guest::{hint_buffer_u32, hint_random, raw_print_str_from_bytes};
8+
use openvm_rv32im_guest::{hint_buffer_chunked, hint_random, raw_print_str_from_bytes};
99

1010
const DIGEST_WORDS: usize = 8;
1111

@@ -73,7 +73,7 @@ pub unsafe extern "C" fn sys_sha_buffer(
7373
#[no_mangle]
7474
pub unsafe extern "C" fn sys_rand(recv_buf: *mut u32, words: usize) {
7575
hint_random(words);
76-
hint_buffer_u32!(recv_buf, words);
76+
hint_buffer_chunked(recv_buf as *mut u8, words);
7777
}
7878

7979
/// # Safety

crates/vm/src/arch/execution.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ pub enum ExecutionError {
3838
DisabledOperation { pc: u32, opcode: VmOpcode },
3939
#[error("at pc = {pc}")]
4040
HintOutOfBounds { pc: u32 },
41+
#[error("at pc {pc}, hint buffer num_words {num_words} exceeds MAX_HINT_BUFFER_WORDS {max_hint_buffer_words}")]
42+
HintBufferTooLarge {
43+
pc: u32,
44+
num_words: u32,
45+
max_hint_buffer_words: u32,
46+
},
4147
#[error("at pc {pc}, tried to publish into index {public_value_index} when num_public_values = {num_public_values}")]
4248
PublicValueIndexOutOfBounds {
4349
pc: u32,

docs/vocs/docs/pages/specs/openvm/isa.mdx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ OpenVM depends on the following parameters, some of which are fixed and some of
3535
| `addr_space_height` | The base 2 log of the number of writable address spaces supported. | Configurable, must satisfy `addr_space_height <= F::bits() - 2` |
3636
| `pointer_max_bits` | The maximum number of bits in a pointer. | Configurable, must satisfy `pointer_max_bits <= F::bits() - 2` |
3737
| `num_public_values` | The number of user public values. | Configurable. If continuation is enabled, it must equal `8` times a power of two(which is nonzero). |
38+
| `MAX_HINT_BUFFER_WORDS_BITS` | The maximum number of bits for hint buffer word count. This determines `MAX_HINT_BUFFER_WORDS = 2^MAX_HINT_BUFFER_WORDS_BITS - 1` = 262,143 words (≈1MB), the maximum words per `HINT_BUFFER_RV32` instruction. | Fixed to 18. |
3839

3940
We explain these parameters in subsequent sections.
4041

@@ -428,9 +429,11 @@ with user input-output.
428429
| Name | Operands | Description |
429430
| ---------------- | --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
430431
| HINT_STOREW_RV32 | `_,b,_,1,2` | `[r32{0}(b):4]_2 = next 4 bytes from hint stream`. Only valid if next 4 values in hint stream are bytes. |
431-
| HINT_BUFFER_RV32 | `a,b,_,1,2` | `[r32{0}(b):4 * l]_2 = next 4 * l bytes from hint stream` where `l = r32{0}(a)`. Only valid if next `4 * l` values in hint stream are bytes. Very important: `l` should not be 0. The pointer address `r32{0}(b)` does not need to be a multiple of `4`. |
432+
| HINT_BUFFER_RV32 | `a,b,_,1,2` | `[r32{0}(b):4 * l]_2 = next 4 * l bytes from hint stream` where `l = r32{0}(a)`. Only valid if next `4 * l` values in hint stream are bytes. `l` must be non-zero and <= `MAX_HINT_BUFFER_WORDS` (262,143 words 1MB). The pointer address `r32{0}(b)` does not need to be a multiple of `4`. |
432433
| REVEAL_RV32 | `a,b,c,1,3,_,g` | Pseudo-instruction for `STOREW_RV32 a,b,c,1,3,_,g` writing to the user IO address space `3`. Only valid when continuations are enabled. |
433434

435+
> **Note:** The `MAX_HINT_BUFFER_WORDS` bound on `HINT_BUFFER_RV32` is enforced by both the executor and AIR constraints. The SDK's `hint_buffer_chunked` function automatically splits larger reads into multiple `HINT_BUFFER_RV32` instructions.
436+
434437
#### Phantom Sub-Instructions
435438

436439
The RV32IM extension defines the following phantom sub-instructions.

extensions/algebra/moduli-macros/src/lib.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -875,15 +875,15 @@ pub fn moduli_declare(input: TokenStream) -> TokenStream {
875875
}
876876
#[cfg(target_os = "zkvm")]
877877
{
878-
use ::openvm_algebra_guest::{openvm_custom_insn, openvm_rv32im_guest}; // needed for hint_store_u32! and hint_buffer_u32!
878+
use ::openvm_algebra_guest::{openvm_custom_insn, openvm_rv32im_guest}; // needed for hint_store_u32! and hint_buffer_chunked
879879

880880
let is_square = core::mem::MaybeUninit::<u32>::uninit();
881-
let sqrt = core::mem::MaybeUninit::<#struct_name>::uninit();
881+
let mut sqrt = core::mem::MaybeUninit::<#struct_name>::uninit();
882882
unsafe {
883883
#hint_sqrt_extern_func(self as *const #struct_name as usize);
884884
let is_square_ptr = is_square.as_ptr() as *const u32;
885885
openvm_rv32im_guest::hint_store_u32!(is_square_ptr);
886-
openvm_rv32im_guest::hint_buffer_u32!(sqrt.as_ptr() as *const u8, <#struct_name as ::openvm_algebra_guest::IntMod>::NUM_LIMBS / 4);
886+
openvm_rv32im_guest::hint_buffer_chunked(sqrt.as_mut_ptr() as *mut u8, <#struct_name as ::openvm_algebra_guest::IntMod>::NUM_LIMBS / 4 as usize);
887887
let is_square = is_square.assume_init();
888888
if is_square == 0 || is_square == 1 {
889889
Some((is_square == 1, sqrt.assume_init()))
@@ -902,14 +902,14 @@ pub fn moduli_declare(input: TokenStream) -> TokenStream {
902902
}
903903
#[cfg(target_os = "zkvm")]
904904
{
905-
use ::openvm_algebra_guest::{openvm_custom_insn, openvm_rv32im_guest}; // needed for hint_buffer_u32!
905+
use ::openvm_algebra_guest::{openvm_custom_insn, openvm_rv32im_guest}; // needed for hint_buffer_chunked
906906

907907
let mut non_qr_uninit = core::mem::MaybeUninit::<Self>::uninit();
908908
let mut non_qr;
909909
unsafe {
910910
#hint_non_qr_extern_func();
911-
let ptr = non_qr_uninit.as_ptr() as *const u8;
912-
openvm_rv32im_guest::hint_buffer_u32!(ptr, <Self as ::openvm_algebra_guest::IntMod>::NUM_LIMBS / 4);
911+
let ptr = non_qr_uninit.as_mut_ptr() as *mut u8;
912+
openvm_rv32im_guest::hint_buffer_chunked(ptr, <Self as ::openvm_algebra_guest::IntMod>::NUM_LIMBS / 4 as usize);
913913
non_qr = non_qr_uninit.assume_init();
914914
}
915915
// ensure non_qr < modulus

extensions/rv32im/circuit/cuda/src/hintstore.cu

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
using namespace riscv;
88
using namespace program;
9+
using hintstore::MAX_HINT_BUFFER_WORDS;
10+
using hintstore::MAX_HINT_BUFFER_WORDS_BITS;
911

1012
template <typename T> struct Rv32HintStoreCols {
1113
// common
@@ -87,11 +89,25 @@ struct Rv32HintStore {
8789
COL_WRITE_ARRAY(row, Rv32HintStoreCols, mem_ptr_limbs, mem_ptr_limbs);
8890

8991
if (local_idx == 0) {
92+
// The overflow check for mem_ptr + num_words * 4 is not needed because
93+
// 4 * MAX_HINT_BUFFER_WORDS < 2^pointer_max_bits guarantees no overflow
94+
assert(MAX_HINT_BUFFER_WORDS_BITS + 2 < pointer_max_bits);
95+
96+
// Range check for mem_ptr (using pointer_max_bits)
9097
uint32_t msl_rshift = (RV32_REGISTER_NUM_LIMBS - 1) * RV32_CELL_BITS;
9198
uint32_t msl_lshift = RV32_REGISTER_NUM_LIMBS * RV32_CELL_BITS - pointer_max_bits;
99+
100+
// Range check for num_words (using MAX_HINT_BUFFER_WORDS_BITS)
101+
// These constraints only work for MAX_HINT_BUFFER_WORDS_BITS in [16, 23]
102+
assert(MAX_HINT_BUFFER_WORDS_BITS >= 16 && MAX_HINT_BUFFER_WORDS_BITS <= 23);
103+
104+
assert(record.num_words <= MAX_HINT_BUFFER_WORDS);
105+
uint32_t rem_words_limb2_lshift = (RV32_REGISTER_NUM_LIMBS - 1) * RV32_CELL_BITS - MAX_HINT_BUFFER_WORDS_BITS;
106+
107+
// Combined range check for mem_ptr and num_words
92108
bitwise_lookup.add_range(
93109
(record.mem_ptr >> msl_rshift) << msl_lshift,
94-
(record.num_words >> msl_rshift) << msl_lshift
110+
((record.num_words >> 16) & 0xFF) << rem_words_limb2_lshift
95111
);
96112
mem_helper.fill(
97113
row.slice_from(COL_INDEX(Rv32HintStoreCols, mem_ptr_aux_cols)),

extensions/rv32im/circuit/src/hintstore/execution.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use openvm_instructions::{
1414
use openvm_rv32im_transpiler::{
1515
Rv32HintStoreOpcode,
1616
Rv32HintStoreOpcode::{HINT_BUFFER, HINT_STOREW},
17+
MAX_HINT_BUFFER_WORDS,
1718
};
1819
use openvm_stark_backend::p3_field::PrimeField32;
1920

@@ -172,6 +173,15 @@ unsafe fn execute_e12_impl<F: PrimeField32, CTX: ExecutionCtxTrait, const IS_HIN
172173
};
173174
debug_assert_ne!(num_words, 0);
174175

176+
// Bounds check: num_words must not exceed MAX_HINT_BUFFER_WORDS
177+
if num_words > MAX_HINT_BUFFER_WORDS as u32 {
178+
return Err(ExecutionError::HintBufferTooLarge {
179+
pc,
180+
num_words,
181+
max_hint_buffer_words: MAX_HINT_BUFFER_WORDS as u32,
182+
});
183+
}
184+
175185
if exec_state.streams.hint_stream.len() < RV32_REGISTER_NUM_LIMBS * num_words as usize {
176186
let err = ExecutionError::HintOutOfBounds { pc };
177187
return Err(err);

0 commit comments

Comments
 (0)