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.

Sign up for GitHub

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

Jump to bottom

fix: Use plain integer addresses for opcodes in DAP disassembly view #4941

Merged
merged 4 commits into from
May 22, 2024
Merged

fix: Use plain integer addresses for opcodes in DAP disassembly view #4941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
14518f8
For disassembly view in DAP mode, return integer addresses
ggiraldez Apr 26, 2024
c0b713d
Fix unit tests
ggiraldez Apr 26, 2024
8e1427c
Update breakpoint set commands to handle new opcode addressing scheme
ggiraldez Apr 29, 2024
7f36701
Merge remote-tracking branch 'upstream/master' into fix/4904-dap-disa…
ggiraldez May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
259 changes: 97 additions & 162 deletions tooling/debugger/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ pub(super) struct DebugContext<'a, B: BlackBoxFunctionSolver> {
breakpoints: HashSet<OpcodeLocation>,
source_to_opcodes: BTreeMap<FileId, Vec<(usize, OpcodeLocation)>>,
unconstrained_functions: &'a [BrilligBytecode],

// Absolute (in terms of all the opcodes ACIR+Brillig) addresses of the ACIR
// opcodes with one additional entry for to indicate the last valid address.
acir_opcode_addresses: Vec<usize>,
}

impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> {
Expand All @@ -46,6 +50,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> {
unconstrained_functions: &'a [BrilligBytecode],
) -> Self {
let source_to_opcodes = build_source_to_opcode_debug_mappings(debug_artifact);
let acir_opcode_addresses = build_acir_opcode_offsets(circuit, unconstrained_functions);
Self {
// TODO: need to handle brillig pointer in the debugger
acvm: ACVM::new(
Expand All @@ -61,6 +66,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> {
breakpoints: HashSet::new(),
source_to_opcodes,
unconstrained_functions,
acir_opcode_addresses,
}
}

Expand Down Expand Up @@ -213,111 +219,55 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> {
.collect()
}

fn get_opcodes_sizes(&self) -> Vec<usize> {
self.get_opcodes()
.iter()
.map(|opcode| match opcode {
Opcode::BrilligCall { id, .. } => {
self.unconstrained_functions[*id as usize].bytecode.len()
}
_ => 1,
})
.collect()
/// Returns the absolute address of the opcode at the given location.
/// Absolute here means accounting for nested Brillig opcodes in BrilligCall
/// opcodes.
pub fn opcode_location_to_address(&self, location: &OpcodeLocation) -> usize {
match location {
OpcodeLocation::Acir(acir_index) => self.acir_opcode_addresses[*acir_index],
OpcodeLocation::Brillig { acir_index, brillig_index } => {
self.acir_opcode_addresses[*acir_index] + *brillig_index
}
}
}

/// Offsets the given location by the given number of opcodes (including
/// Brillig opcodes). If the offset would move the location outside of a
/// valid circuit location, returns None and the number of remaining
/// opcodes/instructions left which span outside the valid range in the
/// second element of the returned tuple.
pub(super) fn offset_opcode_location(
&self,
location: &Option<OpcodeLocation>,
mut offset: i64,
) -> (Option<OpcodeLocation>, i64) {
if offset == 0 {
return (*location, 0);
pub fn address_to_opcode_location(&self, address: usize) -> Option<OpcodeLocation> {
if address >= *self.acir_opcode_addresses.last().unwrap_or(&0) {
return None;
}
let Some(location) = location else {
return (None, offset);
};

let (mut acir_index, mut brillig_index) = match location {
OpcodeLocation::Acir(acir_index) => (*acir_index, 0),
OpcodeLocation::Brillig { acir_index, brillig_index } => (*acir_index, *brillig_index),
};
let opcode_sizes = self.get_opcodes_sizes();
if offset > 0 {
while offset > 0 {
let opcode_size = opcode_sizes[acir_index] as i64 - brillig_index as i64;
if offset >= opcode_size {
acir_index += 1;
offset -= opcode_size;
brillig_index = 0;
} else {
brillig_index += offset as usize;
offset = 0;
}
if acir_index >= opcode_sizes.len() {
return (None, offset);
}
let location = match self.acir_opcode_addresses.binary_search(&address) {
Ok(found_index) => OpcodeLocation::Acir(found_index),
Err(insert_index) => {
let acir_index = insert_index - 1;
let base_offset = self.acir_opcode_addresses[acir_index];
let brillig_index = address - base_offset;
OpcodeLocation::Brillig { acir_index, brillig_index }
}
} else {
while offset < 0 {
if brillig_index > 0 {
if brillig_index > (-offset) as usize {
brillig_index -= (-offset) as usize;
offset = 0;
} else {
offset += brillig_index as i64;
brillig_index = 0;
}
} else {
if acir_index == 0 {
return (None, offset);
}
acir_index -= 1;
let opcode_size = opcode_sizes[acir_index] as i64;
if opcode_size <= -offset {
offset += opcode_size;
} else {
brillig_index = (opcode_size + offset) as usize;
offset = 0;
}
}
}
}
if brillig_index > 0 {
(Some(OpcodeLocation::Brillig { acir_index, brillig_index }), 0)
} else {
(Some(OpcodeLocation::Acir(acir_index)), 0)
}
};
Some(location)
}

pub(super) fn render_opcode_at_location(&self, location: &Option<OpcodeLocation>) -> String {
pub(super) fn render_opcode_at_location(&self, location: &OpcodeLocation) -> String {
let opcodes = self.get_opcodes();
match location {
None => String::from("invalid"),
Some(OpcodeLocation::Acir(acir_index)) => {
OpcodeLocation::Acir(acir_index) => {
let opcode = &opcodes[*acir_index];
match opcode {
Opcode::BrilligCall { id, .. } => {
let first_opcode = &self.unconstrained_functions[*id as usize].bytecode[0];
format!("BRILLIG CALL {first_opcode:?}")
format!("BRILLIG {first_opcode:?}")
}
_ => format!("{opcode:?}"),
}
}
Some(OpcodeLocation::Brillig { acir_index, brillig_index }) => {
match &opcodes[*acir_index] {
Opcode::BrilligCall { id, .. } => {
let bytecode = &self.unconstrained_functions[*id as usize].bytecode;
let opcode = &bytecode[*brillig_index];
format!(" | {opcode:?}")
}
_ => String::from(" | invalid"),
OpcodeLocation::Brillig { acir_index, brillig_index } => match &opcodes[*acir_index] {
Opcode::BrilligCall { id, .. } => {
let bytecode = &self.unconstrained_functions[*id as usize].bytecode;
let opcode = &bytecode[*brillig_index];
format!(" | {opcode:?}")
}
}
_ => String::from(" | invalid"),
},
}
}

Expand Down Expand Up @@ -640,6 +590,28 @@ fn build_source_to_opcode_debug_mappings(
result
}

fn build_acir_opcode_offsets(
circuit: &Circuit,
unconstrained_functions: &[BrilligBytecode],
) -> Vec<usize> {
let mut result = Vec::with_capacity(circuit.opcodes.len() + 1);
// address of the first opcode is always 0
result.push(0);
circuit.opcodes.iter().fold(0, |acc, opcode| {
let acc = acc
+ match opcode {
Opcode::BrilligCall { id, .. } => {
unconstrained_functions[*id as usize].bytecode.len()
}
_ => 1,
};
// push the starting address of the next opcode
result.push(acc);
acc
});
result
}

// TODO: update all debugger tests to use unconstrained brillig pointers
#[cfg(test)]
mod tests {
Expand Down Expand Up @@ -851,7 +823,7 @@ mod tests {
}

#[test]
fn test_offset_opcode_location() {
fn test_address_opcode_location_mapping() {
let brillig_bytecode = BrilligBytecode {
bytecode: vec![
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 },
Expand Down Expand Up @@ -879,85 +851,48 @@ mod tests {
brillig_funcs,
);

assert_eq!(context.offset_opcode_location(&None, 0), (None, 0));
assert_eq!(context.offset_opcode_location(&None, 2), (None, 2));
assert_eq!(context.offset_opcode_location(&None, -2), (None, -2));
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 0),
(Some(OpcodeLocation::Acir(0)), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 1),
(Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 1 }), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 2),
(Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 }), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 3),
(Some(OpcodeLocation::Acir(1)), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 4),
(Some(OpcodeLocation::Acir(2)), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 5),
(Some(OpcodeLocation::Brillig { acir_index: 2, brillig_index: 1 }), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 7),
(Some(OpcodeLocation::Acir(3)), 0)
);
assert_eq!(context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 8), (None, 0));
assert_eq!(context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), 20), (None, 12));
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(1)), 2),
(Some(OpcodeLocation::Brillig { acir_index: 2, brillig_index: 1 }), 0)
);
assert_eq!(context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), -1), (None, -1));
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(0)), -10),
(None, -10)
);
let locations =
(0..=7).map(|address| context.address_to_opcode_location(address)).collect::<Vec<_>>();

// mapping from addresses to opcode locations
assert_eq!(
context.offset_opcode_location(
&Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 1 }),
-1
),
(Some(OpcodeLocation::Acir(0)), 0)
);
assert_eq!(
context.offset_opcode_location(
&Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 }),
-2
),
(Some(OpcodeLocation::Acir(0)), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(1)), -3),
(Some(OpcodeLocation::Acir(0)), 0)
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(2)), -4),
(Some(OpcodeLocation::Acir(0)), 0)
);
assert_eq!(
context.offset_opcode_location(
&Some(OpcodeLocation::Brillig { acir_index: 2, brillig_index: 1 }),
-5
),
(Some(OpcodeLocation::Acir(0)), 0)
locations,
vec![
Some(OpcodeLocation::Acir(0)),
Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 1 }),
Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 }),
Some(OpcodeLocation::Acir(1)),
Some(OpcodeLocation::Acir(2)),
Some(OpcodeLocation::Brillig { acir_index: 2, brillig_index: 1 }),
Some(OpcodeLocation::Brillig { acir_index: 2, brillig_index: 2 }),
Some(OpcodeLocation::Acir(3)),
]
);

let addresses = locations
.iter()
.flatten()
.map(|location| context.opcode_location_to_address(location))
.collect::<Vec<_>>();

// and vice-versa
assert_eq!(addresses, (0..=7).collect::<Vec<_>>());

// check edge cases
assert_eq!(None, context.address_to_opcode_location(8));
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(3)), -7),
(Some(OpcodeLocation::Acir(0)), 0)
0,
context.opcode_location_to_address(&OpcodeLocation::Brillig {
acir_index: 0,
brillig_index: 0
})
);
assert_eq!(
context.offset_opcode_location(&Some(OpcodeLocation::Acir(2)), -2),
(Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 }), 0)
4,
context.opcode_location_to_address(&OpcodeLocation::Brillig {
acir_index: 2,
brillig_index: 0
})
);
}
}