Skip to content

Commit

Permalink
feat: better failure messages when reading too much input (#1950)
Browse files Browse the repository at this point in the history
Co-authored-by: Ratan Kaliani <[email protected]>
  • Loading branch information
nhtyy and ratankaliani authored Jan 17, 2025
1 parent 5f33dd5 commit df100c9
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 10 deletions.
6 changes: 2 additions & 4 deletions crates/core/executor/src/syscalls/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,8 @@ impl Syscall for HintLenSyscall {
_arg1: u32,
_arg2: u32,
) -> Option<u32> {
panic_if_input_exhausted(ctx);

// Note: This cast could be truncating
ctx.rt.state.input_stream.front().map(|data| data.len() as u32)
// Note: If the user supplies an input > than length 2^32, then the length returned will be truncated to 32-bits. Reading from the syscall will definitely fail in that case, as the BabyBear field is < 2^32.
Some(ctx.rt.state.input_stream.front().map_or(u32::MAX, |data| data.len() as u32))
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/zkvm/entrypoint/src/allocators/embedded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ pub fn init() {
}

pub fn used() -> usize {
critical_section::with(|cs| INNER_HEAP.used())
critical_section::with(|_cs| INNER_HEAP.used())
}

pub fn free() -> usize {
critical_section::with(|cs| INNER_HEAP.free())
critical_section::with(|_cs| INNER_HEAP.free())
}

struct EmbeddedAlloc;
Expand Down
8 changes: 8 additions & 0 deletions crates/zkvm/entrypoint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pub struct ReadVecResult {
/// When the `embedded` feature is enabled, the buffer is read into the reserved input region.
///
/// When there is no allocator selected, the program will fail to compile.
///
/// If the input stream is exhausted, the failed flag will be returned as true. In this case, the other outputs from the function are likely incorrect, which is fine as `sp1-lib` always panics in the case that the input stream is exhausted.
#[no_mangle]
pub extern "C" fn read_vec_raw() -> ReadVecResult {
#[cfg(not(target_os = "zkvm"))]
Expand All @@ -70,6 +72,12 @@ pub extern "C" fn read_vec_raw() -> ReadVecResult {
{
// Get the length of the input buffer.
let len = syscall_hint_len();

// If the length is u32::MAX, then the input stream is exhausted.
if len == usize::MAX {
return ReadVecResult { ptr: std::ptr::null_mut(), len: 0, capacity: 0 };
}

// Round up to multiple of 4 for whole-word alignment.
let capacity = (len + 3) / 4 * 4;

Expand Down
30 changes: 26 additions & 4 deletions crates/zkvm/lib/src/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,18 @@ impl Write for SyscallWriter {
/// ```ignore
/// let data: Vec<u8> = sp1_zkvm::io::read_vec();
/// ```
#[track_caller]
pub fn read_vec() -> Vec<u8> {
let ReadVecResult { ptr, len, capacity } = unsafe { read_vec_raw() };
// 1. `ptr` was allocated using alloc
// 2. Assume that the allocator in the VM doesn't deallocate in the input space.
// 3. Size and length are correct from above. Length is <= capacity.

if ptr.is_null() {
panic!(
"Tried to read from the input stream, but it was empty @ {} \n
Was the correct data written into SP1Stdin?",
std::panic::Location::caller()
)
}

unsafe { Vec::from_raw_parts(ptr, len, capacity) }
}

Expand All @@ -60,8 +67,23 @@ pub fn read_vec() -> Vec<u8> {
///
/// let data: MyStruct = sp1_zkvm::io::read();
/// ```
#[track_caller]
pub fn read<T: DeserializeOwned>() -> T {
let vec = read_vec();
let ReadVecResult { ptr, len, capacity } = unsafe { read_vec_raw() };

if ptr.is_null() {
panic!(
"Tried to read from the input stream, but it was empty @ {} \n
Was the correct data written into SP1Stdin?",
std::panic::Location::caller()
)
}

// 1. `ptr` was allocated using alloc
// 2. Assume that the allocator in the VM doesn't deallocate in the input space.
// 3. Size and length are correct from above. Length is <= capacity.
let vec = unsafe { Vec::from_raw_parts(ptr, len, capacity) };

bincode::deserialize(&vec).expect("deserialization failed")
}

Expand Down

0 comments on commit df100c9

Please sign in to comment.