Skip to content

Commit c39508b

Browse files
YairVaknin-starkwareJulianGCalderon
YairVaknin-starkware
and
JulianGCalderon
authored
Minor ret decoding refactor - #1925 cherry pick (#1930)
* Minor ret decoding refactor - #1925 cherry pick * lock cargo-llvm-cov * Update test_install runner to macos-13 --------- Co-authored-by: Julián González Calderón <[email protected]>
1 parent 93f1f54 commit c39508b

File tree

6 files changed

+272
-47
lines changed

6 files changed

+272
-47
lines changed

‎.github/workflows/test_install.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ jobs:
1212
name: "Install on ${{ matrix.os }}"
1313
strategy:
1414
matrix:
15-
os: [ubuntu-22.04, macos-12]
15+
os: [ubuntu-22.04, macos-13]
1616
runs-on: ${{ matrix.os }}
1717
steps:
1818
- uses: actions/checkout@v3

‎CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
#### Upcoming Changes
44

5+
* refactor: Limit ret opcode decodeing to Cairo0's standards. [#1925](https://github.com/lambdaclass/cairo-vm/pull/1925)
6+
57
#### [1.0.1] - 2024-08-12
68

7-
* fix(BREAKING): [#1818](https://github.com/lambdaclass/cairo-vm/pull/1818):
9+
* fix(BREAKING): [#1818](https://github.com/lambdaclass/cairo-vm/pull/1818):
810
* Fix `MemorySegmentManager::add_zero_segment` function when resizing a segment
911
* Signature change(BREAKING): `MemorySegmentManager::get_memory_holes` now receives `builtin_segment_indexes: HashSet<usize>`
1012

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ cargo-deps:
202202
cargo install --version 0.6.1 flamegraph --locked
203203
cargo install --version 1.14.0 hyperfine
204204
cargo install --version 0.9.49 cargo-nextest --locked
205-
cargo install --version 0.5.9 cargo-llvm-cov
205+
cargo install --version 0.5.9 cargo-llvm-cov --locked
206206
cargo install --version 0.12.1 wasm-pack --locked
207207

208208
cairo1-run-deps:

‎vm/src/vm/decoding/decoder.rs

Lines changed: 119 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ pub fn decode_instruction(encoded_instr: u64) -> Result<Instruction, VirtualMach
8282
_ => return Err(VirtualMachineError::InvalidPcUpdate(pc_update_num)),
8383
};
8484

85-
let res = match res_logic_num {
86-
0 if matches!(pc_update, PcUpdate::Jnz) => Res::Unconstrained,
87-
0 => Res::Op1,
88-
1 => Res::Add,
89-
2 => Res::Mul,
85+
let res = match (res_logic_num, pc_update == PcUpdate::Jnz) {
86+
(0, true) => Res::Unconstrained,
87+
(0, false) => Res::Op1,
88+
(1, false) => Res::Add,
89+
(2, false) => Res::Mul,
9090
_ => return Err(VirtualMachineError::InvalidRes(res_logic_num)),
9191
};
9292

@@ -98,17 +98,38 @@ pub fn decode_instruction(encoded_instr: u64) -> Result<Instruction, VirtualMach
9898
_ => return Err(VirtualMachineError::InvalidOpcode(opcode_num)),
9999
};
100100

101-
let ap_update = match ap_update_num {
102-
0 if matches!(opcode, Opcode::Call) => ApUpdate::Add2,
103-
0 => ApUpdate::Regular,
104-
1 => ApUpdate::Add,
105-
2 => ApUpdate::Add1,
101+
let ap_update = match (ap_update_num, opcode == Opcode::Call) {
102+
(0, true) => ApUpdate::Add2,
103+
(0, false) => ApUpdate::Regular,
104+
(1, false) => ApUpdate::Add,
105+
(2, false) => ApUpdate::Add1,
106106
_ => return Err(VirtualMachineError::InvalidApUpdate(ap_update_num)),
107107
};
108108

109109
let fp_update = match opcode {
110-
Opcode::Call => FpUpdate::APPlus2,
111-
Opcode::Ret => FpUpdate::Dst,
110+
Opcode::Call => {
111+
if off0 != 0
112+
|| off1 != 1
113+
|| ap_update != ApUpdate::Add2
114+
|| dst_register != Register::AP
115+
|| op0_register != Register::AP
116+
{
117+
return Err(VirtualMachineError::InvalidOpcode(opcode_num));
118+
};
119+
FpUpdate::APPlus2
120+
}
121+
Opcode::Ret => {
122+
if off0 != -2
123+
|| off2 != -1
124+
|| dst_register != Register::FP
125+
|| op1_addr != Op1Addr::FP
126+
|| res != Res::Op1
127+
|| pc_update != PcUpdate::Jump
128+
{
129+
return Err(VirtualMachineError::InvalidOpcode(opcode_num));
130+
};
131+
FpUpdate::Dst
132+
}
112133
_ => FpUpdate::Regular,
113134
};
114135

@@ -199,56 +220,56 @@ mod decoder_test {
199220

200221
#[test]
201222
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
202-
fn decode_flags_call_add_jmp_add_imm_fp_fp() {
223+
fn decode_flags_nop_add_jmp_add_imm_fp_fp() {
203224
// 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
204225
// 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0
205-
// | CALL| ADD| JUMP| ADD| IMM| FP| FP
206-
// 0 0 0 1 0 1 0 0 1 0 1 0 0 1 1 1
207-
// 0001 0100 1010 0111 = 0x14A7; offx = 0
208-
let inst = decode_instruction(0x14A7800080008000).unwrap();
226+
// | NOp| ADD| JUMP| ADD| IMM| FP| FP
227+
// 0 0 0 0 0 1 0 0 1 0 1 0 0 1 1 1
228+
// 0000 0100 1010 0111 = 0x04A7; offx = 0
229+
let inst = decode_instruction(0x04A7800080008000).unwrap();
209230
assert_matches!(inst.dst_register, Register::FP);
210231
assert_matches!(inst.op0_register, Register::FP);
211232
assert_matches!(inst.op1_addr, Op1Addr::Imm);
212233
assert_matches!(inst.res, Res::Add);
213234
assert_matches!(inst.pc_update, PcUpdate::Jump);
214235
assert_matches!(inst.ap_update, ApUpdate::Add);
215-
assert_matches!(inst.opcode, Opcode::Call);
216-
assert_matches!(inst.fp_update, FpUpdate::APPlus2);
236+
assert_matches!(inst.opcode, Opcode::NOp);
237+
assert_matches!(inst.fp_update, FpUpdate::Regular);
217238
}
218239

219240
#[test]
220241
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
221-
fn decode_flags_ret_add1_jmp_rel_mul_fp_ap_ap() {
242+
fn decode_flags_nop_add1_jmp_rel_mul_fp_ap_ap() {
222243
// 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
223244
// 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0
224-
// | RET| ADD1| JUMP_REL| MUL| FP| AP| AP
225-
// 0 0 1 0 1 0 0 1 0 1 0 0 1 0 0 0
226-
// 0010 1001 0100 1000 = 0x2948; offx = 0
227-
let inst = decode_instruction(0x2948800080008000).unwrap();
245+
// | NOp| ADD1| JUMP_REL| MUL| FP| AP| AP
246+
// 0 0 0 0 1 0 0 1 0 1 0 0 1 0 0 0
247+
// 0000 1001 0100 1000 = 0x0948; offx = 0
248+
let inst = decode_instruction(0x0948800080008000).unwrap();
228249
assert_matches!(inst.dst_register, Register::AP);
229250
assert_matches!(inst.op0_register, Register::AP);
230251
assert_matches!(inst.op1_addr, Op1Addr::FP);
231252
assert_matches!(inst.res, Res::Mul);
232253
assert_matches!(inst.pc_update, PcUpdate::JumpRel);
233254
assert_matches!(inst.ap_update, ApUpdate::Add1);
234-
assert_matches!(inst.opcode, Opcode::Ret);
235-
assert_matches!(inst.fp_update, FpUpdate::Dst);
255+
assert_matches!(inst.opcode, Opcode::NOp);
256+
assert_matches!(inst.fp_update, FpUpdate::Regular);
236257
}
237258

238259
#[test]
239260
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
240-
fn decode_flags_assrt_add_jnz_mul_ap_ap_ap() {
261+
fn decode_flags_assrt_add_regular_mul_ap_ap_ap() {
241262
// 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
242263
// 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0
243-
// |ASSRT_EQ| ADD| JNZ| MUL| AP| AP| AP
244-
// 0 1 0 0 1 0 1 0 0 1 0 1 0 0 0 0
245-
// 0100 1010 0101 0000 = 0x4A50; offx = 0
246-
let inst = decode_instruction(0x4A50800080008000).unwrap();
264+
// |ASSRT_EQ| ADD| REGULAR| MUL| AP| AP| AP
265+
// 0 1 0 0 1 0 0 0 0 1 0 1 0 0 0 0
266+
// 0100 1000 0101 0000 = 0x4850; offx = 0
267+
let inst = decode_instruction(0x4850800080008000).unwrap();
247268
assert_matches!(inst.dst_register, Register::AP);
248269
assert_matches!(inst.op0_register, Register::AP);
249270
assert_matches!(inst.op1_addr, Op1Addr::AP);
250271
assert_matches!(inst.res, Res::Mul);
251-
assert_matches!(inst.pc_update, PcUpdate::Jnz);
272+
assert_matches!(inst.pc_update, PcUpdate::Regular);
252273
assert_matches!(inst.ap_update, ApUpdate::Add1);
253274
assert_matches!(inst.opcode, Opcode::AssertEq);
254275
assert_matches!(inst.fp_update, FpUpdate::Regular);
@@ -305,4 +326,70 @@ mod decoder_test {
305326
assert_eq!(inst.off1, 0);
306327
assert_eq!(inst.off2, 1);
307328
}
329+
330+
#[test]
331+
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
332+
fn decode_ret_cairo_standard() {
333+
// 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
334+
// 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0
335+
// | RET| REGULAR| JUMP| Op1| FP| FP| FP
336+
// 0 0 1 0 0 0 0 0 1 0 0 0 1 0 1 1
337+
// 0010 0000 1000 1011 = 0x208b; off0 = -2, off1 = -1
338+
let inst = decode_instruction(0x208b7fff7fff7ffe).unwrap();
339+
assert_matches!(inst.opcode, Opcode::Ret);
340+
assert_matches!(inst.off0, -2);
341+
assert_matches!(inst.off1, -1);
342+
assert_matches!(inst.dst_register, Register::FP);
343+
assert_matches!(inst.op0_register, Register::FP);
344+
assert_matches!(inst.op1_addr, Op1Addr::FP);
345+
assert_matches!(inst.res, Res::Op1);
346+
assert_matches!(inst.pc_update, PcUpdate::Jump);
347+
assert_matches!(inst.ap_update, ApUpdate::Regular);
348+
assert_matches!(inst.fp_update, FpUpdate::Dst);
349+
}
350+
351+
#[test]
352+
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
353+
fn decode_call_cairo_standard() {
354+
// 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
355+
// 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0
356+
// | CALL| Add2| JumpRel| Op1| FP| FP| FP
357+
// 0 0 0 1 0 0 0 1 0 0 0 0 0 1 0 0
358+
// 0001 0001 0000 0100 = 0x208b; off0 = 0, off1 = 1
359+
let inst = decode_instruction(0x1104800180018000).unwrap();
360+
assert_matches!(inst.opcode, Opcode::Call);
361+
assert_matches!(inst.off0, 0);
362+
assert_matches!(inst.off1, 1);
363+
assert_matches!(inst.dst_register, Register::AP);
364+
assert_matches!(inst.op0_register, Register::AP);
365+
assert_matches!(inst.op1_addr, Op1Addr::Imm);
366+
assert_matches!(inst.res, Res::Op1);
367+
assert_matches!(inst.pc_update, PcUpdate::JumpRel);
368+
assert_matches!(inst.ap_update, ApUpdate::Add2);
369+
assert_matches!(inst.fp_update, FpUpdate::APPlus2);
370+
}
371+
372+
#[test]
373+
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
374+
fn decode_ret_opcode_error() {
375+
// 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
376+
// 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0
377+
// | RET| REGULAR| JUMP| Op1| FP| FP| FP
378+
// 0 0 1 0 0 0 0 0 1 0 0 0 1 0 1 1
379+
// 0010 0000 1000 1011 = 0x208b; off0 = -1, off1 = -1
380+
let error = decode_instruction(0x208b7fff7fff7fff);
381+
assert_matches!(error, Err(VirtualMachineError::InvalidOpcode(2)));
382+
}
383+
384+
#[test]
385+
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
386+
fn decode_call_opcode_error() {
387+
// 0| opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
388+
// 15|14 13 12| 11 10| 9 8 7| 6 5|4 3 2| 1| 0
389+
// | CALL| Add2| JumpRel| Op1| FP| FP| FP
390+
// 0 0 0 1 0 0 0 1 0 0 0 0 0 1 0 0
391+
// 0001 0001 0000 0100 = 0x208b; off0 = 1, off1 = 1
392+
let error = decode_instruction(0x1104800180018001);
393+
assert_matches!(error, Err(VirtualMachineError::InvalidOpcode(1)));
394+
}
308395
}

‎vm/src/vm/runners/cairo_runner.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ pub struct CairoRunner {
154154
run_ended: bool,
155155
segments_finalized: bool,
156156
execution_public_memory: Option<Vec<usize>>,
157-
runner_mode: RunnerMode,
157+
pub(crate) runner_mode: RunnerMode,
158158
pub relocated_memory: Vec<Option<Felt252>>,
159159
pub exec_scopes: ExecutionScopes,
160160
pub relocated_trace: Option<Vec<RelocatedTraceEntry>>,

0 commit comments

Comments
 (0)