diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000000..e17edf5742 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,7 @@ +* Add traceback to VmException [#657](https://github.com/lambdaclass/cairo-rs/pull/657) + * Public API changes: + * `traceback` field added to `VmException` struct + * `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. diff --git a/README.md b/README.md index 17a4059e0f..71d5ab1504 100644 --- a/README.md +++ b/README.md @@ -205,6 +205,10 @@ StarkWare's STARK Math blog series: * Make the alloc functionality an internal feature of the VM rather than a hint. +### Changelog + +Keep track of the latest changes [here](CHANGELOG.md). + ## License [MIT](/LICENSE) diff --git a/src/cairo_run.rs b/src/cairo_run.rs index 3ba526425b..483af3ae16 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/types/instruction.rs b/src/types/instruction.rs index 7140c6ef0f..aec88a4de1 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,38 @@ impl Instruction { } } } + +// Returns True if the given instruction looks like a call instruction. +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) { + Ok(inst) => inst, + Err(_) => return false, + }; + 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 +} + +#[cfg(test)] +mod tests { + use crate::bigint; + + use super::*; + + #[test] + fn is_call_instruction_true() { + let encoded_instruction = bigint!(1226245742482522112_i64); + assert!(is_call_instruction(&encoded_instruction, Some(&bigint!(2)))); + } + #[test] + fn is_call_instruction_false() { + let encoded_instruction = bigint!(4612671187288031229_i64); + assert!(!is_call_instruction(&encoded_instruction, None)); + } +} diff --git a/src/vm/decoding/decoder.rs b/src/vm/decoding/decoder.rs index d408fc6a49..33d0274605 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 @@ -8,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; @@ -119,7 +118,7 @@ pub fn decode_instruction( off0, off1, off2, - imm, + imm: imm.cloned(), dst_register, op0_register, op1_addr, @@ -198,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/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 2ed601bebb..820c95caeb 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)] @@ -11,42 +14,70 @@ 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 { + pub fn from_vm_error( + runner: &CairoRunner, + vm: &VirtualMachine, + error: VirtualMachineError, + ) -> Self { + let pc = vm.run_context.pc.offset; let error_attr_value = get_error_attr_value(pc, runner); VmException { pc, - inst_location: get_location(&pc, runner), + inst_location: get_location(pc, runner), inner_exc: error, error_attr_value, + traceback: get_traceback(vm, runner), } } } 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 { +pub fn get_location(pc: usize, runner: &CairoRunner) -> Option { runner .program .instruction_locations .as_ref()? - .get(pc) + .get(&pc) .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=0:{})\n", traceback_pc.offset))), + None => traceback.push_str(&format!( + "Unknown location (pc=0:{})\n", + traceback_pc.offset + )), + } + } + (!traceback.is_empty()) + .then(|| 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 - 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 { @@ -69,6 +100,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) } @@ -77,7 +111,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 @@ -86,8 +120,11 @@ impl Location { } #[cfg(test)] 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; @@ -96,7 +133,7 @@ mod test { use super::*; #[test] fn get_vm_exception_from_vm_error() { - let pc = 1; + let pc = 0; let location = Location { end_line: 2, end_col: 2, @@ -115,9 +152,10 @@ 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,), vm_excep ) } @@ -156,7 +194,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") ) } @@ -170,11 +208,12 @@ mod test { Relocatable::from((0, 4)), ), error_attr_value: None, + traceback: None, }; 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)) @@ -193,11 +232,12 @@ 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(), 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)) @@ -226,11 +266,12 @@ mod test { Relocatable::from((0, 4)), ), error_attr_value: None, + traceback: None, }; 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)) @@ -271,11 +312,12 @@ mod test { Relocatable::from((0, 4)), ), error_attr_value: None, + traceback: None, }; 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))) ) ) @@ -325,7 +367,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] @@ -342,6 +384,48 @@ 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); + } + + #[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 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, &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)); + } + + #[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 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, &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/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 911bd01662..2b073c9191 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::{ @@ -26,6 +28,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, @@ -503,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)?; @@ -731,6 +734,51 @@ impl VirtualMachine { } } + // Returns the values (fp, pc) corresponding to each call instruction in the traceback. + // Returns the most recent call last. + 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 { + // 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)) { + 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)) { + true => ret_pc.sub(2).unwrap(), // This unwrap wont fail as it is checked before + false => break, + } + } + _ => break, + } + } + } + } + _ => break, + }; + // Append traceback entries + entries.push((fp, call_pc)) + } + entries.reverse(); + 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) @@ -953,6 +1001,7 @@ mod tests { bitwise_instance_def::BitwiseInstanceDef, ec_op_instance_def::EcOpInstanceDef, }, instruction::{Op1Addr, Register}, + program::Program, relocatable::Relocatable, }, utils::test_utils::*, @@ -963,8 +1012,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]; @@ -3854,4 +3904,48 @@ 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 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, &mut 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); + } + + #[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 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, &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); + } }