Skip to content

Commit d23f7a6

Browse files
YairVaknin-starkwareYairVaknin-starkware
committed
Fix_temp_segment_chain_bug
1 parent e2c6c91 commit d23f7a6

File tree

2 files changed

+264
-0
lines changed

2 files changed

+264
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
#### Upcoming Changes
44

5+
* bugfix: Fix temp segment chain bug [#2195](https://github.com/lambdaclass/cairo-vm/pull/2195)
6+
57
* chore: Pin types-rs version to the one set in lockfile [#2140](https://github.com/lambdaclass/cairo-vm/pull/2140)
68

79
* BREAKING CHANGE: `get_prover_input_info()` now requires `&mut self` and takes ownershop on the trace instead of cloning it. [#2127](https://github.com/lambdaclass/cairo-vm/pull/2127)

‎vm/src/vm/vm_memory/memory.rs‎

Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,95 @@ impl Memory {
286286
}
287287
Ok(addr.into())
288288
}
289+
#[cfg(not(feature = "extensive_hints"))]
290+
fn flatten_relocation_rules(&mut self) -> Result<(), MemoryError> {
291+
let keys: Vec<usize> = self.relocation_rules.keys().copied().collect();
292+
let max_hops = self.relocation_rules.len().saturating_add(1);
293+
for key in keys {
294+
let mut dst = *self
295+
.relocation_rules
296+
.get(&key)
297+
.expect("key taken from keys vec must exist");
298+
299+
let mut hops = 0usize;
300+
while dst.segment_index < 0 {
301+
let next_key = (-(dst.segment_index + 1)) as usize;
302+
let next = *self.relocation_rules.get(&next_key).ok_or_else(|| {
303+
MemoryError::UnallocatedSegment(Box::new((next_key, self.temp_data.len())))
304+
})?;
305+
dst = (next + dst.offset).map_err(MemoryError::Math)?;
306+
hops += 1;
307+
if hops > max_hops {
308+
return Err(MemoryError::Relocation); // cycle guard
309+
}
310+
}
311+
self.relocation_rules.insert(key, dst);
312+
}
313+
Ok(())
314+
}
315+
316+
#[cfg(feature = "extensive_hints")]
317+
fn flatten_relocation_rules(&mut self) -> Result<(), MemoryError> {
318+
let keys: Vec<usize> = self.relocation_rules.keys().copied().collect();
319+
let max_hops = self.relocation_rules.len().saturating_add(1);
320+
for key in keys {
321+
let mut dst = self
322+
.relocation_rules
323+
.get(&key)
324+
.expect("key taken from keys vec must exist")
325+
.clone();
326+
327+
let mut hops = 0usize;
328+
loop {
329+
match dst {
330+
MaybeRelocatable::RelocatableValue(r) if r.segment_index < 0 => {
331+
let next_key = (-(r.segment_index + 1)) as usize;
332+
let next = self
333+
.relocation_rules
334+
.get(&next_key)
335+
.ok_or_else(|| {
336+
MemoryError::UnallocatedSegment(Box::new((
337+
next_key,
338+
self.temp_data.len(),
339+
)))
340+
})?
341+
.clone();
342+
343+
match next {
344+
MaybeRelocatable::RelocatableValue(nr) => {
345+
dst = MaybeRelocatable::RelocatableValue(
346+
(nr + r.offset).map_err(MemoryError::Math)?,
347+
);
348+
}
349+
MaybeRelocatable::Int(i) => {
350+
if r.offset != 0 {
351+
return Err(MemoryError::NonZeroOffset(r.offset));
352+
}
353+
dst = MaybeRelocatable::Int(i);
354+
}
355+
}
356+
}
357+
_ => break,
358+
}
359+
hops += 1;
360+
if hops > max_hops {
361+
return Err(MemoryError::Relocation);
362+
}
363+
}
364+
self.relocation_rules.insert(key, dst);
365+
}
366+
Ok(())
367+
}
289368

290369
/// Relocates the memory according to the relocation rules and clears `self.relocaction_rules`.
291370
pub fn relocate_memory(&mut self) -> Result<(), MemoryError> {
292371
if self.relocation_rules.is_empty() || self.temp_data.is_empty() {
293372
return Ok(());
294373
}
374+
375+
// flatten chains (temp->temp->...->real).
376+
self.flatten_relocation_rules()?;
377+
295378
// Relocate temporary addresses in memory
296379
for segment in self.data.iter_mut().chain(self.temp_data.iter_mut()) {
297380
for cell in segment.iter_mut() {
@@ -345,6 +428,7 @@ impl Memory {
345428
self.relocation_rules.clear();
346429
Ok(())
347430
}
431+
348432
/// Add a new relocation rule.
349433
///
350434
/// When using feature "extensive_hints" the destination is allowed to be an Integer (via
@@ -1971,4 +2055,182 @@ mod memory_tests {
19712055
])
19722056
)
19732057
}
2058+
2059+
#[test]
2060+
#[cfg(not(feature = "extensive_hints"))]
2061+
fn flatten_relocation_rules_chain_happy() {
2062+
let mut mem = Memory::new();
2063+
// temp segments just to keep indices sensible (not strictly required here)
2064+
mem.temp_data = vec![vec![], vec![]];
2065+
2066+
// key 0 -> (-2, 3) ; key 1 -> (10, 4)
2067+
// after flatten: key 0 -> (10, 7)
2068+
mem.relocation_rules.insert(0, Relocatable::from((-2, 3)));
2069+
mem.relocation_rules.insert(1, Relocatable::from((10, 4)));
2070+
// sanity: an unrelated rule stays as-is
2071+
mem.relocation_rules.insert(2, Relocatable::from((7, 1)));
2072+
2073+
assert_eq!(mem.flatten_relocation_rules(), Ok(()));
2074+
assert_eq!(
2075+
*mem.relocation_rules.get(&0).unwrap(),
2076+
Relocatable::from((10, 7))
2077+
);
2078+
assert_eq!(
2079+
*mem.relocation_rules.get(&1).unwrap(),
2080+
Relocatable::from((10, 4))
2081+
);
2082+
assert_eq!(
2083+
*mem.relocation_rules.get(&2).unwrap(),
2084+
Relocatable::from((7, 1))
2085+
);
2086+
}
2087+
2088+
#[test]
2089+
#[cfg(not(feature = "extensive_hints"))]
2090+
fn flatten_relocation_rules_cycle_err() {
2091+
let mut mem = Memory::new();
2092+
mem.temp_data = vec![vec![]];
2093+
2094+
// Self-loop: key 0 -> (-1, 0) (i.e., points back to itself)
2095+
mem.relocation_rules.insert(0, Relocatable::from((-1, 0)));
2096+
2097+
assert_eq!(mem.flatten_relocation_rules(), Err(MemoryError::Relocation));
2098+
}
2099+
2100+
#[test]
2101+
#[cfg(feature = "extensive_hints")]
2102+
fn flatten_relocation_rules_chain_happy_extensive_reloc_and_int() {
2103+
let mut mem = Memory::new();
2104+
mem.temp_data = vec![vec![], vec![], vec![], vec![], vec![]];
2105+
2106+
// ---- Chain A (ends at relocatable) ----
2107+
// key 0 -> (-2, 3) (=> key 1)
2108+
// key 1 -> (10, 4) (real)
2109+
// Expect: key 0 -> (10, 7)
2110+
mem.relocation_rules.insert(
2111+
0,
2112+
MaybeRelocatable::RelocatableValue(Relocatable::from((-2, 3))),
2113+
);
2114+
mem.relocation_rules.insert(
2115+
1,
2116+
MaybeRelocatable::RelocatableValue(Relocatable::from((10, 4))),
2117+
);
2118+
2119+
// ---- Chain B (ends at int) ----
2120+
// key 2 -> (-4, 0) (=> key 3)
2121+
// key 3 -> (-5, 0) (=> key 4)
2122+
// key 4 -> 99 (final Int)
2123+
// Expect: key 2 -> 99 (offset is zero along the way)
2124+
mem.relocation_rules.insert(
2125+
2,
2126+
MaybeRelocatable::RelocatableValue(Relocatable::from((-4, 0))),
2127+
);
2128+
mem.relocation_rules.insert(
2129+
3,
2130+
MaybeRelocatable::RelocatableValue(Relocatable::from((-5, 0))),
2131+
);
2132+
mem.relocation_rules
2133+
.insert(4, MaybeRelocatable::Int(Felt252::from(99)));
2134+
2135+
assert_eq!(mem.flatten_relocation_rules(), Ok(()));
2136+
2137+
assert_eq!(
2138+
mem.relocation_rules.get(&0),
2139+
Some(&MaybeRelocatable::RelocatableValue(Relocatable::from((
2140+
10, 7
2141+
))))
2142+
);
2143+
assert_eq!(
2144+
mem.relocation_rules.get(&1),
2145+
Some(&MaybeRelocatable::RelocatableValue(Relocatable::from((
2146+
10, 4
2147+
))))
2148+
);
2149+
assert_eq!(
2150+
mem.relocation_rules.get(&2),
2151+
Some(&MaybeRelocatable::Int(Felt252::from(99)))
2152+
);
2153+
}
2154+
2155+
#[test]
2156+
#[cfg(feature = "extensive_hints")]
2157+
fn flatten_relocation_rules_int_with_non_zero_offset_err() {
2158+
let mut mem = Memory::new();
2159+
// temp_data length only matters for error messages; keep it >= biggest temp key + 1
2160+
mem.temp_data = vec![vec![], vec![], vec![]];
2161+
2162+
// key 0 -> (-2, 5) (points to key 1, offset 5)
2163+
// key 1 -> (-3, 0) (points to key 2, offset 2)
2164+
// key 2 -> 7 (final Int)
2165+
mem.relocation_rules.insert(
2166+
0,
2167+
MaybeRelocatable::RelocatableValue(Relocatable::from((-2, 5))),
2168+
);
2169+
mem.relocation_rules.insert(
2170+
1,
2171+
MaybeRelocatable::RelocatableValue(Relocatable::from((-3, 0))),
2172+
);
2173+
mem.relocation_rules
2174+
.insert(2, MaybeRelocatable::Int(Felt252::from(7)));
2175+
2176+
assert_eq!(
2177+
mem.flatten_relocation_rules(),
2178+
Err(MemoryError::NonZeroOffset(5))
2179+
);
2180+
}
2181+
2182+
#[test]
2183+
#[cfg(feature = "extensive_hints")]
2184+
fn flatten_relocation_rules_cycle_err_extensive() {
2185+
let mut mem = Memory::new();
2186+
mem.temp_data = vec![vec![], vec![]];
2187+
2188+
// 2-cycle:
2189+
// key 0 -> (-1, 0) (points to key 0)
2190+
// key 1 -> (-2, 0) (points to key 1)
2191+
mem.relocation_rules.insert(
2192+
0,
2193+
MaybeRelocatable::RelocatableValue(Relocatable::from((-1, 0))),
2194+
);
2195+
mem.relocation_rules.insert(
2196+
1,
2197+
MaybeRelocatable::RelocatableValue(Relocatable::from((-2, 0))),
2198+
);
2199+
2200+
assert_eq!(mem.flatten_relocation_rules(), Err(MemoryError::Relocation));
2201+
}
2202+
#[test]
2203+
#[cfg(not(feature = "extensive_hints"))]
2204+
fn flatten_relocation_rules_missing_next_err() {
2205+
let mut mem = Memory::new();
2206+
mem.temp_data = vec![vec![], vec![], vec![]];
2207+
2208+
// key 0 -> (-4, 1) => next_key = -( -4 + 1 ) = 3
2209+
// No rule for key 3, so we expect UnallocatedSegment((3, 3)).
2210+
mem.relocation_rules.insert(0, Relocatable::from((-4, 1)));
2211+
2212+
assert_eq!(
2213+
mem.flatten_relocation_rules(),
2214+
Err(MemoryError::UnallocatedSegment(Box::new((3, 3))))
2215+
);
2216+
}
2217+
2218+
#[test]
2219+
#[cfg(feature = "extensive_hints")]
2220+
fn flatten_relocation_rules_missing_next_err_extensive() {
2221+
let mut mem = Memory::new();
2222+
mem.temp_data = vec![vec![], vec![], vec![]];
2223+
2224+
// key 0 -> (-4, 0) => next_key = 3
2225+
// No rule for key 3, so we expect UnallocatedSegment((3, 3)).
2226+
mem.relocation_rules.insert(
2227+
0,
2228+
MaybeRelocatable::RelocatableValue(Relocatable::from((-4, 0))),
2229+
);
2230+
2231+
assert_eq!(
2232+
mem.flatten_relocation_rules(),
2233+
Err(MemoryError::UnallocatedSegment(Box::new((3, 3))))
2234+
);
2235+
}
19742236
}

0 commit comments

Comments
 (0)