From c47e701cb3191b72ddbd0693b2ad58343da03ebb Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 11:29:02 -0300 Subject: [PATCH 01/22] Start get_traceback_entries + add convenience methos --- src/vm/vm_core.rs | 28 ++++++++++++++++++++++++++++ src/vm/vm_memory/memory.rs | 13 +++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 0a13450fc6..392b7d0822 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -26,6 +26,8 @@ use num_integer::Integer; use num_traits::{ToPrimitive, Zero}; use std::{any::Any, borrow::Cow, collections::HashMap}; +const MAX_TRACEBACK_ENTRIES: u32 = 20; + #[derive(PartialEq, Debug)] pub struct Operands { dst: MaybeRelocatable, @@ -727,6 +729,32 @@ impl VirtualMachine { } } + // Returns the values (fp, pc) corresponding to each call instruction in the traceback. + // Returns the most recent call last. + // The returned values consist of the offsets, not the full addresses as the index is constant + pub(crate) fn get_traceback_entries(&self) -> Vec<(usize, usize)> { + let entries = Vec::<(usize, usize)>::new(); + let fp = Relocatable::from((1, self.run_context.fp)); + for _ in 0..MAX_TRACEBACK_ENTRIES { + if let (Some(Ok(opt_fp)), Some(Ok(opt_ret_pc))) = ( + fp.sub(2) + .ok() + .map(|ref r| self.memory.get_owned_relocatable(r)), + fp.sub(1) + .ok() + .map(|ref r| self.memory.get_owned_relocatable(r)), + ) { + // Check that memory.get(fp -2) != fp + if opt_fp == fp { + break; + }; + } else { + break; + } + } + entries + } + ///Adds a new segment and to the VirtualMachine.memory returns its starting location as a RelocatableValue. pub fn add_memory_segment(&mut self) -> Relocatable { self.segments.add(&mut self.memory) diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index 60cae3a6b4..b0a716f921 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -263,6 +263,19 @@ impl Memory { } } + pub fn get_owned_relocatable( + &self, + key: &Relocatable, + ) -> Result { + match self.get(key).map_err(VirtualMachineError::MemoryError)? { + Some(Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel))) => Ok(rel.clone()), + Some(Cow::Owned(MaybeRelocatable::RelocatableValue(rel))) => Ok(rel), + _ => Err(VirtualMachineError::ExpectedRelocatable( + MaybeRelocatable::from(key), + )), + } + } + pub fn insert_value>( &mut self, key: &Relocatable, From 51219ca4f2b877f22abd1ed9900bf91bc23104ae Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 13:00:55 -0300 Subject: [PATCH 02/22] Add fn is_call_instruction --- src/types/instruction.rs | 20 ++++++++++++++++++++ src/vm/decoding/decoder.rs | 3 +-- src/vm/vm_core.rs | 11 ++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/types/instruction.rs b/src/types/instruction.rs index 7140c6ef0f..7372119f21 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -1,6 +1,9 @@ use num_bigint::BigInt; +use num_traits::ToPrimitive; use serde::Deserialize; +use crate::vm::decoding::decoder::decode_instruction; + #[derive(Deserialize, Debug, PartialEq, Eq, Clone)] pub enum Register { AP, @@ -78,3 +81,20 @@ impl Instruction { } } } + +// Returns True if the given instruction looks like a call instruction. +pub(crate) fn is_call_instruction(encoded_instruction: BigInt, imm: Option) -> bool { + let encoded_i64_instruction = match encoded_instruction.to_i64() { + Some(num) => num, + None => return false, + }; + let instruction = match decode_instruction(encoded_i64_instruction, imm) { + Ok(inst) => inst, + Err(_) => return false, + }; + return instruction.res == Res::Op1 + && (instruction.pc_update == PcUpdate::Jump || instruction.pc_update == PcUpdate::JumpRel) + && instruction.ap_update == ApUpdate::Add2 + && instruction.fp_update == FpUpdate::APPlus2 + && instruction.opcode == Opcode::Call; +} diff --git a/src/vm/decoding/decoder.rs b/src/vm/decoding/decoder.rs index d408fc6a49..432aeda40e 100644 --- a/src/vm/decoding/decoder.rs +++ b/src/vm/decoding/decoder.rs @@ -1,5 +1,4 @@ -use crate::types::instruction; -use crate::vm::errors::vm_errors::VirtualMachineError; +use crate::{types::instruction, vm::errors::vm_errors::VirtualMachineError}; use num_bigint::BigInt; // 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 392b7d0822..15fabc97c5 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -734,7 +734,7 @@ impl VirtualMachine { // The returned values consist of the offsets, not the full addresses as the index is constant pub(crate) fn get_traceback_entries(&self) -> Vec<(usize, usize)> { let entries = Vec::<(usize, usize)>::new(); - let fp = Relocatable::from((1, self.run_context.fp)); + let mut fp = Relocatable::from((1, self.run_context.fp)); for _ in 0..MAX_TRACEBACK_ENTRIES { if let (Some(Ok(opt_fp)), Some(Ok(opt_ret_pc))) = ( fp.sub(2) @@ -748,6 +748,15 @@ impl VirtualMachine { if opt_fp == fp { break; }; + fp = opt_fp; + let ret_pc = opt_ret_pc; + if let (Some(Ok(instruction0)), Some(Ok(instruction1))) = ( + ret_pc.sub(2).ok().map(|ref r| self.memory.get_integer(r)), + ret_pc.sub(1).ok().map(|ref r| self.memory.get_integer(r)), + ) { + } else { + break; + } } else { break; } From 3b276dbdfb224f713b44584e8810ad4ecc24654f Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 14:38:45 -0300 Subject: [PATCH 03/22] add code --- src/types/instruction.rs | 6 ++++-- src/vm/vm_core.rs | 37 ++++++++++++++++++++++++++++--------- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/types/instruction.rs b/src/types/instruction.rs index 7372119f21..cd4664bd5e 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use num_bigint::BigInt; use num_traits::ToPrimitive; use serde::Deserialize; @@ -83,8 +85,8 @@ impl Instruction { } // Returns True if the given instruction looks like a call instruction. -pub(crate) fn is_call_instruction(encoded_instruction: BigInt, imm: Option) -> bool { - let encoded_i64_instruction = match encoded_instruction.to_i64() { +pub(crate) fn is_call_instruction(encoded_instruction: &Cow, imm: Option) -> bool { + let encoded_i64_instruction: i64 = match encoded_instruction.to_i64() { Some(num) => num, None => return false, }; diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 15fabc97c5..ba734e8a56 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -9,7 +9,9 @@ use crate::{ serde::deserialize_program::{ApTracking, Attribute}, types::{ exec_scope::ExecutionScopes, - instruction::{ApUpdate, FpUpdate, Instruction, Opcode, PcUpdate, Res}, + instruction::{ + is_call_instruction, ApUpdate, FpUpdate, Instruction, Opcode, PcUpdate, Res, + }, relocatable::{MaybeRelocatable, Relocatable}, }, vm::{ @@ -731,13 +733,12 @@ impl VirtualMachine { // Returns the values (fp, pc) corresponding to each call instruction in the traceback. // Returns the most recent call last. - // The returned values consist of the offsets, not the full addresses as the index is constant - pub(crate) fn get_traceback_entries(&self) -> Vec<(usize, usize)> { - let entries = Vec::<(usize, usize)>::new(); + pub(crate) fn get_traceback_entries(&self) -> Vec<(Relocatable, Relocatable)> { + let mut entries = Vec::<(Relocatable, Relocatable)>::new(); let mut fp = Relocatable::from((1, self.run_context.fp)); for _ in 0..MAX_TRACEBACK_ENTRIES { if let (Some(Ok(opt_fp)), Some(Ok(opt_ret_pc))) = ( - fp.sub(2) + &fp.sub(2) .ok() .map(|ref r| self.memory.get_owned_relocatable(r)), fp.sub(1) @@ -750,10 +751,27 @@ impl VirtualMachine { }; fp = opt_fp; let ret_pc = opt_ret_pc; - if let (Some(Ok(instruction0)), Some(Ok(instruction1))) = ( - ret_pc.sub(2).ok().map(|ref r| self.memory.get_integer(r)), - ret_pc.sub(1).ok().map(|ref r| self.memory.get_integer(r)), - ) { + // Get the two memory cells before ret_pc. + if let Some(Ok(instruction1)) = + ret_pc.sub(1).ok().map(|ref r| self.memory.get_integer(r)) + { + // Try to check if the call instruction is (instruction0, instruction1) or just + // instruction1 (with no immediate). + let mut call_pc = Relocatable::from((9, 9)); // initial val + if is_call_instruction(&instruction1, None) { + call_pc = ret_pc.sub(1).unwrap(); // This unwrap wont fail as we checked it before + } else { + if let Some(Ok(instruction0)) = + ret_pc.sub(2).ok().map(|ref r| self.memory.get_integer(r)) + { + if is_call_instruction(&instruction0, Some(instruction1.into_owned())) { + call_pc = ret_pc.sub(2).unwrap(); // This unwrap wont fail as we checked it before + } else { + break; + } + } + } + entries.push((fp, call_pc)) } else { break; } @@ -761,6 +779,7 @@ impl VirtualMachine { break; } } + entries.reverse(); entries } From c89eb3f20837f94a8ce9a5ec73b65c54c3f66dab Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 15:04:58 -0300 Subject: [PATCH 04/22] Refactor code --- src/vm/vm_core.rs | 78 +++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 8035bc1cfb..c09bfd544f 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -740,48 +740,52 @@ impl VirtualMachine { pub(crate) fn get_traceback_entries(&self) -> Vec<(Relocatable, Relocatable)> { let mut entries = Vec::<(Relocatable, Relocatable)>::new(); let mut fp = Relocatable::from((1, self.run_context.fp)); + // Fetch the fp and pc traceback entries for _ in 0..MAX_TRACEBACK_ENTRIES { - if let (Some(Ok(opt_fp)), Some(Ok(opt_ret_pc))) = ( - &fp.sub(2) - .ok() - .map(|ref r| self.memory.get_owned_relocatable(r)), - fp.sub(1) - .ok() - .map(|ref r| self.memory.get_owned_relocatable(r)), - ) { - // Check that memory.get(fp -2) != fp - if opt_fp == fp { - break; - }; - fp = opt_fp; - let ret_pc = opt_ret_pc; - // Get the two memory cells before ret_pc. - if let Some(Ok(instruction1)) = - ret_pc.sub(1).ok().map(|ref r| self.memory.get_integer(r)) - { - // Try to check if the call instruction is (instruction0, instruction1) or just - // instruction1 (with no immediate). - let mut call_pc = Relocatable::from((9, 9)); // initial val - if is_call_instruction(&instruction1, None) { - call_pc = ret_pc.sub(1).unwrap(); // This unwrap wont fail as we checked it before - } else { - if let Some(Ok(instruction0)) = - ret_pc.sub(2).ok().map(|ref r| self.memory.get_integer(r)) - { - if is_call_instruction(&instruction0, Some(instruction1.into_owned())) { - call_pc = ret_pc.sub(2).unwrap(); // This unwrap wont fail as we checked it before - } else { - break; + // First we get the fp traceback + match fp + .sub(2) + .ok() + .map(|ref r| self.memory.get_owned_relocatable(r)) + { + Some(Ok(opt_fp)) if opt_fp != fp => fp = opt_fp, + _ => break, + } + // Then we get the pc traceback + let ret_pc = match fp + .sub(1) + .ok() + .map(|ref r| self.memory.get_owned_relocatable(r)) + { + Some(Ok(opt_pc)) => opt_pc, + _ => break, + }; + // Try to check if the call instruction is (instruction0, instruction1) or just + // instruction1 (with no immediate). + let call_pc = match ret_pc.sub(1).ok().map(|ref r| self.memory.get_integer(r)) { + Some(Ok(instruction1)) => { + match is_call_instruction(&instruction1, None) { + true => ret_pc.sub(1).unwrap(), // This unwrap wont fail as it is checked before + false => { + match ret_pc.sub(2).ok().map(|ref r| self.memory.get_integer(r)) { + Some(Ok(instruction0)) => { + match is_call_instruction( + &instruction0, + Some(instruction1.into_owned()), + ) { + true => ret_pc.sub(1).unwrap(), // This unwrap wont fail as it is checked before + false => break, + } + } + _ => break, } } } - entries.push((fp, call_pc)) - } else { - break; } - } else { - break; - } + _ => break, + }; + // Append traceback entries + entries.push((fp, call_pc)) } entries.reverse(); entries From f02db3ebe47e3bf253324b3f6f0b251bd76dbcee Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 15:08:00 -0300 Subject: [PATCH 05/22] Clippy --- src/types/instruction.rs | 6 +++--- src/vm/vm_core.rs | 12 ++---------- src/vm/vm_memory/memory.rs | 13 ------------- 3 files changed, 5 insertions(+), 26 deletions(-) diff --git a/src/types/instruction.rs b/src/types/instruction.rs index cd4664bd5e..4387f15943 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -85,7 +85,7 @@ impl Instruction { } // Returns True if the given instruction looks like a call instruction. -pub(crate) fn is_call_instruction(encoded_instruction: &Cow, imm: Option) -> bool { +pub(crate) fn is_call_instruction(encoded_instruction: &BigInt, imm: Option) -> bool { let encoded_i64_instruction: i64 = match encoded_instruction.to_i64() { Some(num) => num, None => return false, @@ -94,9 +94,9 @@ pub(crate) fn is_call_instruction(encoded_instruction: &Cow, imm: Option Ok(inst) => inst, Err(_) => return false, }; - return instruction.res == Res::Op1 + instruction.res == Res::Op1 && (instruction.pc_update == PcUpdate::Jump || instruction.pc_update == PcUpdate::JumpRel) && instruction.ap_update == ApUpdate::Add2 && instruction.fp_update == FpUpdate::APPlus2 - && instruction.opcode == Opcode::Call; + && instruction.opcode == Opcode::Call } diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index c09bfd544f..d9b075fb9f 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -743,20 +743,12 @@ impl VirtualMachine { // Fetch the fp and pc traceback entries for _ in 0..MAX_TRACEBACK_ENTRIES { // First we get the fp traceback - match fp - .sub(2) - .ok() - .map(|ref r| self.memory.get_owned_relocatable(r)) - { + match fp.sub(2).ok().map(|ref r| self.memory.get_relocatable(r)) { Some(Ok(opt_fp)) if opt_fp != fp => fp = opt_fp, _ => break, } // Then we get the pc traceback - let ret_pc = match fp - .sub(1) - .ok() - .map(|ref r| self.memory.get_owned_relocatable(r)) - { + let ret_pc = match fp.sub(1).ok().map(|ref r| self.memory.get_relocatable(r)) { Some(Ok(opt_pc)) => opt_pc, _ => break, }; diff --git a/src/vm/vm_memory/memory.rs b/src/vm/vm_memory/memory.rs index d3b720a251..d91b01cdba 100644 --- a/src/vm/vm_memory/memory.rs +++ b/src/vm/vm_memory/memory.rs @@ -260,19 +260,6 @@ impl Memory { } } - pub fn get_owned_relocatable( - &self, - key: &Relocatable, - ) -> Result { - match self.get(key).map_err(VirtualMachineError::MemoryError)? { - Some(Cow::Borrowed(MaybeRelocatable::RelocatableValue(rel))) => Ok(rel.clone()), - Some(Cow::Owned(MaybeRelocatable::RelocatableValue(rel))) => Ok(rel), - _ => Err(VirtualMachineError::ExpectedRelocatable( - MaybeRelocatable::from(key), - )), - } - } - pub fn insert_value>( &mut self, key: &Relocatable, From 05813bd60da7009e81d9269e5257ff84535ab88f Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 16:33:31 -0300 Subject: [PATCH 06/22] Add get_traceback method --- src/vm/errors/vm_exception.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 2ed601bebb..c30e1b8cf4 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -2,7 +2,10 @@ use std::fmt::{self, Display}; use thiserror::Error; -use crate::{serde::deserialize_program::Location, vm::runners::cairo_runner::CairoRunner}; +use crate::{ + serde::deserialize_program::Location, + vm::{runners::cairo_runner::CairoRunner, vm_core::VirtualMachine}, +}; use super::vm_errors::VirtualMachineError; #[derive(Debug, PartialEq, Error)] @@ -25,6 +28,7 @@ impl VmException { } } +//MISSING LOGIC HERE -> fp param pub fn get_error_attr_value(pc: usize, runner: &CairoRunner) -> Option { for attribute in &runner.program.error_message_attributes { if attribute.start_pc <= pc && attribute.end_pc > pc { @@ -43,6 +47,30 @@ pub fn get_location(pc: &usize, runner: &CairoRunner) -> Option { .cloned() } +// Returns the traceback at the current pc. +pub fn get_traceback(vm: &VirtualMachine, runner: &CairoRunner) -> Option { + let mut traceback = String::new(); + for (_fp, traceback_pc) in vm.get_traceback_entries() { + if let Some(ref attr) = get_error_attr_value(traceback_pc.offset, runner) { + traceback.push_str(attr) + } + match get_location(&traceback_pc.offset, runner) { + Some(location) => { + traceback.push_str(&location.to_string(&format!("(pc={})\n", traceback_pc.offset))) + } + None => traceback.push_str(&format!("Unknown location (pc={})\n", traceback_pc.offset)), + } + } + if traceback.is_empty() { + None + } else { + Some(format!( + "Cairo traceback (most recent call last):\n{}", + traceback + )) + } +} + impl Display for VmException { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // Build initial message From 58992c31b11826991d35e6c4b6fafca64271025c Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 16:43:41 -0300 Subject: [PATCH 07/22] Fix get_error_attr_value --- src/types/instruction.rs | 2 -- src/vm/errors/vm_exception.rs | 16 +++++----------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/types/instruction.rs b/src/types/instruction.rs index 4387f15943..5c98e94f3c 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -1,5 +1,3 @@ -use std::borrow::Cow; - use num_bigint::BigInt; use num_traits::ToPrimitive; use serde::Deserialize; diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index c30e1b8cf4..495bf0d9f6 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -28,14 +28,14 @@ impl VmException { } } -//MISSING LOGIC HERE -> fp param pub fn get_error_attr_value(pc: usize, runner: &CairoRunner) -> Option { + let mut errors = String::new(); for attribute in &runner.program.error_message_attributes { if attribute.start_pc <= pc && attribute.end_pc > pc { - return Some(format!("Error message: {}\n", attribute.value)); + errors.push_str(&format!("Error message: {}\n", attribute.value)); } } - None + (!errors.is_empty()).then(|| errors) } pub fn get_location(pc: &usize, runner: &CairoRunner) -> Option { @@ -61,14 +61,8 @@ pub fn get_traceback(vm: &VirtualMachine, runner: &CairoRunner) -> Option traceback.push_str(&format!("Unknown location (pc={})\n", traceback_pc.offset)), } } - if traceback.is_empty() { - None - } else { - Some(format!( - "Cairo traceback (most recent call last):\n{}", - traceback - )) - } + (!traceback.is_empty()) + .then(|| format!("Cairo traceback (most recent call last):\n{}", traceback)) } impl Display for VmException { From b3816246f9c316194f097e105ef4885c19bea399 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 17:02:17 -0300 Subject: [PATCH 08/22] Add traceback to VmException --- src/cairo_run.rs | 2 +- src/vm/errors/vm_exception.rs | 38 ++++++++++++++++++++++++++--------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/cairo_run.rs b/src/cairo_run.rs index 3c478f97d7..c99436b30b 100644 --- a/src/cairo_run.rs +++ b/src/cairo_run.rs @@ -34,7 +34,7 @@ pub fn cairo_run( cairo_runner .run_until_pc(end, &mut vm, hint_executor) - .map_err(|err| VmException::from_vm_error(&cairo_runner, err, vm.run_context.pc.offset))?; + .map_err(|err| VmException::from_vm_error(&cairo_runner, &vm, err))?; cairo_runner.end_run(false, false, &mut vm, hint_executor)?; vm.verify_auto_deductions()?; diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 495bf0d9f6..0f09378fa0 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -14,16 +14,23 @@ pub struct VmException { inst_location: Option, inner_exc: VirtualMachineError, error_attr_value: Option, + traceback: Option, } impl VmException { - pub fn from_vm_error(runner: &CairoRunner, error: VirtualMachineError, pc: usize) -> Self { - let error_attr_value = get_error_attr_value(pc, runner); + pub fn from_vm_error( + runner: &CairoRunner, + vm: &VirtualMachine, + error: VirtualMachineError, + with_traceback: bool, + ) -> Self { + let error_attr_value = get_error_attr_value(vm.run_context.pc.offset, runner); VmException { - pc, - inst_location: get_location(&pc, runner), + pc: vm.run_context.pc.offset, + inst_location: get_location(vm.run_context.pc.offset, runner), inner_exc: error, error_attr_value, + traceback: with_traceback.then(|| get_traceback(vm, runner)).flatten(), } } } @@ -38,12 +45,12 @@ pub fn get_error_attr_value(pc: usize, runner: &CairoRunner) -> Option { (!errors.is_empty()).then(|| errors) } -pub fn get_location(pc: &usize, runner: &CairoRunner) -> Option { +pub fn get_location(pc: usize, runner: &CairoRunner) -> Option { runner .program .instruction_locations .as_ref()? - .get(pc) + .get(&pc) .cloned() } @@ -54,7 +61,7 @@ pub fn get_traceback(vm: &VirtualMachine, runner: &CairoRunner) -> Option { traceback.push_str(&location.to_string(&format!("(pc={})\n", traceback_pc.offset))) } @@ -108,6 +115,7 @@ impl Location { } #[cfg(test)] mod test { + use num_bigint::{BigInt, Sign}; use std::collections::HashMap; use crate::serde::deserialize_program::{Attribute, InputFile}; @@ -137,9 +145,15 @@ mod test { inst_location: Some(location), inner_exc: VirtualMachineError::CouldntPopPositions, error_attr_value: None, + traceback: None, }; assert_eq!( - VmException::from_vm_error(&runner, VirtualMachineError::CouldntPopPositions, pc), + VmException::from_vm_error( + &runner, + &vm!(), + VirtualMachineError::CouldntPopPositions, + false + ), vm_excep ) } @@ -192,6 +206,7 @@ mod test { Relocatable::from((0, 4)), ), error_attr_value: None, + traceback: None, }; assert_eq!( vm_excep.to_string(), @@ -215,6 +230,7 @@ mod test { Relocatable::from((0, 4)), ), error_attr_value: Some(String::from("Error message: Block may fail\n")), + traceback: None, }; assert_eq!( vm_excep.to_string(), @@ -248,6 +264,7 @@ mod test { Relocatable::from((0, 4)), ), error_attr_value: None, + traceback: None, }; assert_eq!( vm_excep.to_string(), @@ -293,6 +310,7 @@ mod test { Relocatable::from((0, 4)), ), error_attr_value: None, + traceback: None, }; assert_eq!( vm_excep.to_string(), @@ -347,7 +365,7 @@ mod test { let program = program!(instruction_locations = Some(HashMap::from([(2, location.clone())])),); let runner = cairo_runner!(program); - assert_eq!(get_location(&2, &runner), Some(location)); + assert_eq!(get_location(2, &runner), Some(location)); } #[test] @@ -364,6 +382,6 @@ mod test { }; let program = program!(instruction_locations = Some(HashMap::from([(2, location)])),); let runner = cairo_runner!(program); - assert_eq!(get_location(&3, &runner), None); + assert_eq!(get_location(3, &runner), None); } } From 936eadce60a70c520d0d3b1c838d13a40c19fee9 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 17:05:43 -0300 Subject: [PATCH 09/22] Make traceback non-optional --- src/vm/errors/vm_exception.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 0f09378fa0..1a42649ca3 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -22,7 +22,6 @@ impl VmException { runner: &CairoRunner, vm: &VirtualMachine, error: VirtualMachineError, - with_traceback: bool, ) -> Self { let error_attr_value = get_error_attr_value(vm.run_context.pc.offset, runner); VmException { @@ -30,7 +29,7 @@ impl VmException { inst_location: get_location(vm.run_context.pc.offset, runner), inner_exc: error, error_attr_value, - traceback: with_traceback.then(|| get_traceback(vm, runner)).flatten(), + traceback: get_traceback(vm, runner), } } } @@ -148,12 +147,7 @@ mod test { traceback: None, }; assert_eq!( - VmException::from_vm_error( - &runner, - &vm!(), - VirtualMachineError::CouldntPopPositions, - false - ), + VmException::from_vm_error(&runner, &vm!(), VirtualMachineError::CouldntPopPositions,), vm_excep ) } From e72dff535c6ef48c96f1fca09ba92d8e7056bf43 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 18:04:16 -0300 Subject: [PATCH 10/22] Add tests for is_call_instruction --- src/types/instruction.rs | 18 ++++++++++++++++++ src/vm/errors/vm_exception.rs | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/types/instruction.rs b/src/types/instruction.rs index 5c98e94f3c..2a4a58beed 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -98,3 +98,21 @@ pub(crate) fn is_call_instruction(encoded_instruction: &BigInt, imm: Option Date: Thu, 22 Dec 2022 18:09:58 -0300 Subject: [PATCH 11/22] Add traceback to error display --- src/vm/errors/vm_exception.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 8c1bb9cc57..08ca153117 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -97,6 +97,9 @@ impl Display for VmException { } else { error_msg.push_str(&format!("{}\n", message)); } + if let Some(ref string) = self.traceback { + error_msg.push_str(&format!("{}\n", string)); + } // Write error message write!(f, "{}", error_msg) } From 67ccc395ec0e361dcd4cf7ab3bb1dcb3b559c6d1 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 18:45:01 -0300 Subject: [PATCH 12/22] Add test + fix logic for get_traceback_entries --- src/vm/vm_core.rs | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index d9b075fb9f..d206b9a370 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -743,12 +743,21 @@ impl VirtualMachine { // Fetch the fp and pc traceback entries for _ in 0..MAX_TRACEBACK_ENTRIES { // First we get the fp traceback - match fp.sub(2).ok().map(|ref r| self.memory.get_relocatable(r)) { + let initial_fp = fp; + match initial_fp + .sub(2) + .ok() + .map(|ref r| self.memory.get_relocatable(r)) + { Some(Ok(opt_fp)) if opt_fp != fp => fp = opt_fp, _ => break, } // Then we get the pc traceback - let ret_pc = match fp.sub(1).ok().map(|ref r| self.memory.get_relocatable(r)) { + let ret_pc = match initial_fp + .sub(1) + .ok() + .map(|ref r| self.memory.get_relocatable(r)) + { Some(Ok(opt_pc)) => opt_pc, _ => break, }; @@ -765,7 +774,7 @@ impl VirtualMachine { &instruction0, Some(instruction1.into_owned()), ) { - true => ret_pc.sub(1).unwrap(), // This unwrap wont fail as it is checked before + true => ret_pc.sub(2).unwrap(), // This unwrap wont fail as it is checked before false => break, } } @@ -1005,6 +1014,7 @@ mod tests { bitwise_instance_def::BitwiseInstanceDef, ec_op_instance_def::EcOpInstanceDef, }, instruction::{Op1Addr, Register}, + program::Program, relocatable::Relocatable, }, utils::test_utils::*, @@ -1015,8 +1025,9 @@ mod tests { }; use crate::bigint; + use crate::vm::runners::cairo_runner::CairoRunner; use num_bigint::Sign; - use std::collections::HashSet; + use std::{collections::HashSet, path::Path}; from_bigint_str![18, 75, 76]; @@ -3906,4 +3917,28 @@ mod tests { assert_eq!(vm.compute_effective_sizes(), &vec![4]); } + + #[test] + fn get_traceback_entries_bad_usort() { + let program = Program::from_file( + Path::new("cairo_programs/bad_programs/bad_usort.json"), + Some("main"), + ) + .expect("Call to `Program::from_file()` failed."); + + let hint_processor = BuiltinHintProcessor::new_empty(); + let mut cairo_runner = cairo_runner!(program, "all", false); + let mut vm = vm!(); + + let end = cairo_runner.initialize(&mut vm).unwrap(); + assert!(cairo_runner + .run_until_pc(end, &mut vm, &hint_processor) + .is_err()); + let expected_traceback = vec![ + (Relocatable::from((1, 3)), Relocatable::from((0, 97))), + (Relocatable::from((1, 14)), Relocatable::from((0, 30))), + (Relocatable::from((1, 26)), Relocatable::from((0, 60))), + ]; + assert_eq!(vm.get_traceback_entries(), expected_traceback); + } } From ba410b00c01b1e0032504e53c75416db952bec9a Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 18:48:12 -0300 Subject: [PATCH 13/22] Code refactor --- src/vm/vm_core.rs | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index d206b9a370..813b3e65d6 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -742,25 +742,16 @@ impl VirtualMachine { let mut fp = Relocatable::from((1, self.run_context.fp)); // Fetch the fp and pc traceback entries for _ in 0..MAX_TRACEBACK_ENTRIES { - // First we get the fp traceback - let initial_fp = fp; - match initial_fp - .sub(2) - .ok() - .map(|ref r| self.memory.get_relocatable(r)) - { - Some(Ok(opt_fp)) if opt_fp != fp => fp = opt_fp, - _ => break, - } - // Then we get the pc traceback - let ret_pc = match initial_fp - .sub(1) - .ok() - .map(|ref r| self.memory.get_relocatable(r)) - { + // Get return pc + let ret_pc = match fp.sub(1).ok().map(|ref r| self.memory.get_relocatable(r)) { Some(Ok(opt_pc)) => opt_pc, _ => break, }; + // Get fp traceback + match fp.sub(2).ok().map(|ref r| self.memory.get_relocatable(r)) { + Some(Ok(opt_fp)) if opt_fp != fp => fp = opt_fp, + _ => break, + } // Try to check if the call instruction is (instruction0, instruction1) or just // instruction1 (with no immediate). let call_pc = match ret_pc.sub(1).ok().map(|ref r| self.memory.get_integer(r)) { From acf3299cc1ccd62d499a4edc42b2dcc39ac8c254 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 18:50:40 -0300 Subject: [PATCH 14/22] Add one more test for get_traceback_entries --- src/vm/vm_core.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 813b3e65d6..8979f2bfbe 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -3932,4 +3932,24 @@ mod tests { ]; assert_eq!(vm.get_traceback_entries(), expected_traceback); } + + #[test] + fn get_traceback_entries_bad_dict_update() { + let program = Program::from_file( + Path::new("cairo_programs/bad_programs/bad_dict_update.json"), + Some("main"), + ) + .expect("Call to `Program::from_file()` failed."); + + let hint_processor = BuiltinHintProcessor::new_empty(); + let mut cairo_runner = cairo_runner!(program, "all", false); + let mut vm = vm!(); + + let end = cairo_runner.initialize(&mut vm).unwrap(); + assert!(cairo_runner + .run_until_pc(end, &mut vm, &hint_processor) + .is_err()); + let expected_traceback = vec![(Relocatable::from((1, 2)), Relocatable::from((0, 34)))]; + assert_eq!(vm.get_traceback_entries(), expected_traceback); + } } From bc66d0f018266f574f739814984db9e90097a5f8 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 22 Dec 2022 19:12:00 -0300 Subject: [PATCH 15/22] Fix string format + add test for get_traceback --- src/vm/errors/vm_exception.rs | 68 +++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 08ca153117..09fdb79f63 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -61,10 +61,12 @@ pub fn get_traceback(vm: &VirtualMachine, runner: &CairoRunner) -> Option { - traceback.push_str(&location.to_string(&format!("(pc={})\n", traceback_pc.offset))) - } - None => traceback.push_str(&format!("Unknown location (pc={})\n", traceback_pc.offset)), + Some(location) => traceback + .push_str(&location.to_string(&format!("(pc=0:{})\n", traceback_pc.offset))), + None => traceback.push_str(&format!( + "Unknown location (pc=0:{})\n", + traceback_pc.offset + )), } } (!traceback.is_empty()) @@ -74,7 +76,7 @@ pub fn get_traceback(vm: &VirtualMachine, runner: &CairoRunner) -> Option fmt::Result { // Build initial message - let message = format!("Error at pc={}:\n{}", self.pc, self.inner_exc); + let message = format!("Error at pc=0:{}:\n{}", self.pc, self.inner_exc); let mut error_msg = String::new(); // Add error attribute value if let Some(ref string) = self.error_attr_value { @@ -108,7 +110,7 @@ impl Display for VmException { impl Location { /// Prints the location with the passed message. pub fn to_string(&self, message: &String) -> String { - let msg_prefix = if message.is_empty() { "" } else { ":" }; + let msg_prefix = if message.is_empty() { "" } else { ": " }; format!( "{}:{}:{}{}{}", self.input_file.filename, self.start_line, self.start_col, msg_prefix, message @@ -119,7 +121,9 @@ impl Location { mod test { use num_bigint::{BigInt, Sign}; use std::collections::HashMap; + use std::path::Path; + use crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor; use crate::serde::deserialize_program::{Attribute, InputFile}; use crate::types::program::Program; use crate::types::relocatable::Relocatable; @@ -189,7 +193,7 @@ mod test { let message = String::from("While expanding the reference"); assert_eq!( location.to_string(&message), - String::from("Folder/file.cairo:1:1:While expanding the reference") + String::from("Folder/file.cairo:1:1: While expanding the reference") ) } @@ -208,7 +212,7 @@ mod test { assert_eq!( vm_excep.to_string(), format!( - "Error at pc=2:\n{}\n", + "Error at pc=0:2:\n{}\n", VirtualMachineError::FailedToComputeOperands( "op0".to_string(), Relocatable::from((0, 4)) @@ -232,7 +236,7 @@ mod test { assert_eq!( vm_excep.to_string(), format!( - "Error message: Block may fail\nError at pc=2:\n{}\n", + "Error message: Block may fail\nError at pc=0:2:\n{}\n", VirtualMachineError::FailedToComputeOperands( "op0".to_string(), Relocatable::from((0, 4)) @@ -266,7 +270,7 @@ mod test { assert_eq!( vm_excep.to_string(), format!( - "Folder/file.cairo:1:1:Error at pc=2:\n{}\n", + "Folder/file.cairo:1:1: Error at pc=0:2:\n{}\n", VirtualMachineError::FailedToComputeOperands( "op0".to_string(), Relocatable::from((0, 4)) @@ -312,7 +316,7 @@ mod test { assert_eq!( vm_excep.to_string(), format!( - "Folder/file_b.cairo:2:2:While expanding the reference:\nFolder/file.cairo:1:1:Error at pc=2:\n{}\n", + "Folder/file_b.cairo:2:2: While expanding the reference:\nFolder/file.cairo:1:1: Error at pc=0:2:\n{}\n", VirtualMachineError::FailedToComputeOperands("op0".to_string(), Relocatable::from((0, 4))) ) ) @@ -381,4 +385,46 @@ mod test { let runner = cairo_runner!(program); assert_eq!(get_location(3, &runner), None); } + + #[test] + // TEST CASE WITHOUT FILE CONTENTS + fn get_traceback_bad_dict_update() { + let program = Program::from_file( + Path::new("cairo_programs/bad_programs/bad_dict_update.json"), + Some("main"), + ) + .expect("Call to `Program::from_file()` failed."); + + let hint_processor = BuiltinHintProcessor::new_empty(); + let mut cairo_runner = cairo_runner!(program, "all", false); + let mut vm = vm!(); + + let end = cairo_runner.initialize(&mut vm).unwrap(); + assert!(cairo_runner + .run_until_pc(end, &mut vm, &hint_processor) + .is_err()); + let expected_traceback = String::from("Cairo traceback (most recent call last):\ncairo_programs/bad_programs/bad_dict_update.cairo:10:5: (pc=0:34)\n"); + assert_eq!(get_traceback(&vm, &cairo_runner), Some(expected_traceback)); + } + + #[test] + // TEST CASE WITHOUT FILE CONTENTS + fn get_traceback_bad_usort() { + let program = Program::from_file( + Path::new("cairo_programs/bad_programs/bad_usort.json"), + Some("main"), + ) + .expect("Call to `Program::from_file()` failed."); + + let hint_processor = BuiltinHintProcessor::new_empty(); + let mut cairo_runner = cairo_runner!(program, "all", false); + let mut vm = vm!(); + + let end = cairo_runner.initialize(&mut vm).unwrap(); + assert!(cairo_runner + .run_until_pc(end, &mut vm, &hint_processor) + .is_err()); + let expected_traceback = String::from("Cairo traceback (most recent call last):\ncairo_programs/bad_programs/bad_usort.cairo:91:48: (pc=0:97)\ncairo_programs/bad_programs/bad_usort.cairo:36:5: (pc=0:30)\ncairo_programs/bad_programs/bad_usort.cairo:64:5: (pc=0:60)\n"); + assert_eq!(get_traceback(&vm, &cairo_runner), Some(expected_traceback)); + } } From b3952cb08febb6b2e369522d9323ff3de438783d Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 27 Dec 2022 11:26:26 -0300 Subject: [PATCH 16/22] Improve fn --- src/vm/errors/vm_exception.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 09fdb79f63..b0ff1b1d92 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -23,10 +23,11 @@ impl VmException { vm: &VirtualMachine, error: VirtualMachineError, ) -> Self { - let error_attr_value = get_error_attr_value(vm.run_context.pc.offset, runner); + let pc = vm.run_context.pc.offset; + let error_attr_value = get_error_attr_value(pc, runner); VmException { - pc: vm.run_context.pc.offset, - inst_location: get_location(vm.run_context.pc.offset, runner), + pc, + inst_location: get_location(pc, runner), inner_exc: error, error_attr_value, traceback: get_traceback(vm, runner), From c2fa8a0f0f8e5f4b8f8427b59eb3a5a29e9571bb Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 27 Dec 2022 11:31:56 -0300 Subject: [PATCH 17/22] Add reference to is_call_instruction signature --- src/types/instruction.rs | 6 +++--- src/vm/vm_core.rs | 5 +---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/types/instruction.rs b/src/types/instruction.rs index 2a4a58beed..152fee1a52 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -83,12 +83,12 @@ impl Instruction { } // Returns True if the given instruction looks like a call instruction. -pub(crate) fn is_call_instruction(encoded_instruction: &BigInt, imm: Option) -> bool { +pub(crate) fn is_call_instruction(encoded_instruction: &BigInt, imm: Option<&BigInt>) -> bool { let encoded_i64_instruction: i64 = match encoded_instruction.to_i64() { Some(num) => num, None => return false, }; - let instruction = match decode_instruction(encoded_i64_instruction, imm) { + let instruction = match decode_instruction(encoded_i64_instruction, imm.cloned()) { Ok(inst) => inst, Err(_) => return false, }; @@ -108,7 +108,7 @@ mod tests { #[test] fn is_call_instruction_true() { let encoded_instruction = bigint!(1226245742482522112_i64); - assert!(is_call_instruction(&encoded_instruction, Some(bigint!(2)))); + assert!(is_call_instruction(&encoded_instruction, Some(&bigint!(2)))); } #[test] fn is_call_instruction_false() { diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 8979f2bfbe..9cea35ce4d 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -761,10 +761,7 @@ impl VirtualMachine { false => { match ret_pc.sub(2).ok().map(|ref r| self.memory.get_integer(r)) { Some(Ok(instruction0)) => { - match is_call_instruction( - &instruction0, - Some(instruction1.into_owned()), - ) { + match is_call_instruction(&instruction0, Some(&instruction1)) { true => ret_pc.sub(2).unwrap(), // This unwrap wont fail as it is checked before false => break, } From 88e4b528d47ccfa1040d97cda4eb0de6803cf88e Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 27 Dec 2022 11:39:51 -0300 Subject: [PATCH 18/22] Add reference to immediate in decode_instruction + remove clone --- src/types/instruction.rs | 2 +- src/vm/decoding/decoder.rs | 6 +++--- src/vm/trace/mod.rs | 2 +- src/vm/vm_core.rs | 3 +-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/types/instruction.rs b/src/types/instruction.rs index 152fee1a52..aec88a4de1 100644 --- a/src/types/instruction.rs +++ b/src/types/instruction.rs @@ -88,7 +88,7 @@ pub(crate) fn is_call_instruction(encoded_instruction: &BigInt, imm: Option<&Big Some(num) => num, None => return false, }; - let instruction = match decode_instruction(encoded_i64_instruction, imm.cloned()) { + let instruction = match decode_instruction(encoded_i64_instruction, imm) { Ok(inst) => inst, Err(_) => return false, }; diff --git a/src/vm/decoding/decoder.rs b/src/vm/decoding/decoder.rs index 432aeda40e..33d0274605 100644 --- a/src/vm/decoding/decoder.rs +++ b/src/vm/decoding/decoder.rs @@ -7,7 +7,7 @@ use num_bigint::BigInt; /// Decodes an instruction. The encoding is little endian, so flags go from bit 63 to 48. pub fn decode_instruction( encoded_instr: i64, - mut imm: Option, + mut imm: Option<&BigInt>, ) -> Result { const DST_REG_MASK: i64 = 0x0001; const DST_REG_OFF: i64 = 0; @@ -118,7 +118,7 @@ pub fn decode_instruction( off0, off1, off2, - imm, + imm: imm.cloned(), dst_register, op0_register, op1_addr, @@ -197,7 +197,7 @@ mod decoder_test { // | CALL| ADD| JUMP| ADD| IMM| FP| FP // 0 0 0 1 0 1 0 0 1 0 1 0 0 1 1 1 // 0001 0100 1010 0111 = 0x14A7; offx = 0 - let inst = decode_instruction(0x14A7800080008000, Some(bigint!(7))).unwrap(); + let inst = decode_instruction(0x14A7800080008000, Some(&bigint!(7))).unwrap(); assert!(matches!(inst.dst_register, instruction::Register::FP)); assert!(matches!(inst.op0_register, instruction::Register::FP)); assert!(matches!(inst.op1_addr, instruction::Op1Addr::Imm)); diff --git a/src/vm/trace/mod.rs b/src/vm/trace/mod.rs index aa60d6da4e..26a0130cc5 100644 --- a/src/vm/trace/mod.rs +++ b/src/vm/trace/mod.rs @@ -34,7 +34,7 @@ pub fn get_perm_range_check_limits( }) .transpose()?; - let decoded_instruction = decode_instruction(instruction, immediate)?; + let decoded_instruction = decode_instruction(instruction, immediate.as_ref())?; let off0 = decoded_instruction .off0 .to_isize() diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 9cea35ce4d..1c09f2973c 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -507,8 +507,7 @@ impl VirtualMachine { match instruction_ref.to_i64() { Some(instruction) => { if let Some(MaybeRelocatable::Int(imm_ref)) = imm.as_ref().map(|x| x.as_ref()) { - let decoded_instruction = - decode_instruction(instruction, Some(imm_ref.clone()))?; + let decoded_instruction = decode_instruction(instruction, Some(imm_ref))?; return Ok(decoded_instruction); } let decoded_instruction = decode_instruction(instruction, None)?; From 371dc43dcc2605725734514ee37db4404054f44f Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 27 Dec 2022 11:48:33 -0300 Subject: [PATCH 19/22] Fix hint_processor mutability in tests --- src/vm/errors/vm_exception.rs | 8 ++++---- src/vm/vm_core.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index b0ff1b1d92..820c95caeb 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -396,13 +396,13 @@ mod test { ) .expect("Call to `Program::from_file()` failed."); - let hint_processor = BuiltinHintProcessor::new_empty(); + let mut hint_processor = BuiltinHintProcessor::new_empty(); let mut cairo_runner = cairo_runner!(program, "all", false); let mut vm = vm!(); let end = cairo_runner.initialize(&mut vm).unwrap(); assert!(cairo_runner - .run_until_pc(end, &mut vm, &hint_processor) + .run_until_pc(end, &mut vm, &mut hint_processor) .is_err()); let expected_traceback = String::from("Cairo traceback (most recent call last):\ncairo_programs/bad_programs/bad_dict_update.cairo:10:5: (pc=0:34)\n"); assert_eq!(get_traceback(&vm, &cairo_runner), Some(expected_traceback)); @@ -417,13 +417,13 @@ mod test { ) .expect("Call to `Program::from_file()` failed."); - let hint_processor = BuiltinHintProcessor::new_empty(); + let mut hint_processor = BuiltinHintProcessor::new_empty(); let mut cairo_runner = cairo_runner!(program, "all", false); let mut vm = vm!(); let end = cairo_runner.initialize(&mut vm).unwrap(); assert!(cairo_runner - .run_until_pc(end, &mut vm, &hint_processor) + .run_until_pc(end, &mut vm, &mut hint_processor) .is_err()); let expected_traceback = String::from("Cairo traceback (most recent call last):\ncairo_programs/bad_programs/bad_usort.cairo:91:48: (pc=0:97)\ncairo_programs/bad_programs/bad_usort.cairo:36:5: (pc=0:30)\ncairo_programs/bad_programs/bad_usort.cairo:64:5: (pc=0:60)\n"); assert_eq!(get_traceback(&vm, &cairo_runner), Some(expected_traceback)); diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 5bc683f811..2b073c9191 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -3913,13 +3913,13 @@ mod tests { ) .expect("Call to `Program::from_file()` failed."); - let hint_processor = BuiltinHintProcessor::new_empty(); + let mut hint_processor = BuiltinHintProcessor::new_empty(); let mut cairo_runner = cairo_runner!(program, "all", false); let mut vm = vm!(); let end = cairo_runner.initialize(&mut vm).unwrap(); assert!(cairo_runner - .run_until_pc(end, &mut vm, &hint_processor) + .run_until_pc(end, &mut vm, &mut hint_processor) .is_err()); let expected_traceback = vec![ (Relocatable::from((1, 3)), Relocatable::from((0, 97))), @@ -3937,13 +3937,13 @@ mod tests { ) .expect("Call to `Program::from_file()` failed."); - let hint_processor = BuiltinHintProcessor::new_empty(); + let mut hint_processor = BuiltinHintProcessor::new_empty(); let mut cairo_runner = cairo_runner!(program, "all", false); let mut vm = vm!(); let end = cairo_runner.initialize(&mut vm).unwrap(); assert!(cairo_runner - .run_until_pc(end, &mut vm, &hint_processor) + .run_until_pc(end, &mut vm, &mut hint_processor) .is_err()); let expected_traceback = vec![(Relocatable::from((1, 2)), Relocatable::from((0, 34)))]; assert_eq!(vm.get_traceback_entries(), expected_traceback); From 61f8f4c235c089740df78e16c2186833b7eddaa2 Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 28 Dec 2022 12:52:28 -0300 Subject: [PATCH 20/22] Add changelog --- CHANGELOG.md | 10 ++++++++++ README.md | 3 +++ 2 files changed, 13 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000000..9eddebb417 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ +2022-12-28 +* Add traceback to VmException + * PR: #657 + * Main functionality changes: `VmException` now contains a traceback in the form of a String which lists the locations (consisting of filename, line, column and pc) of the calls leading to the error. + * Public API changes: + * `traceback` field added to `VmException` struct + * `VmException::from_vm_error()` signature changed from `pub fn from_vm_error(runner: &CairoRunner, error: VirtualMachineError, pc: usize) -> Self` to `pub fn from_vm_error(runner: &CairoRunner, vm: &VirtualMachine, error: VirtualMachineError) -> Self` + * `get_location()` signature changed from `pub fn get_location(pc: &usize, runner: &CairoRunner) -> Option` to `pub fn get_location(pc: usize, runner: &CairoRunner) -> Option` + * `decode_instruction()` signature changed from `pub fn decode_instruction(encoded_instr: i64, mut imm: Option) -> Result` to `pub fn decode_instruction(encoded_instr: i64, mut imm: Option<&BigInt>) -> Result` + * Minor changes in `VmExcepion` field's string format to mirror their cairo-lang conterparts. diff --git a/README.md b/README.md index 33b0e34ef7..8b6867fd9d 100644 --- a/README.md +++ b/README.md @@ -163,3 +163,6 @@ StarkWare's STARK Math blog series: ### Possible changes for the future * Make the alloc functionality an internal feature of the VM rather than a hint. + +### Changelog +Keep track of the latest changes [here](CHANGELOG.md). From 54969d3fb1e844fe3f2cd42176b6877f5d7e0c87 Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 28 Dec 2022 12:59:56 -0300 Subject: [PATCH 21/22] Add PR link --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9eddebb417..4d3d3fa717 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ 2022-12-28 * Add traceback to VmException - * PR: #657 + * PR: [#657](https://github.com/lambdaclass/cairo-rs/pull/657) * Main functionality changes: `VmException` now contains a traceback in the form of a String which lists the locations (consisting of filename, line, column and pc) of the calls leading to the error. * Public API changes: * `traceback` field added to `VmException` struct From 74239cb556f609d8546b47d2b4291574e7750882 Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 28 Dec 2022 13:35:05 -0300 Subject: [PATCH 22/22] Remove redundant information --- CHANGELOG.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d3d3fa717..e17edf5742 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,7 @@ -2022-12-28 -* Add traceback to VmException - * PR: [#657](https://github.com/lambdaclass/cairo-rs/pull/657) - * Main functionality changes: `VmException` now contains a traceback in the form of a String which lists the locations (consisting of filename, line, column and pc) of the calls leading to the error. +* Add traceback to VmException [#657](https://github.com/lambdaclass/cairo-rs/pull/657) * Public API changes: * `traceback` field added to `VmException` struct - * `VmException::from_vm_error()` signature changed from `pub fn from_vm_error(runner: &CairoRunner, error: VirtualMachineError, pc: usize) -> Self` to `pub fn from_vm_error(runner: &CairoRunner, vm: &VirtualMachine, error: VirtualMachineError) -> Self` - * `get_location()` signature changed from `pub fn get_location(pc: &usize, runner: &CairoRunner) -> Option` to `pub fn get_location(pc: usize, runner: &CairoRunner) -> Option` - * `decode_instruction()` signature changed from `pub fn decode_instruction(encoded_instr: i64, mut imm: Option) -> Result` to `pub fn decode_instruction(encoded_instr: i64, mut imm: Option<&BigInt>) -> Result` - * Minor changes in `VmExcepion` field's string format to mirror their cairo-lang conterparts. + * `pub fn from_vm_error(runner: &CairoRunner, error: VirtualMachineError, pc: usize) -> Self` is now `pub fn from_vm_error(runner: &CairoRunner, vm: &VirtualMachine, error: VirtualMachineError) -> Self` + * `pub fn get_location(pc: &usize, runner: &CairoRunner) -> Option` is now `pub fn get_location(pc: usize, runner: &CairoRunner) -> Option` + * `pub fn decode_instruction(encoded_instr: i64, mut imm: Option) -> Result` is now `pub fn decode_instruction(encoded_instr: i64, mut imm: Option<&BigInt>) -> Result` + * `VmExcepion` field's string format now mirror their cairo-lang conterparts.