Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add traceback to VmException #657

Merged
merged 25 commits into from
Dec 28, 2022
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c47e701
Start get_traceback_entries + add convenience methos
fmoletta Dec 22, 2022
51219ca
Add fn is_call_instruction
fmoletta Dec 22, 2022
3b276db
add code
fmoletta Dec 22, 2022
05f0bdd
Merge branch 'main' of github.com:lambdaclass/cairo-rs into traceback
fmoletta Dec 22, 2022
c89eb3f
Refactor code
fmoletta Dec 22, 2022
f02db3e
Clippy
fmoletta Dec 22, 2022
05813bd
Add get_traceback method
fmoletta Dec 22, 2022
58992c3
Fix get_error_attr_value
fmoletta Dec 22, 2022
b381624
Add traceback to VmException
fmoletta Dec 22, 2022
936eadc
Make traceback non-optional
fmoletta Dec 22, 2022
e72dff5
Add tests for is_call_instruction
fmoletta Dec 22, 2022
93eae26
Add traceback to error display
fmoletta Dec 22, 2022
67ccc39
Add test + fix logic for get_traceback_entries
fmoletta Dec 22, 2022
ba410b0
Code refactor
fmoletta Dec 22, 2022
acf3299
Add one more test for get_traceback_entries
fmoletta Dec 22, 2022
bc66d0f
Fix string format + add test for get_traceback
fmoletta Dec 22, 2022
b3952cb
Improve fn
fmoletta Dec 27, 2022
c2fa8a0
Add reference to is_call_instruction signature
fmoletta Dec 27, 2022
88e4b52
Add reference to immediate in decode_instruction + remove clone
fmoletta Dec 27, 2022
7a03cec
Merge branch 'main' of github.com:lambdaclass/cairo-rs into traceback
fmoletta Dec 27, 2022
371dc43
Fix hint_processor mutability in tests
fmoletta Dec 27, 2022
61f8f4c
Add changelog
fmoletta Dec 28, 2022
54969d3
Add PR link
fmoletta Dec 28, 2022
8436021
Merge branch 'main' of github.com:lambdaclass/cairo-rs into traceback
fmoletta Dec 28, 2022
74239cb
Remove redundant information
fmoletta Dec 28, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
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.
fmoletta marked this conversation as resolved.
Show resolved Hide resolved
* 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<Location>` to `pub fn get_location(pc: usize, runner: &CairoRunner) -> Option<Location>`
* `decode_instruction()` signature changed from `pub fn decode_instruction(encoded_instr: i64, mut imm: Option<BigInt>) -> Result<instruction::Instruction, VirtualMachineError>` to `pub fn decode_instruction(encoded_instr: i64, mut imm: Option<&BigInt>) -> Result<instruction::Instruction, VirtualMachineError>`
* Minor changes in `VmExcepion` field's string format to mirror their cairo-lang conterparts.
fmoletta marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion src/cairo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
38 changes: 38 additions & 0 deletions src/types/instruction.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
};
fmoletta marked this conversation as resolved.
Show resolved Hide resolved
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));
}
}
9 changes: 4 additions & 5 deletions src/vm/decoding/decoder.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -119,7 +118,7 @@ pub fn decode_instruction(
off0,
off1,
off2,
imm,
imm: imm.cloned(),
dst_register,
op0_register,
op1_addr,
Expand Down Expand Up @@ -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));
Expand Down
120 changes: 102 additions & 18 deletions src/vm/errors/vm_exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)
fmoletta marked this conversation as resolved.
Show resolved Hide resolved
.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 {
Expand All @@ -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)
}
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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
)
}
Expand Down Expand Up @@ -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")
)
}

Expand All @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)))
)
)
Expand Down Expand Up @@ -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]
Expand All @@ -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));
}
}
2 changes: 1 addition & 1 deletion src/vm/trace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading