Skip to content

Commit

Permalink
fix: uninitialized_memory hashmap issue (#1536)
Browse files Browse the repository at this point in the history
  • Loading branch information
ctian1 authored Sep 24, 2024
2 parents 25cdde5 + faf120f commit fca5db0
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 30 deletions.
6 changes: 3 additions & 3 deletions crates/core/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ impl<'a> Executor<'a> {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Vacant(entry) => {
// If addr has a specific value to be initialized with, use that, otherwise 0.
let value = self.state.uninitialized_memory.get(&addr).unwrap_or(&0);
let value = self.state.uninitialized_memory.get(addr).unwrap_or(&0);
entry.insert(MemoryRecord { value: *value, shard: 0, timestamp: 0 })
}
};
Expand Down Expand Up @@ -428,7 +428,7 @@ impl<'a> Executor<'a> {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Vacant(entry) => {
// If addr has a specific value to be initialized with, use that, otherwise 0.
let value = self.state.uninitialized_memory.get(&addr).unwrap_or(&0);
let value = self.state.uninitialized_memory.get(addr).unwrap_or(&0);

entry.insert(MemoryRecord { value: *value, shard: 0, timestamp: 0 })
}
Expand Down Expand Up @@ -1373,7 +1373,7 @@ impl<'a> Executor<'a> {
// Program memory is initialized in the MemoryProgram chip and doesn't require any
// events, so we only send init events for other memory addresses.
if !self.record.program.memory_image.contains_key(&addr) {
let initial_value = self.state.uninitialized_memory.get(&addr).unwrap_or(&0);
let initial_value = self.state.uninitialized_memory.get(addr).unwrap_or(&0);
memory_initialize_events.push(MemoryInitializeFinalizeEvent::initialize(
addr,
*initial_value,
Expand Down
1 change: 0 additions & 1 deletion crates/core/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ mod report;
mod state;
pub mod subproof;
pub mod syscalls;
mod utils;

pub use context::*;
pub use executor::*;
Expand Down
9 changes: 9 additions & 0 deletions crates/core/executor/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,15 @@ impl<'a, V> Entry<'a, V> {
Entry::Occupied(entry) => entry.into_mut(),
}
}

/// Provides in-place mutable access to an occupied entry before any potential inserts into the map.
pub fn and_modify<F: FnOnce(&mut V)>(mut self, f: F) -> Self {
match &mut self {
Entry::Vacant(_) => {}
Entry::Occupied(entry) => f(entry.get_mut()),
}
self
}
}

/// A vacant entry of `PagedMemory`, for in-place manipulation.
Expand Down
10 changes: 2 additions & 8 deletions crates/core/executor/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::{
};

use hashbrown::HashMap;
use nohash_hasher::BuildNoHashHasher;
use serde::{Deserialize, Serialize};
use sp1_stark::{baby_bear_poseidon2::BabyBearPoseidon2, ShardProof, StarkVerifyingKey};

Expand All @@ -13,7 +12,6 @@ use crate::{
memory::PagedMemory,
record::{ExecutionRecord, MemoryAccessRecord},
syscalls::SyscallCode,
utils::{deserialize_hashmap_as_vec, serialize_hashmap_as_vec},
ExecutorMode,
};

Expand Down Expand Up @@ -43,11 +41,7 @@ pub struct ExecutionState {

/// Uninitialized memory addresses that have a specific value they should be initialized with.
/// `SyscallHintRead` uses this to write hint data into uninitialized memory.
#[serde(
serialize_with = "serialize_hashmap_as_vec",
deserialize_with = "deserialize_hashmap_as_vec"
)]
pub uninitialized_memory: HashMap<u32, u32, BuildNoHashHasher<u32>>,
pub uninitialized_memory: PagedMemory<u32>,

/// A stream of input values (global to the entire program).
pub input_stream: Vec<Vec<u8>>,
Expand Down Expand Up @@ -84,7 +78,7 @@ impl ExecutionState {
channel: 0,
pc: pc_start,
memory: PagedMemory::new_preallocated(),
uninitialized_memory: HashMap::default(),
uninitialized_memory: PagedMemory::default(),
input_stream: Vec::new(),
input_stream_ptr: 0,
public_values_stream: Vec::new(),
Expand Down
18 changes: 0 additions & 18 deletions crates/core/executor/src/utils.rs

This file was deleted.

0 comments on commit fca5db0

Please sign in to comment.