Add traceback to VmException (#657)

* Start get_traceback_entries + add convenience methos

* Add fn is_call_instruction

* add code

* Refactor code

* Clippy

* Add get_traceback method

* Fix get_error_attr_value

* Add traceback to VmException

* Make traceback non-optional

* Add tests for is_call_instruction

* Add traceback to error display

* Add test + fix logic for get_traceback_entries

* Code refactor

* Add one more test for get_traceback_entries

* Fix string format + add test for get_traceback

* Improve fn

* Add reference to is_call_instruction signature

* Add reference to immediate in decode_instruction + remove clone

* Fix hint_processor mutability in tests

* Add changelog

* Add PR link

* Remove redundant information

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<Location>` is now `pub fn get_location(pc: usize, runner: &CairoRunner) -> Option<Location>`
+        * `pub fn decode_instruction(encoded_instr: i64, mut imm: Option<BigInt>) -> Result<instruction::Instruction, VirtualMachineError>` is now `pub fn decode_instruction(encoded_instr: i64, mut imm: Option<&BigInt>) -> Result<instruction::Instruction, VirtualMachineError>`
+        * `VmExcepion` field's string format now mirror their cairo-lang conterparts.

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)

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()?;

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));
+    }
+}

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<BigInt>,
+    mut imm: Option<&BigInt>,
 ) -> Result<instruction::Instruction, VirtualMachineError> {
     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));

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<Location>,
     inner_exc: VirtualMachineError,
     error_attr_value: Option<String>,
+    traceback: Option<String>,
 }
 
 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<String> {
+    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<Location> {
+pub fn get_location(pc: usize, runner: &CairoRunner) -> Option<Location> {
     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<String> {
+    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));
     }
 }

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()