Skip to content

fix(codegen): emit trace events to fix stack traces in debugger #507

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

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 9 additions & 1 deletion codegen/masm/src/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use midenc_session::{
Session,
};

use crate::{lower::NativePtr, masm};
use crate::{lower::NativePtr, masm, TraceEvent};

pub struct MasmComponent {
pub id: builtin::ComponentId,
Expand Down Expand Up @@ -339,7 +339,13 @@ impl MasmComponent {
}

// Invoke the program entrypoint
block.push(Op::Inst(Span::new(
span,
Inst::Trace(TraceEvent::FrameStart.as_u32().into()),
)));
block.push(Op::Inst(Span::new(span, Inst::Exec(entrypoint.clone()))));
block
.push(Op::Inst(Span::new(span, Inst::Trace(TraceEvent::FrameEnd.as_u32().into()))));

// Truncate the stack to 16 elements on exit
let truncate_stack = InvocationTarget::AbsoluteProcedurePath {
Expand Down Expand Up @@ -376,13 +382,15 @@ impl MasmComponent {
// => [num_words, dest_ptr] on operand stack
block.push(Op::Inst(Span::new(span, Inst::AdvPush(2.into()))));
// => [C, B, A, dest_ptr] on operand stack
block.push(Op::Inst(Span::new(span, Inst::Trace(TraceEvent::FrameStart.as_u32().into()))));
block.push(Op::Inst(Span::new(
span,
Inst::Exec(InvocationTarget::AbsoluteProcedurePath {
name: pipe_words_to_memory,
path: std_mem,
}),
)));
block.push(Op::Inst(Span::new(span, Inst::Trace(TraceEvent::FrameEnd.as_u32().into()))));
// Drop C, B, A
block.push(Op::Inst(Span::new(span, Inst::DropW)));
block.push(Op::Inst(Span::new(span, Inst::DropW)));
Expand Down
12 changes: 9 additions & 3 deletions codegen/masm/src/emit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ use miden_assembly::ast::InvokeKind;
use midenc_hir::{Immediate, Operation, SourceSpan, Type, ValueRef};

use super::{Operand, OperandStack};
use crate::masm::{self as masm, Op};
use crate::{
masm::{self as masm, Op},
TraceEvent,
};

/// This structure is used to emit the Miden Assembly ops corresponding to an IR instruction.
///
Expand Down Expand Up @@ -202,8 +205,11 @@ impl<'a> OpEmitter<'a> {
)));
let path = masm::LibraryPath::new(callee.module.as_str()).unwrap();
let target = masm::InvocationTarget::AbsoluteProcedurePath { name, path };
let inst = masm::Instruction::Exec(target);
self.emit(inst, span);
self.emit(masm::Instruction::Trace(TraceEvent::FrameStart.as_u32().into()), span);
self.emit(masm::Instruction::Nop, span);
self.emit(masm::Instruction::Exec(target), span);
self.emit(masm::Instruction::Trace(TraceEvent::FrameEnd.as_u32().into()), span);
self.emit(masm::Instruction::Nop, span);
}

/// Emit `op` to the current block
Expand Down
9 changes: 9 additions & 0 deletions codegen/masm/src/emit/primop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use midenc_hir::{
};

use super::{int64, masm, OpEmitter};
use crate::TraceEvent;

impl OpEmitter<'_> {
/// Assert that an integer value on the stack has the value 1
Expand Down Expand Up @@ -254,7 +255,11 @@ impl OpEmitter<'_> {
) {
self.process_call_signature(&callee, signature, span);

self.emit(masm::Instruction::Trace(TraceEvent::FrameStart.as_u32().into()), span);
self.emit(masm::Instruction::Nop, span);
self.emit(masm::Instruction::Exec(callee), span);
self.emit(masm::Instruction::Trace(TraceEvent::FrameEnd.as_u32().into()), span);
self.emit(masm::Instruction::Nop, span);
}

/// Execute the given procedure in a new context.
Expand All @@ -268,7 +273,11 @@ impl OpEmitter<'_> {
) {
self.process_call_signature(&callee, signature, span);

self.emit(masm::Instruction::Trace(TraceEvent::FrameStart.as_u32().into()), span);
self.emit(masm::Instruction::Nop, span);
self.emit(masm::Instruction::Call(callee), span);
self.emit(masm::Instruction::Trace(TraceEvent::FrameEnd.as_u32().into()), span);
self.emit(masm::Instruction::Nop, span);
}

fn process_call_signature(
Expand Down
10 changes: 10 additions & 0 deletions codegen/masm/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ impl TraceEvent {
pub fn is_frame_end(&self) -> bool {
matches!(self, Self::FrameEnd)
}

pub fn as_u32(self) -> u32 {
match self {
Self::FrameStart => TRACE_FRAME_START,
Self::FrameEnd => TRACE_FRAME_END,
Self::AssertionFailed(None) => 0,
Self::AssertionFailed(Some(code)) => code.get(),
Self::Unknown(event) => event,
}
}
}
impl From<u32> for TraceEvent {
fn from(raw: u32) -> Self {
Expand Down
1 change: 1 addition & 0 deletions codegen/masm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,6 @@ pub fn register_dialect_hooks(context: &midenc_hir::Context) {
info.register_operation_trait::<hir::MemSize, dyn HirLowering>();
info.register_operation_trait::<hir::MemSet, dyn HirLowering>();
info.register_operation_trait::<hir::MemCpy, dyn HirLowering>();
info.register_operation_trait::<hir::Breakpoint, dyn HirLowering>();
});
}
14 changes: 13 additions & 1 deletion codegen/masm/src/lower/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
artifact::MasmComponent,
emitter::BlockEmitter,
linker::{LinkInfo, Linker},
masm,
masm, TraceEvent,
};

/// This trait represents a conversion pass from some HIR entity to a Miden Assembly component.
Expand Down Expand Up @@ -161,13 +161,19 @@ impl MasmComponentBuilder<'_> {
self.init_body.push(masm::Op::Inst(Span::new(span, Inst::PushU32(heap_base))));
let heap_init = masm::ProcedureName::new("heap_init").unwrap();
let memory_intrinsics = masm::LibraryPath::new("intrinsics::mem").unwrap();
self.init_body.push(Op::Inst(Span::new(
span,
Inst::Trace(TraceEvent::FrameStart.as_u32().into()),
)));
self.init_body.push(Op::Inst(Span::new(
span,
Inst::Exec(InvocationTarget::AbsoluteProcedurePath {
name: heap_init,
path: memory_intrinsics,
}),
)));
self.init_body
.push(Op::Inst(Span::new(span, Inst::Trace(TraceEvent::FrameEnd.as_u32().into()))));

// Data segment initialization
self.emit_data_segment_initialization();
Expand Down Expand Up @@ -307,13 +313,19 @@ impl MasmComponentBuilder<'_> {
self.init_body
.push(Op::Inst(Span::new(span, Inst::PushU32(rodata.size_in_words() as u32))));
// [num_words, write_ptr, COM, ..] -> [write_ptr']
self.init_body.push(Op::Inst(Span::new(
span,
Inst::Trace(TraceEvent::FrameStart.as_u32().into()),
)));
self.init_body.push(Op::Inst(Span::new(
span,
Inst::Exec(InvocationTarget::AbsoluteProcedurePath {
name: pipe_preimage_to_memory.clone(),
path: std_mem.clone(),
}),
)));
self.init_body
.push(Op::Inst(Span::new(span, Inst::Trace(TraceEvent::FrameEnd.as_u32().into()))));
// drop write_ptr'
self.init_body.push(Op::Inst(Span::new(span, Inst::Drop)));
}
Expand Down
8 changes: 8 additions & 0 deletions codegen/masm/src/lower/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,14 @@ impl HirLowering for hir::AssertEq {
}
}

impl HirLowering for hir::Breakpoint {
fn emit(&self, emitter: &mut BlockEmitter<'_>) -> Result<(), Report> {
emitter.emit_op(masm::Op::Inst(Span::new(self.span(), masm::Instruction::Breakpoint)));

Ok(())
}
}

impl HirLowering for ub::Unreachable {
fn emit(&self, emitter: &mut BlockEmitter<'_>) -> Result<(), Report> {
// This instruction, if reached, must cause the VM to trap, so we emit an assertion that
Expand Down
8 changes: 8 additions & 0 deletions dialects/hir/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ pub trait HirOpBuilder<'f, B: ?Sized + Builder> {
self.assert_eq(lhs, rhs, span)
}

fn breakpoint(
&mut self,
span: SourceSpan,
) -> Result<UnsafeIntrusiveEntityRef<crate::ops::Breakpoint>, Report> {
let op_builder = self.builder_mut().create::<crate::ops::Breakpoint, _>(span);
op_builder()
}

/// Grow the global heap by `num_pages` pages, in 64kb units.
///
/// Returns the previous size (in pages) of the heap, or -1 if the heap could not be grown.
Expand Down
1 change: 1 addition & 0 deletions dialects/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,5 +127,6 @@ impl DialectRegistration for HirDialect {
info.register_operation::<ops::MemCpy>();
info.register_operation::<ops::Spill>();
info.register_operation::<ops::Reload>();
info.register_operation::<ops::Breakpoint>();
}
}
15 changes: 15 additions & 0 deletions dialects/hir/src/ops/primop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,18 @@ impl EffectOpInterface<MemoryEffect> for MemCpy {
])
}
}

#[operation(
dialect = HirDialect,
implements(MemoryEffectOpInterface)
)]
pub struct Breakpoint {}

impl EffectOpInterface<MemoryEffect> for Breakpoint {
fn effects(&self) -> EffectIterator<MemoryEffect> {
EffectIterator::from_smallvec(smallvec![
EffectInstance::new(MemoryEffect::Read),
EffectInstance::new(MemoryEffect::Write),
])
}
}
33 changes: 33 additions & 0 deletions frontend/wasm/src/intrinsics/debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use midenc_dialect_hir::HirOpBuilder;
use midenc_hir::{
dialects::builtin::FunctionRef,
interner::{symbols, Symbol},
smallvec, Builder, SmallVec, SourceSpan, SymbolNameComponent, ValueRef,
};

use crate::{error::WasmResult, module::function_builder_ext::FunctionBuilderExt};

pub(crate) const MODULE_ID: &str = "intrinsics::debug";
pub(crate) const MODULE_PREFIX: &[SymbolNameComponent] = &[
SymbolNameComponent::Root,
SymbolNameComponent::Component(symbols::Intrinsics),
SymbolNameComponent::Component(symbols::Debug),
];

/// Convert a call to a debugging intrinsic function into instruction(s)
pub(crate) fn convert_debug_intrinsics<B: ?Sized + Builder>(
function: Symbol,
_function_ref: Option<FunctionRef>,
args: &[ValueRef],
builder: &mut FunctionBuilderExt<'_, B>,
span: SourceSpan,
) -> WasmResult<SmallVec<[ValueRef; 1]>> {
match function.as_str() {
"break" => {
assert_eq!(args.len(), 0, "{function} takes exactly one argument");
builder.breakpoint(span)?;
Ok(smallvec![])
}
_ => panic!("no debug intrinsics found named '{function}'"),
}
}
13 changes: 10 additions & 3 deletions frontend/wasm/src/intrinsics/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use midenc_hir::{
FunctionType, SymbolNameComponent, SymbolPath,
};

use super::{felt, mem};
use super::{debug, felt, mem};

/// Error raised when an attempt is made to use or load an unrecognized intrinsic
#[derive(Debug, thiserror::Error, Diagnostic)]
Expand All @@ -18,6 +18,8 @@ pub struct UnknownIntrinsicError(SymbolPath);
/// intrinsic up to the point it was encoded in this type.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum Intrinsic {
/// A debugging intrinsic
Debug(Symbol),
/// A memory intrinsic
Mem(Symbol),
/// A field element intrinsic
Expand Down Expand Up @@ -56,6 +58,7 @@ impl TryFrom<&SymbolPath> for Intrinsic {
.ok_or_else(|| UnknownIntrinsicError(path.clone()))?;

match kind {
symbols::Debug => Ok(Self::Debug(function)),
symbols::Mem => Ok(Self::Mem(function)),
symbols::Felt => Ok(Self::Felt(function)),
_ => Err(UnknownIntrinsicError(path.clone())),
Expand All @@ -75,6 +78,7 @@ impl Intrinsic {
/// intrinsic is defined.
pub fn module_name(&self) -> Symbol {
match self {
Self::Debug(_) => symbols::Debug,
Self::Mem(_) => symbols::Mem,
Self::Felt(_) => symbols::Felt,
}
Expand All @@ -83,6 +87,7 @@ impl Intrinsic {
/// Get a [SymbolPath] corresponding to the module containing this intrinsic
pub fn module_path(&self) -> SymbolPath {
match self {
Self::Debug(_) => SymbolPath::from_iter(debug::MODULE_PREFIX.iter().copied()),
Self::Mem(_) => SymbolPath::from_iter(mem::MODULE_PREFIX.iter().copied()),
Self::Felt(_) => SymbolPath::from_iter(felt::MODULE_PREFIX.iter().copied()),
}
Expand All @@ -91,7 +96,7 @@ impl Intrinsic {
/// Get the name of the intrinsic function as a [Symbol]
pub fn function_name(&self) -> Symbol {
match self {
Self::Mem(function) | Self::Felt(function) => *function,
Self::Debug(function) | Self::Mem(function) | Self::Felt(function) => *function,
}
}

Expand All @@ -101,6 +106,8 @@ impl Intrinsic {
pub fn function_type(&self) -> Option<FunctionType> {
match self {
Self::Mem(function) => mem::function_type(*function),
// All debugging intrinsics are currently implemented as native instructions
Self::Debug(_) => None,
// All field element intrinsics are currently implemented as native instructions
Self::Felt(_) => None,
}
Expand All @@ -114,7 +121,7 @@ impl Intrinsic {
Self::Mem(function) => {
mem::function_type(*function).map(IntrinsicsConversionResult::FunctionType)
}
Self::Felt(_) => Some(IntrinsicsConversionResult::MidenVmOp),
Self::Debug(_) | Self::Felt(_) => Some(IntrinsicsConversionResult::MidenVmOp),
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions frontend/wasm/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod intrinsic;

pub use self::intrinsic::*;

pub mod debug;
pub mod felt;
pub mod mem;

Expand All @@ -21,6 +22,7 @@ fn modules() -> &'static FxHashSet<SymbolPath> {
let mut s = FxHashSet::default();
s.insert(SymbolPath::from_iter(mem::MODULE_PREFIX.iter().copied()));
s.insert(SymbolPath::from_iter(felt::MODULE_PREFIX.iter().copied()));
s.insert(SymbolPath::from_iter(debug::MODULE_PREFIX.iter().copied()));
s
});
&MODULES
Expand All @@ -35,6 +37,9 @@ pub fn convert_intrinsics_call<B: ?Sized + Builder>(
span: SourceSpan,
) -> WasmResult<SmallVec<[ValueRef; 1]>> {
match intrinsic {
Intrinsic::Debug(function) => {
debug::convert_debug_intrinsics(function, function_ref, args, builder, span)
}
Intrinsic::Mem(function) => {
mem::convert_mem_intrinsics(function, function_ref, args, builder, span)
}
Expand Down
2 changes: 2 additions & 0 deletions frontend/wasm/src/miden_abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ pub fn recover_imported_masm_module(wasm_module_id: &str) -> Result<SymbolPath,
Ok(SymbolPath::from_masm_module_id(intrinsics::mem::MODULE_ID))
} else if wasm_module_id.starts_with("miden:core-import/intrinsics-felt") {
Ok(SymbolPath::from_masm_module_id(intrinsics::felt::MODULE_ID))
} else if wasm_module_id.starts_with("miden:core-import/intrinsics-debug") {
Ok(SymbolPath::from_masm_module_id(intrinsics::debug::MODULE_ID))
} else if wasm_module_id.starts_with("miden:core-import/account") {
Ok(SymbolPath::from_masm_module_id(tx_kernel::account::MODULE_ID))
} else if wasm_module_id.starts_with("miden:core-import/note") {
Expand Down
1 change: 1 addition & 0 deletions hir-symbol/src/symbols.toml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ ub = {}
account = {}
blake3 = {}
crypto = {}
debug = {}
dsa = {}
hashes = {}
intrinsics = {}
Expand Down
14 changes: 14 additions & 0 deletions sdk/stdlib-sys/src/intrinsics/debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#[link(wasm_import_module = "miden:core-import/[email protected]")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface with the break function should be in our core-import WIT file at tests/rust-apps-wasm/rust-sdk/wit-sdk/miden-core-import.wit, otherwise wit-component errors on core module import. I'm in the middle of splitting our WIT files and moving them to SDK crates at #505, and I can take care of it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah noted, I'll leave the fix to you then!

extern "C" {
#[link_name = "break"]
fn extern_break();
}

/// Sets a breakpoint in the emitted Miden Assembly at the point this function is called.
#[inline(always)]
#[track_caller]
pub fn breakpoint() {
unsafe {
extern_break();
}
}
1 change: 1 addition & 0 deletions sdk/stdlib-sys/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use core::ops::{Deref, DerefMut};

pub mod debug;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I understand why it's public.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike the other two modules, which we're wildcard re-exporting the contents of from the root, we only need to export the debug module and require fully-qualified paths instead to access its declarations.

mod felt;
mod word;

Expand Down
Loading