From 168bbacf3efe4b9b62253decdfa98ac34b5efb7c Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 30 Nov 2023 12:48:24 -0300 Subject: [PATCH 1/7] Handle pc's outside of program segment in VmException' ' ' --- vm/src/vm/errors/vm_exception.rs | 90 ++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/vm/src/vm/errors/vm_exception.rs b/vm/src/vm/errors/vm_exception.rs index e12b413ff9..6916cf2313 100644 --- a/vm/src/vm/errors/vm_exception.rs +++ b/vm/src/vm/errors/vm_exception.rs @@ -1,7 +1,10 @@ -use crate::stdlib::{ - fmt::{self, Display}, - prelude::*, - str, +use crate::{ + stdlib::{ + fmt::{self, Display}, + prelude::*, + str, + }, + types::relocatable::Relocatable, }; use thiserror_no_std::Error; @@ -16,7 +19,7 @@ use crate::{ use super::vm_errors::VirtualMachineError; #[derive(Debug, Error)] pub struct VmException { - pub pc: usize, + pub pc: Relocatable, pub inst_location: Option, pub inner_exc: VirtualMachineError, pub error_attr_value: Option, @@ -29,8 +32,12 @@ impl VmException { vm: &VirtualMachine, error: VirtualMachineError, ) -> Self { - let pc = vm.run_context.pc.offset; - let error_attr_value = get_error_attr_value(pc, runner, vm); + let pc = vm.run_context.pc; + let error_attr_value = if pc.segment_index == 0 { + get_error_attr_value(pc.offset, runner, vm) + } else { + None + }; let hint_index = if let VirtualMachineError::Hint(ref bx) = error { Some(bx.0) } else { @@ -38,7 +45,11 @@ impl VmException { }; VmException { pc, - inst_location: get_location(pc, runner, hint_index), + inst_location: if pc.segment_index == 0 { + get_location(pc.offset, runner, hint_index) + } else { + None + }, inner_exc: error, error_attr_value, traceback: get_traceback(vm, runner), @@ -95,15 +106,15 @@ pub fn get_traceback(vm: &VirtualMachine, runner: &CairoRunner) -> Option traceback.push_str(&format!( + match ( + traceback_pc.segment_index, + get_location(traceback_pc.offset, runner, None), + ) { + (0, Some(location)) => traceback.push_str(&format!( "{}\n", - location.to_string_with_content(&format!("(pc=0:{})", traceback_pc.offset)) - )), - None => traceback.push_str(&format!( - "Unknown location (pc=0:{})\n", - traceback_pc.offset + location.to_string_with_content(&format!("(pc={})", traceback_pc)) )), + _ => traceback.push_str(&format!("Unknown location (pc={})\n", traceback_pc)), } } (!traceback.is_empty()) @@ -194,7 +205,7 @@ fn get_value_from_simple_reference( impl Display for VmException { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // Build initial message - let message = format!("Error at pc=0:{}:\n{}", self.pc, self.inner_exc); + let message = format!("Error at pc={}:\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 { @@ -314,7 +325,7 @@ mod test { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn get_vm_exception_from_vm_error() { - let pc = 0; + let pc: Relocatable = (0, 0).into(); let location = Location { end_line: 2, end_col: 2, @@ -330,7 +341,7 @@ mod test { hints: vec![], }; let program = - program!(instruction_locations = Some(HashMap::from([(pc, instruction_location)])),); + program!(instruction_locations = Some(HashMap::from([(pc.offset, instruction_location)])),); let runner = cairo_runner!(program); assert_matches!( VmException::from_vm_error(&runner, &vm!(), VirtualMachineError::NoImm,), @@ -388,7 +399,7 @@ mod test { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn vm_exception_display_instruction_no_location_no_attributes() { let vm_excep = VmException { - pc: 2, + pc: (0, 2).into(), inst_location: None, inner_exc: VirtualMachineError::FailedToComputeOperands(Box::new(( "op0".to_string(), @@ -413,7 +424,7 @@ mod test { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn vm_exception_display_instruction_no_location_with_attributes() { let vm_excep = VmException { - pc: 2, + pc: (0, 2).into(), inst_location: None, inner_exc: VirtualMachineError::FailedToComputeOperands(Box::new(( "op0".to_string(), @@ -448,7 +459,7 @@ mod test { start_col: 1, }; let vm_excep = VmException { - pc: 2, + pc: (0, 2).into(), inst_location: Some(location), inner_exc: VirtualMachineError::FailedToComputeOperands(Box::new(( "op0".to_string(), @@ -495,7 +506,7 @@ mod test { start_col: 1, }; let vm_excep = VmException { - pc: 2, + pc: (0, 2).into(), inst_location: Some(location), inner_exc: VirtualMachineError::FailedToComputeOperands(Box::new(( "op0".to_string(), @@ -1143,4 +1154,39 @@ cairo_programs/bad_programs/uint256_sub_b_gt_256.cairo:10:2: (pc=0:12) ) ); } + + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn get_vm_exception_from_vm_error_pc_not_program_segment() { + let pc = (9, 5).into(); + let location = Location { + end_line: 2, + end_col: 2, + input_file: InputFile { + filename: String::from("Folder/file.cairo"), + }, + parent_location: None, + start_line: 1, + start_col: 1, + }; + let instruction_location = InstructionLocation { + inst: location.clone(), + hints: vec![], + }; + let program = + program!(instruction_locations = Some(HashMap::from([(5, instruction_location)])),); + let runner = cairo_runner!(program); + let mut vm = vm!(); + vm.set_pc(pc); + assert_matches!( + VmException::from_vm_error(&runner, &vm!(), VirtualMachineError::NoImm,), + VmException { + pc: x, + inst_location: None, + inner_exc: VirtualMachineError::NoImm, + error_attr_value: None, + traceback: Some(_), + } if x == pc + ) + } } From bf9d8d9b43d3c2591731db1db186fba8b9761c7e Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 30 Nov 2023 12:59:10 -0300 Subject: [PATCH 2/7] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddde24eec7..7f1172934c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ #### Upcoming Changes +* feat: Handle `pc`s outside of program segment in `VmException` [#1501] (https://github.com/lambdaclass/cairo-vm/pull/1501) + + * `VmException` now shows the full pc value instead of just the offset (`VmException.pc` field type changed to `Relocatable`) + * `VmException.traceback` now shows the full pc value for each entry instead of hardcoding its index to 0. + * Disable debug information for errors produced when `pc` is outside of the program segment (segment_index != 0). `VmException` fields `inst_location` & `error_attr_value` will be `None` in such case. + * feat: Allow running instructions from pcs outside the program segement [#1493](https://github.com/lambdaclass/cairo-vm/pull/14923) * BREAKING: Partially Revert `Optimize trace relocation #906` [#1492](https://github.com/lambdaclass/cairo-vm/pull/1492) From d8f166c5629d7404149aac534b30ab78ab3d5337 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 30 Nov 2023 13:00:35 -0300 Subject: [PATCH 3/7] clippy --- vm/src/vm/errors/vm_exception.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/vm/errors/vm_exception.rs b/vm/src/vm/errors/vm_exception.rs index 6916cf2313..c7e5c063b4 100644 --- a/vm/src/vm/errors/vm_exception.rs +++ b/vm/src/vm/errors/vm_exception.rs @@ -1170,7 +1170,7 @@ cairo_programs/bad_programs/uint256_sub_b_gt_256.cairo:10:2: (pc=0:12) start_col: 1, }; let instruction_location = InstructionLocation { - inst: location.clone(), + inst: location, hints: vec![], }; let program = From 8272e1b40ce454f9c40ac4a18341f388ff0171d2 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 30 Nov 2023 13:01:24 -0300 Subject: [PATCH 4/7] Fix test value --- vm/src/vm/errors/vm_exception.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/vm/errors/vm_exception.rs b/vm/src/vm/errors/vm_exception.rs index c7e5c063b4..dfda375d68 100644 --- a/vm/src/vm/errors/vm_exception.rs +++ b/vm/src/vm/errors/vm_exception.rs @@ -1185,7 +1185,7 @@ cairo_programs/bad_programs/uint256_sub_b_gt_256.cairo:10:2: (pc=0:12) inst_location: None, inner_exc: VirtualMachineError::NoImm, error_attr_value: None, - traceback: Some(_), + traceback: None, } if x == pc ) } From b8c55cabde18a4d127ec8c67d3ae9c56adad0607 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 30 Nov 2023 13:03:44 -0300 Subject: [PATCH 5/7] Disable attributes in traceback if si != 0 --- vm/src/vm/errors/vm_exception.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/vm/errors/vm_exception.rs b/vm/src/vm/errors/vm_exception.rs index dfda375d68..c7465f6e99 100644 --- a/vm/src/vm/errors/vm_exception.rs +++ b/vm/src/vm/errors/vm_exception.rs @@ -103,7 +103,7 @@ pub fn get_location( 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, vm) { + if let (0, Some(ref attr)) = (traceback_pc.segment_index, get_error_attr_value(traceback_pc.offset, runner, vm)) { traceback.push_str(attr) } match ( From 09033a402e04a37275f7e4407d0f92b30a0b9e66 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 30 Nov 2023 13:10:01 -0300 Subject: [PATCH 6/7] fmt --- vm/src/vm/errors/vm_exception.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/vm/src/vm/errors/vm_exception.rs b/vm/src/vm/errors/vm_exception.rs index c7465f6e99..38d56baa9b 100644 --- a/vm/src/vm/errors/vm_exception.rs +++ b/vm/src/vm/errors/vm_exception.rs @@ -103,7 +103,10 @@ pub fn get_location( 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 (0, Some(ref attr)) = (traceback_pc.segment_index, get_error_attr_value(traceback_pc.offset, runner, vm)) { + if let (0, Some(ref attr)) = ( + traceback_pc.segment_index, + get_error_attr_value(traceback_pc.offset, runner, vm), + ) { traceback.push_str(attr) } match ( @@ -340,8 +343,9 @@ mod test { inst: location.clone(), hints: vec![], }; - let program = - program!(instruction_locations = Some(HashMap::from([(pc.offset, instruction_location)])),); + let program = program!( + instruction_locations = Some(HashMap::from([(pc.offset, instruction_location)])), + ); let runner = cairo_runner!(program); assert_matches!( VmException::from_vm_error(&runner, &vm!(), VirtualMachineError::NoImm,), From 8fdbc6f70e9f488a42bbb76068a1fbb9eec9a806 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 30 Nov 2023 13:24:00 -0300 Subject: [PATCH 7/7] Fix test --- vm/src/vm/errors/vm_exception.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/vm/errors/vm_exception.rs b/vm/src/vm/errors/vm_exception.rs index 38d56baa9b..6e4e762ef4 100644 --- a/vm/src/vm/errors/vm_exception.rs +++ b/vm/src/vm/errors/vm_exception.rs @@ -1183,7 +1183,7 @@ cairo_programs/bad_programs/uint256_sub_b_gt_256.cairo:10:2: (pc=0:12) let mut vm = vm!(); vm.set_pc(pc); assert_matches!( - VmException::from_vm_error(&runner, &vm!(), VirtualMachineError::NoImm,), + VmException::from_vm_error(&runner, &vm, VirtualMachineError::NoImm,), VmException { pc: x, inst_location: None,