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 8 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)
4 changes: 2 additions & 2 deletions src/types/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ 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 {
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,
Expand All @@ -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() {
Expand Down
6 changes: 3 additions & 3 deletions src/vm/decoding/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<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 @@ -118,7 +118,7 @@ pub fn decode_instruction(
off0,
off1,
off2,
imm,
imm: imm.cloned(),
dst_register,
op0_register,
op1_addr,
Expand Down Expand Up @@ -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));
Expand Down
15 changes: 8 additions & 7 deletions src/vm/errors/vm_exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -395,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));
Expand All @@ -416,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));
Expand Down
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
16 changes: 6 additions & 10 deletions src/vm/vm_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down Expand Up @@ -761,10 +760,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,
}
Expand Down Expand Up @@ -3917,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))),
Expand All @@ -3941,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);
Expand Down