Skip to content

Commit

Permalink
fix: substraction between relocatables should be signed
Browse files Browse the repository at this point in the history
It is valid as per the reference implementation to substract two
relocatables `a - b` where `b.offset > a.offset`.
The fix consists in not using the trait as implemented for `Relocatable`
(where it represents a positive distance between pointers) but to
promote the offsets to `i128` and only substract them for
`MaybeRelocatable`, as those are the language-level types.
  • Loading branch information
Oppen committed Jun 9, 2023
1 parent 061b2c3 commit 23b1b99
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 1 deletion.
16 changes: 16 additions & 0 deletions cairo_programs/if_reloc_equal.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from starkware.cairo.common.alloc import alloc

func main{}() {
let arr: felt* = alloc();

assert arr[0] = 1;
assert arr[5] = 2;

let end = arr + 5;

if (arr == end) {
ret;
}

ret;
}
9 changes: 9 additions & 0 deletions src/tests/cairo_run_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,3 +967,12 @@ fn cairo_run_reduce() {
let program_data = include_bytes!("../../cairo_programs/reduce.json");
run_program_simple(program_data.as_slice());
}

#[test]
fn cairo_run_if_reloc_equal() {
const PROGRAM_DATA: &'static [u8] = include_bytes!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/cairo_programs/if_reloc_equal.json",
));
run_program_simple(PROGRAM_DATA);
}
4 changes: 3 additions & 1 deletion src/types/relocatable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ impl MaybeRelocatable {
MaybeRelocatable::RelocatableValue(rel_b),
) => {
if rel_a.segment_index == rel_b.segment_index {
return Ok(MaybeRelocatable::from(Felt252::new((*rel_a - *rel_b)?)));
return Ok(MaybeRelocatable::from(Felt252::from(
rel_a.offset as i128 - rel_b.offset as i128,
)));
}
Err(MathError::RelocatableSubDiffIndex(Box::new((
*rel_a, *rel_b,
Expand Down
4 changes: 4 additions & 0 deletions src/vm/vm_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ impl VirtualMachine {
) if !num_op1.is_zero() => {
Ok((Some(MaybeRelocatable::Int(num_dst / num_op1)), dst.cloned()))
}
// FIXME: missing case: div by 0 should return error
_ => Ok((None, None)),
},
_ => Ok((None, None)),
Expand Down Expand Up @@ -382,6 +383,8 @@ impl VirtualMachine {
Ok(())
}

// TODO: TEST: run_instruction with update JmpNz, operation Add, (dst, op1) = (Rel, Rel) and
// op1 > dst
fn run_instruction(&mut self, instruction: &Instruction) -> Result<(), VirtualMachineError> {
let (operands, operands_addresses, deduced_operands) =
self.compute_operands(instruction)?;
Expand Down Expand Up @@ -449,6 +452,7 @@ impl VirtualMachine {
*instruction = Some(self.decode_current_instruction()?);
}
let instruction = instruction.as_ref().unwrap();
dbg!(instruction);
if !self.skip_instruction_execution {
self.run_instruction(instruction)?;
} else {
Expand Down

0 comments on commit 23b1b99

Please sign in to comment.