Skip to content

Commit

Permalink
feat: add breakpoint instruction
Browse files Browse the repository at this point in the history
This commit introduces `breakpoint`, a transparent issue that will
cause a debug execution to break when reached.

This instruction will not be serialized into libraries, and will not
have an opcode or be part of the code block tree.

For debug executions, it will produce a `NOOP`, so the internal clock of
the VM will be affected.

The decision of not including this as part of the code block is to avoid
further consuming variants of opcodes as they are already scarce.

related issue: #580
  • Loading branch information
vlopes11 committed Mar 6, 2023
1 parent 0ec30b5 commit ad66dde
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 18 deletions.
12 changes: 11 additions & 1 deletion assembly/src/assembler/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,17 @@ impl AssemblyContext {
// HELPER METHODS
// --------------------------------------------------------------------------------------------

/// Returns the context of the procedure currently being complied, or None if module or
/// Returns the context of the procedure currently being compiled, or None if module or
/// procedure stacks are empty.
fn current_proc_context(&self) -> Option<&ProcedureContext> {
self.module_stack.last().and_then(|m| m.proc_stack.last())
}

/// Returns the name of the procedure currently being compilied, or None if module or
/// procedure stacks are empty.
pub(crate) fn current_proc_context_name(&self) -> Option<&str> {
self.current_proc_context().map(|p| p.name().as_ref())
}
}

// MODULE CONTEXT
Expand Down Expand Up @@ -478,6 +484,10 @@ impl ProcedureContext {
self.name.is_main()
}

pub const fn name(&self) -> &ProcedureName {
&self.name
}

pub fn into_procedure(self, id: ProcedureId, code_root: CodeBlock) -> Procedure {
let Self {
name,
Expand Down
11 changes: 10 additions & 1 deletion assembly/src/assembler/instruction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl Assembler {
// this will allow us to map the instruction to the sequence of operations which were
// executed as a part of this instruction.
if self.in_debug_mode() {
span.track_instruction(instruction);
span.track_instruction(instruction, ctx);
}

let result = match instruction {
Expand Down Expand Up @@ -306,6 +306,15 @@ impl Assembler {
Instruction::CallLocal(idx) => self.call_local(*idx, ctx),
Instruction::CallImported(id) => self.call_imported(id, ctx),
Instruction::SysCall(id) => self.syscall(id, ctx),

// ----- debug decorators -------------------------------------------------------------
Instruction::Breakpoint => {
if self.in_debug_mode() {
span.add_op(Noop)?;
span.track_instruction(instruction, ctx);
}
Ok(None)
}
};

// compute and update the cycle count of the instruction which just finished executing
Expand Down
11 changes: 7 additions & 4 deletions assembly/src/assembler/span_builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
AssemblyError, BodyWrapper, Borrow, CodeBlock, Decorator, DecoratorList, Instruction,
Operation, ToString, Vec,
AssemblyContext, AssemblyError, BodyWrapper, Borrow, CodeBlock, Decorator, DecoratorList,
Instruction, Operation, ToString, Vec,
};
use vm_core::AssemblyOp;

Expand Down Expand Up @@ -102,8 +102,11 @@ impl SpanBuilder {
///
/// This indicates that the provided instruction should be tracked and the cycle count for
/// this instruction will be computed when the call to set_instruction_cycle_count() is made.
pub fn track_instruction(&mut self, instruction: &Instruction) {
let op = AssemblyOp::new(instruction.to_string(), 0);
pub fn track_instruction(&mut self, instruction: &Instruction, ctx: &mut AssemblyContext) {
let proc = ctx.current_proc_context_name().unwrap_or("#main");
let op = instruction.to_string();
let op = format!("{proc}:{op}");
let op = AssemblyOp::new(op, 0);
self.push_decorator(Decorator::AsmOp(op));
self.last_asmop_pos = self.decorators.len() - 1;
}
Expand Down
2 changes: 1 addition & 1 deletion assembly/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub use procedures::{ProcedureId, ProcedureName};

mod parsers;
use parsers::PROCEDURE_LABEL_PARSER;
pub use parsers::{parse_module, parse_program, ModuleAst, ProcedureAst, ProgramAst};
pub use parsers::{parse_module, parse_program, Instruction, ModuleAst, ProcedureAst, ProgramAst};

mod serde;
pub use serde::{ByteReader, ByteWriter, Deserializable, Serializable};
Expand Down
3 changes: 3 additions & 0 deletions assembly/src/parsers/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,9 @@ impl ParserContext {
// ----- constant statements ----------------------------------------------------------
"const" => Err(ParsingError::const_invalid_scope(op)),

// ----- debug decorators -------------------------------------------------------------
"breakpoint" => simple_instruction(op, Breakpoint),

// ----- catch all --------------------------------------------------------------------
_ => Err(ParsingError::invalid_op(op)),
}
Expand Down
3 changes: 2 additions & 1 deletion assembly/src/parsers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use core::{fmt::Display, ops::RangeBounds};

mod nodes;
use crate::utils::bound_into_included_u64;
pub(crate) use nodes::{Instruction, Node};
pub use nodes::Instruction;
pub(crate) use nodes::Node;
mod context;
use context::ParserContext;
mod labels;
Expand Down
6 changes: 6 additions & 0 deletions assembly/src/parsers/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,9 @@ pub enum Instruction {
CallLocal(u16),
CallImported(ProcedureId),
SysCall(ProcedureId),

// ----- debug decorators ---------------------------------------------------------------------
Breakpoint,
}

impl fmt::Display for Instruction {
Expand Down Expand Up @@ -543,6 +546,9 @@ impl fmt::Display for Instruction {
Self::CallLocal(index) => write!(f, "call.{index}"),
Self::CallImported(proc_id) => write!(f, "call.{proc_id}"),
Self::SysCall(proc_id) => write!(f, "syscall.{proc_id}"),

// ----- debug decorators -------------------------------------------------------------
Self::Breakpoint => write!(f, "breakpoint"),
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions assembly/src/parsers/serde/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,11 @@ impl Serializable for Instruction {
OpCode::SysCall.write_into(target)?;
imported.write_into(target)?
}

// ----- debug decorators -------------------------------------------------------------
Self::Breakpoint => {
// this is a transparent instruction and will not be encoded into the library
}
}
Ok(())
}
Expand Down
14 changes: 14 additions & 0 deletions miden/src/cli/debug/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ impl DebugExecutor {
DebugCommand::Continue => {
while let Some(new_vm_state) = self.next_vm_state() {
self.vm_state = new_vm_state;
if self.should_break() {
break;
}
}
self.print_vm_state();
}
Expand All @@ -53,6 +56,9 @@ impl DebugExecutor {
match self.next_vm_state() {
Some(next_vm_state) => {
self.vm_state = next_vm_state;
if self.should_break() {
break;
}
}
None => break,
}
Expand All @@ -70,6 +76,9 @@ impl DebugExecutor {
match self.prev_vm_state() {
Some(new_vm_state) => {
self.vm_state = new_vm_state;
if self.should_break() {
break;
}
}
None => break,
}
Expand Down Expand Up @@ -213,4 +222,9 @@ impl DebugExecutor {

println!("{}", message);
}

/// Returns `true` if the current state should break.
fn should_break(&self) -> bool {
self.vm_state.asmop.as_ref().map(|asm| asm.should_break()).unwrap_or(false)
}
}
2 changes: 1 addition & 1 deletion processor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ std = ["vm-core/std", "winter-prover/std", "log/std"]

[dependencies]
log = "0.4.14"
assembly = { package = "miden-assembly", path = "../assembly", version = "0.5", default-features = false }
vm-core = { package = "miden-core", path = "../core", version = "0.5", default-features = false }
winter-prover = { package = "winter-prover", version = "0.5", default-features = false }

[dev-dependencies]
logtest = { version = "2.0", default-features = false }
miden-assembly = { package = "miden-assembly", path = "../assembly", version = "0.5", default-features = false }
rand-utils = { package = "winter-rand-utils", version = "0.5" }
winter-fri = { package = "winter-fri", version = "0.5" }
winter-utils = { package = "winter-utils", version = "0.5" }
47 changes: 38 additions & 9 deletions processor/src/debug.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
advice::AdviceProvider, Chiplets, Decoder, ExecutionError, Felt, Process, Stack, StarkField,
System, Vec,
advice::AdviceProvider, Chiplets, Decoder, ExecutionError, Felt, Instruction, Process, Stack,
StarkField, System, Vec,
};
use core::fmt;
use vm_core::{utils::string::String, Operation, StackOutputs, Word};
Expand All @@ -24,8 +24,17 @@ impl fmt::Display for VmState {
self.memory.iter().map(|x| (x.0, word_to_ints(&x.1))).collect();
write!(
f,
"clk={}, op={:?}, asmop={:?}, fmp={}, stack={stack:?}, memory={memory:?}",
self.clk, self.op, self.asmop, self.fmp
"clk={}{}{}, fmp={}, stack={stack:?}, memory={memory:?}",
self.clk,
match self.op {
Some(op) => format!(", op={op}"),
None => "".to_string(),
},
match &self.asmop {
Some(op) => format!(", {op}"),
None => "".to_string(),
},
self.fmp
)
}
}
Expand Down Expand Up @@ -87,7 +96,7 @@ impl VmStateIterator {
(
&assembly_ops[self.asmop_idx - 1],
// difference between current clock cycle and start clock cycle of the current asmop
(self.clk - assembly_ops[self.asmop_idx - 1].0 as u32) as u8,
(self.clk as i32 - assembly_ops[self.asmop_idx - 1].0 as i32) as i8,
)
} else {
(next_asmop, 0) //dummy value, never used.
Expand All @@ -106,7 +115,7 @@ impl VmStateIterator {
// if this is not the first asmop in the list and if this op is part of current asmop,
// returns a new instance of [AsmOp] instantiated with current asmop, num_cycles and
// cycle_idx of current op.
else if self.asmop_idx > 0 && cycle_idx <= curr_asmop.1.num_cycles() {
else if self.asmop_idx > 0 && cycle_idx <= curr_asmop.1.num_cycles() as i8 {
let asmop = Some(AsmOpInfo::new(
curr_asmop.1.op().clone(),
curr_asmop.1.num_cycles(),
Expand Down Expand Up @@ -222,7 +231,8 @@ fn word_to_ints(word: &Word) -> [u64; 4] {
pub struct AsmOpInfo {
op: String,
num_cycles: u8,
cycle_idx: u8,
cycle_idx: i8,
should_break: bool,
}

// ASMOP STATE
Expand All @@ -232,11 +242,19 @@ impl AsmOpInfo {
/// Returns [AsmOpInfo] instantiated with the specified assembly instruction string, number of
/// cycles it takes to execute the assembly instruction and op index in sequence of operations
/// corresponding to the current assembly instruction. The first index is 1 instead of 0.
pub fn new(op: String, num_cycles: u8, cycle_idx: u8) -> Self {
pub fn new(op: String, num_cycles: u8, cycle_idx: i8) -> Self {
// breakpoint doesn't exist as operation. we instead assume the properly formatted
// decorator will carry this data
let should_break = op
.split(':')
.last()
.filter(|l| l == &Instruction::Breakpoint.to_string().as_str())
.is_some();
Self {
op,
num_cycles,
cycle_idx,
should_break,
}
}

Expand All @@ -263,7 +281,18 @@ impl AsmOpInfo {

/// Returns the operation index of the operation at the specified clock cycle in the sequence
/// of operations corresponding to the current assembly instruction.
pub fn cycle_idx(&self) -> u8 {
pub fn cycle_idx(&self) -> i8 {
self.cycle_idx
}

/// Returns `true` if the debug should break for this line.
pub const fn should_break(&self) -> bool {
self.should_break
}
}

impl fmt::Display for AsmOpInfo {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "decorator={}, cost={}, cycles={}", self.op, self.num_cycles, self.cycle_idx)
}
}
1 change: 1 addition & 0 deletions processor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#[macro_use]
extern crate alloc;

use assembly::Instruction;
pub use vm_core::{
chiplets::hasher::Digest,
crypto::{
Expand Down

0 comments on commit ad66dde

Please sign in to comment.