Debug: Cleanup handling of breakpoints and stepping. (#2181)
diff --git a/probe-rs/src/bin/probe-rs/cmd/dap_server/debug_adapter/dap/adapter.rs b/probe-rs/src/bin/probe-rs/cmd/dap_server/debug_adapter/dap/adapter.rs index f6f88de..99e2025 100644 --- a/probe-rs/src/bin/probe-rs/cmd/dap_server/debug_adapter/dap/adapter.rs +++ b/probe-rs/src/bin/probe-rs/cmd/dap_server/debug_adapter/dap/adapter.rs
@@ -1655,7 +1655,7 @@ { Ok((new_status, program_counter)) => (new_status, program_counter), Err(error) => match &error { - probe_rs::debug::DebugError::NoValidHaltLocation { + probe_rs::debug::DebugError::IncompleteDebugInfo { message, pc_at_error, } => {
diff --git a/probe-rs/src/bin/probe-rs/cmd/dap_server/server/core_data.rs b/probe-rs/src/bin/probe-rs/cmd/dap_server/server/core_data.rs index 0223f30..38f41cc 100644 --- a/probe-rs/src/bin/probe-rs/cmd/dap_server/server/core_data.rs +++ b/probe-rs/src/bin/probe-rs/cmd/dap_server/server/core_data.rs
@@ -19,10 +19,10 @@ util::rtt::RttConfig, }; use anyhow::{anyhow, Result}; +use probe_rs::debug::VerifiedBreakpoint; use probe_rs::{ debug::{ debug_info::DebugInfo, stack_frame::StackFrameInfo, ColumnType, ObjectRef, VariableCache, - VerifiedBreakpoint, }, rtt::{Rtt, ScanRegion}, Core, CoreStatus, Error, HaltReason,
diff --git a/probe-rs/src/debug/debug_info.rs b/probe-rs/src/debug/debug_info.rs index 7958f27..f94e470 100644 --- a/probe-rs/src/debug/debug_info.rs +++ b/probe-rs/src/debug/debug_info.rs
@@ -1,19 +1,21 @@ use super::{ function_die::FunctionDie, get_object_reference, unit_info::UnitInfo, variable::*, DebugError, - DebugRegisters, SourceLocation, StackFrame, VariableCache, + DebugRegisters, StackFrame, VariableCache, }; use crate::core::UnwindRule; -use crate::debug::source_statement::SourceStatement; +use crate::debug::source_instructions::InstructionLocation; use crate::debug::stack_frame::StackFrameInfo; +use crate::debug::unit_info::RangeExt; +use crate::debug::{ColumnType, SourceLocation, VerifiedBreakpoint}; use crate::{ core::{ExceptionInterface, RegisterRole, RegisterValue}, - debug::{registers, source_statement::SourceStatements}, + debug::{registers, source_instructions::InstructionSequence}, MemoryInterface, }; use anyhow::anyhow; use gimli::{ - BaseAddresses, ColumnType, DebugFrame, FileEntry, LineProgramHeader, UnwindContext, - UnwindSection, UnwindTableRow, + BaseAddresses, DebugFrame, FileEntry, LineProgramHeader, UnwindContext, UnwindSection, + UnwindTableRow, }; use object::read::{Object, ObjectSection}; use probe_rs_target::InstructionSet; @@ -30,18 +32,6 @@ pub(crate) type DwarfReader = gimli::read::EndianRcSlice<gimli::LittleEndian>; -/// Capture the required information when a breakpoint is set based on a requested source location. -/// It is possible that the requested source location cannot be resolved to a valid instruction address, -/// in which case the first 'valid' instruction address will be used, and the source location will be -/// updated to reflect the actual source location, not the requested source location. -#[derive(Clone, Debug)] -pub struct VerifiedBreakpoint { - /// The address in target memory, where the breakpoint was set. - pub address: u64, - /// If the breakpoint request was for a specific source location, then this field will contain the resolved source location. - pub source_location: SourceLocation, -} - /// Debug information which is parsed from DWARF debugging information. pub struct DebugInfo { pub(crate) dwarf: gimli::Dwarf<DwarfReader>, @@ -95,8 +85,14 @@ while let Ok(Some(header)) = iter.next() { if let Ok(unit) = dwarf_cow.unit(header) { - // TODO: maybe it's not correct to read from arbitrary units + // The DWARF V5 standard, section 2.4 specifies that the address size + // for the object file (or the target architecture default) will be used for + // DWARF debugging information. + // The following line is a workaround for instances where the address size of the + // CIE (Common Information Entry) is not correctly set. + // The frame section address size is only used for CIE versions before 4. frame_section.set_address_size(unit.encoding().address_size); + unit_infos.push(UnitInfo::new(unit)); }; } @@ -152,8 +148,8 @@ Ok(ranges) => ranges, Err(error) => { tracing::warn!( - "No valid source code ranges found for address {}: {:?}", - address, + "No valid source code ranges found for unit {:?}: {:?}", + unit.dwo_name(), error ); continue; @@ -164,8 +160,7 @@ if !(range.begin <= address && address < range.end) { continue; } - // Get the function name. - + // Get the DWARF LineProgram. let ilnp = unit.line_program.as_ref()?.clone(); let (program, sequences) = match ilnp.sequences() { @@ -216,8 +211,6 @@ column: Some(previous_row.column().into()), file, directory, - low_pc: Some(target_seq.start as u32), - high_pc: Some(target_seq.end as u32), }); } } @@ -236,8 +229,6 @@ column: Some(row.column().into()), file, directory, - low_pc: Some(target_seq.start as u32), - high_pc: Some(target_seq.end as u32), }); } } @@ -873,6 +864,7 @@ /// Find the program counter where a breakpoint should be set, /// given a source file, a line and optionally a column. + // TODO: Move (and fix) this to the [`InstructionSequence::for_source_location`] method. pub fn get_breakpoint_location( &self, path: &TypedPathBuf, @@ -888,25 +880,25 @@ .unwrap_or_else(|| "-".to_owned()) ); - for unit_header in &self.unit_infos { - let Some(ref line_program) = &unit_header.unit.line_program else { + // We need to look through all the compilation units, and inspect their line programs, + // to determine the correct address for the breakpoint. + for progam_unit in &self.unit_infos { + // TODO: We do NOT need to look up the line program here. + let Some(ref line_program) = &progam_unit.unit.line_program else { continue; }; if let Some(location) = - self.get_breakpoint_location_in_unit(unit_header, line_program, path, line, column)? + self.get_breakpoint_location_in_unit(progam_unit, line_program, path, line, column)? { return Ok(location); }; } - let p = path.to_path(); - + // If we get here, it means we didn't find any valid breakpoint locations. Err(DebugError::Other(anyhow::anyhow!( - "No valid breakpoint information found for file: {}, line: {:?}, column: {:?}", - p.display(), - line, - column + "No valid breakpoint information found for file: {}, line: {line:?}, column: {column:?}", + path.to_path().display() ))) } @@ -921,7 +913,7 @@ let unit = &unit_header.unit; let header: &LineProgramHeader<GimliReader, usize> = line_program.header(); - // Check if any of the file names in the header match the path we are looking for. + // Early return, if none of the file names in the header match the path we are looking for. if !header.file_names().iter().any(|file_name| { let combined_path = self.get_path(unit, header, file_name); @@ -935,12 +927,13 @@ let mut rows = line_program.clone().rows(); while let Some((header, row)) = rows.next_row()? { + // If the row is not in the same file, we can skip it. let row_path = row .file(header) .and_then(|file_entry| self.get_path(unit, header, file_entry)); - if row_path - .map(|p| !canonical_path_eq(path, &p)) + .as_ref() + .map(|p| !canonical_path_eq(path, p)) .unwrap_or(true) { continue; @@ -954,18 +947,17 @@ continue; } - // The first match of the file and row will be used to build the SourceStatements, and then: + let instruction_sequence = InstructionSequence::for_address(self, row.address())?; + + // The first match of the file and row will be used to build the InstructionSequence, and then: // 1. If there is an exact column match, we will use the low_pc of the statement at that column and line. // 2. If there is no exact column match, we use the first available statement in the line. - let source_statements = - SourceStatements::new(self, unit_header, row.address())?.statements; - - let halt_address_and_location = |source_statement: &SourceStatement| { + let halt_address_and_location = |instruction_location: &InstructionLocation| { ( - source_statement.low_pc(), + instruction_location.address, line_program .header() - .file(source_statement.file_index) + .file(instruction_location.file_index) .and_then(|file_entry| { self.find_file_and_directory( &unit_header.unit, @@ -973,25 +965,22 @@ file_entry, ) .map(|(file, directory)| SourceLocation { - line: source_statement.line.map(std::num::NonZeroU64::get), - column: Some(source_statement.column.into()), + line: instruction_location.line.map(std::num::NonZeroU64::get), + column: Some(instruction_location.column), file, directory, - low_pc: Some(source_statement.low_pc() as u32), - high_pc: Some(source_statement.instruction_range.end as u32), }) }), ) }; - let first_find = source_statements.iter().find(|statement| { + // The case where we have exact match on file, line AND column. + let first_find = instruction_sequence.instructions.iter().find(|statement| { column - .and_then(NonZeroU64::new) .map(ColumnType::Column) .map_or(false, |col| col == statement.column) && statement.line == Some(cur_line) }); - if let Some((halt_address, Some(halt_location))) = first_find.map(halt_address_and_location) { @@ -1000,7 +989,10 @@ source_location: halt_location, })); } - let second_find = source_statements + + // The fallback case where we have exact match on file and line, but no column. + let second_find = instruction_sequence + .instructions .iter() .find(|statement| statement.line == Some(cur_line)); @@ -1075,6 +1067,29 @@ Some((file_name, directory)) } + + // Return the compilation unit that contains the given address + pub(crate) fn compile_unit_info( + &self, + address: u64, + ) -> Result<&super::unit_info::UnitInfo, DebugError> { + for header in &self.unit_infos { + match self.dwarf.unit_ranges(&header.unit) { + Ok(mut ranges) => { + while let Ok(Some(range)) = ranges.next() { + if range.contains(address) { + return Ok(header); + } + } + } + Err(_) => continue, + }; + } + Err(DebugError::IncompleteDebugInfo{ + message: "No debug information available for the specified instruction address. Please consider using instruction level stepping.".to_string(), + pc_at_error: address, + }) + } } /// Uses the [std::fs::canonicalize] function to canonicalize both paths before applying the [std::path::PathBuf::eq]
diff --git a/probe-rs/src/debug/debug_step.rs b/probe-rs/src/debug/debug_step.rs index 1972839..5859ccb 100644 --- a/probe-rs/src/debug/debug_step.rs +++ b/probe-rs/src/debug/debug_step.rs
@@ -1,7 +1,5 @@ use super::{ - debug_info::DebugInfo, - source_statement::SourceStatements, - {DebugError, SourceLocation}, + debug_info::DebugInfo, source_instructions::InstructionSequence, DebugError, VerifiedBreakpoint, }; use crate::{ architecture::{ @@ -73,7 +71,7 @@ return Ok((core_status, program_counter)); } SteppingMode::IntoStatement => { - self.get_halt_location(core, debug_info, program_counter, None) + self.get_halt_location(core, debug_info, program_counter, Some(return_address)) } SteppingMode::BreakPoint => { self.get_halt_location(core, debug_info, program_counter, None) @@ -82,8 +80,8 @@ self.get_halt_location(core, debug_info, program_counter, Some(return_address)) } } { - Ok((post_step_target_address, _)) => { - target_address = post_step_target_address; + Ok(post_step_target) => { + target_address = Some(post_step_target.address); // Re-read the program_counter, because it may have changed during the `get_halt_location` call. program_counter = core .read_core_reg(core.program_counter().id())? @@ -91,7 +89,7 @@ break; } Err(error) => match error { - DebugError::NoValidHaltLocation { + DebugError::IncompleteDebugInfo { message, pc_at_error, } => { @@ -144,7 +142,7 @@ run_to_address(program_counter, target_address, core)? } None => { - return Err(DebugError::NoValidHaltLocation { + return Err(DebugError::IncompleteDebugInfo { message: "Unable to determine target address for this step request." .to_string(), pc_at_error: program_counter, @@ -156,23 +154,17 @@ /// To understand how this method works, use the following framework: /// - Everything is calculated from a given machine instruction address, usually the current program counter. - /// - To calculate where the user might step to (step-over, step-into, step-out), we start from the given instruction address/program counter, and work our way through all the rows in the sequence of instructions it is part of. - /// - A sequence of instructions represents a series of contiguous target machine instructions, and does not necessarily represent the whole of a function. - /// - Similarly, the instructions belonging to a source statement are not necessarily contiguous inside the sequence of instructions (e.g. conditional branching inside the sequence). - /// - /// - /// - The next row address in the target processor's instruction sequence may qualify as (one, or more) of the following: - /// - The start of a new source statement (a source file may have multiple statements on a single line) - /// - Another instruction that is part of the source statement started previously - /// - The first instruction after the end of the sequence epilogue. - /// - The end of the current sequence of instructions. - /// - DWARF defines other flags that are not relevant/used here. - /// - /// - /// - Depending on the combinations of the above, we only use instructions that qualify as: + /// - To calculate where the user might step to (step-over, step-into, step-out), we start from the given instruction + /// address/program counter, and work our way through all the rows in the sequence of instructions it is part of. + /// - A sequence of instructions represents a series of monotonically increasing target machine instructions, + /// and does not necessarily represent the whole of a function. + /// - Similarly, the instructions belonging to a sequence are not necessarily contiguous inside the sequence of instructions, + /// e.g. conditional branching inside the sequence. + /// - To determine valid halt points for breakpoints and stepping, we only use instructions that qualify as: /// - The beginning of a statement that is neither inside the prologue, nor inside the epilogue. /// - Based on this, we will attempt to return the "most appropriate" address for the [`SteppingMode`], given the available information in the instruction sequence. /// All data is calculated using the [`gimli::read::CompleteLineProgram`] as well as, function call data from the debug info frame section. + /// /// NOTE about errors returned: Sometimes the target program_counter is at a location where the debug_info program row data does not contain valid statements /// for halt points, and we will return a `DebugError::NoValidHaltLocation`. In this case, we recommend the consumer of this API step the core to the next instruction /// and try again, with a reasonable retry limit. All other error kinds are should be treated as non recoverable errors. @@ -182,145 +174,100 @@ debug_info: &DebugInfo, program_counter: u64, return_address: Option<u64>, - ) -> Result<(Option<u64>, Option<SourceLocation>), DebugError> { - let program_unit = get_compile_unit_info(debug_info, program_counter)?; + ) -> Result<VerifiedBreakpoint, DebugError> { + let program_unit = debug_info.compile_unit_info(program_counter)?; match self { SteppingMode::BreakPoint => { // Find the first_breakpoint_address - for source_statement in - SourceStatements::new(debug_info, program_unit, program_counter)?.statements - { - if let Some(halt_address) = - source_statement.get_first_halt_address(program_counter) - { - tracing::debug!( - "Found first breakpoint {:#010x} for address: {:#010x}", - halt_address, - program_counter - ); - // We have a good first halt address. - let first_breakpoint_address = Some(halt_address); - let first_breakpoint_source_location = program_unit - .unit - .line_program - .as_ref() - .and_then(|line_program| { - line_program - .header() - .file(source_statement.file_index) - .and_then(|file_entry| { - debug_info - .find_file_and_directory( - &program_unit.unit, - line_program.header(), - file_entry, - ) - .map(|(file, directory)| SourceLocation { - line: source_statement - .line - .map(std::num::NonZeroU64::get), - column: Some(source_statement.column.into()), - file, - directory, - low_pc: Some(source_statement.low_pc() as u32), - high_pc: Some( - source_statement.instruction_range.end as u32, - ), - }) - }) - }); - return Ok((first_breakpoint_address, first_breakpoint_source_location)); - } - } + let instruction_sequence = + InstructionSequence::for_address(debug_info, program_counter)?; + return instruction_sequence.get_first_breakpoint(program_counter); } SteppingMode::OverStatement => { // Find the next_statement_address - // - The instructions in a source statement are not necessarily contiguous in the sequence, and the next_statement_address may be affected by conditonal branching at runtime. - // - Therefore, in order to find the correct next_statement_address, we iterate through the source statements, and : - // -- Find the starting address of the next `statement` in the source statements. - // -- If there is one, it means the step over target is in the current sequence, so we get the get_first_halt_address() for this next statement. - // -- Otherwise the step over target is the same as the step out target. - let source_statements = - SourceStatements::new(debug_info, program_unit, program_counter)?.statements; - let mut source_statements_iter = source_statements.iter(); - if let Some((target_address, target_location)) = source_statements_iter - .find(|source_statement| { - source_statement - .instruction_range - .contains(&program_counter) - }) - .and_then(|_| { - if source_statements.len() == 1 { - // Force a SteppingMode::OutOfStatement below. - None - } else { - source_statements_iter.next().and_then(|next_line| { - SteppingMode::BreakPoint - .get_halt_location(core, debug_info, next_line.low_pc(), None) - .ok() - }) - } - }) - .or_else(|| { - SteppingMode::OutOfStatement - .get_halt_location(core, debug_info, program_counter, return_address) - .ok() - }) - { - return Ok((target_address, target_location)); - } + // - The instructions in a sequence do not necessarily have contiguous addresses, + // and the next instruction address may be affected by conditonal branching at runtime. + // - Therefore, in order to find the correct next_statement_address, we iterate through the + // instructions to find the starting address of the next halt location in the source instructions. + // -- If there is one, it means the step over target is in the current sequence, + // so we get the valid breakpoint location for this next location. + // -- If there is not one, the step over target is the same as the step out target. + let instruction_sequence = + InstructionSequence::for_address(debug_info, program_counter)?; + + // Find the breakpoint_address where the address is > program_counter (not inclusive). + // We do this by by forcing the `get_first_breakpoint` method to look for an address + // that is greater than the current program_counter. + return instruction_sequence + .get_first_breakpoint(program_counter.checked_add(1).unwrap_or(program_counter)) + .or_else(|_| { + // If we cannot find a valid breakpoint in the current sequence, we will step out of the current sequence. + SteppingMode::OutOfStatement.get_halt_location( + core, + debug_info, + program_counter, + return_address, + ) + }); } SteppingMode::IntoStatement => { - // This is a tricky case because the current RUST generated DWARF, does not store the DW_TAG_call_site information described in the DWARF 5 standard. It is not a mandatory attribute, so not sure if we can ever expect it. + // This is a tricky case because the current RUST generated DWARF, does not store the DW_TAG_call_site information described in the DWARF 5 standard. + // - It is not a mandatory attribute, so not sure if we can ever expect it. // To find if any functions are called from the current program counter: - // 1. Find the statement with the address corresponding to the current PC, - // 2. Single step the target core, until either ... - // (a) We hit a PC that is NOT in the address range of the current statement. This location, which could be any of the following: - // (a.i) A legitimate branch outside the current sequence (call to another instruction) such as a explicit call to a function, or something the compiler injected, like a `drop()`, - // (a.ii) An interrupt handler diverted the processing. - // (b) We hit a PC that matches the end of the address range, which means there was nothing to step into, so the target is now halted (correctly) at the next statement. - // TODO: In theory, we could disassemble the instructions in this statement's address range, and find branching instructions, then we would not need to single step the core past the original haltpoint. - - let source_statements = - SourceStatements::new(debug_info, program_unit, program_counter)?.statements; - let mut source_statements_iter = source_statements.iter(); - if let Some(current_source_statement) = - source_statements_iter.find(|source_statement| { - source_statement - .instruction_range - .contains(&program_counter) - }) + // 1. Identify the next instruction location after the instruction corresponding to the current PC, + // 2. Single step the target core, until either of the following: + // (a) We hit a PC that is NOT in the range between the current PC and the next instruction location. + // This location, which could be any of the following: + // (a.i) A legitimate branch outside the current sequence (call to another instruction) such as + // an explicit call to a function, or something the compiler injected, like a `drop()`, + // (a.ii) An interrupt handler diverted the processing. + // (b) We hit a PC at the address of the identified next instruction location, + // which means there was nothing to step into, so the target is now halted (correctly) at the next statement. + let instruction_sequence = + InstructionSequence::for_address(debug_info, program_counter)?; + let target_pc = match instruction_sequence + .get_first_breakpoint(program_counter.checked_add(1).unwrap_or(program_counter)) { - let inclusive_range = current_source_statement.instruction_range.start - ..=current_source_statement.instruction_range.end; - let (core_status, new_pc) = step_to_address(inclusive_range, core)?; - if new_pc == current_source_statement.instruction_range.end { - // We have halted at the address after the current statement, so we can conclude there was no branching calls in this sequence. - tracing::debug!("Stepping into next statement, but no branching calls found. Stepped to next available statement."); - } else if new_pc < current_source_statement.instruction_range.end - && matches!(core_status, CoreStatus::Halted(HaltReason::Breakpoint(_))) - { - // We have halted at a PC that is within the current statement, so there must be another breakpoint. - tracing::debug!( - "Stepping into next statement, but encountered a breakpoint." - ); - } else { - // We have reached a location that is not in the current statement range (branch instruction or breakpoint in an interrupt handler). - tracing::debug!( - "Stepping into next statement at address: {:#010x}.", - new_pc - ); + Ok(identified_next_breakpoint) => identified_next_breakpoint.address, + Err(DebugError::IncompleteDebugInfo { .. }) => { + // There are no next statements in this sequence, so we will use the return address as the target. + if let Some(return_address) = return_address { + return_address + } else { + return Err(DebugError::IncompleteDebugInfo { + message: + "Could not determine a 'step in' target. Please use 'step over'." + .to_string(), + pc_at_error: program_counter, + }); + } } + Err(other_error) => { + return Err(other_error); + } + }; - return SteppingMode::BreakPoint - .get_halt_location(core, debug_info, new_pc, None); + let (core_status, new_pc) = + step_to_address(instruction_sequence.address_range.start..=target_pc, core)?; + if (program_counter..instruction_sequence.address_range.end).contains(&new_pc) { + // We have halted at an address after the current instruction, but inside the same sequence, + // so we can conclude there were no branching calls in this instruction. + tracing::debug!("Stepping into next statement, but no branching calls found. Stepped to next available statement."); + } else if matches!(core_status, CoreStatus::Halted(HaltReason::Breakpoint(_))) { + // We have halted at a PC that is within the current statement, so there must be another breakpoint. + tracing::debug!("Stepping into next statement, but encountered a breakpoint."); + } else { + tracing::debug!("Stepping into next statement at address: {:#010x}.", new_pc); } + + return SteppingMode::BreakPoint.get_halt_location(core, debug_info, new_pc, None); } SteppingMode::OutOfStatement => { if let Ok(function_dies) = program_unit.get_function_dies(debug_info, program_counter, true) { - // We want the first qualifying (PC is in range) function from the back of this list, to access the 'innermost' functions first. + // We want the first qualifying (PC is in range) function from the back of this list, + // to access the 'innermost' functions first. if let Some(function) = function_dies.iter().next_back() { tracing::trace!( "Step Out target: Evaluating function {:?}, low_pc={:?}, high_pc={:?}", @@ -369,10 +316,10 @@ } } - Err(DebugError::NoValidHaltLocation{ + Err(DebugError::IncompleteDebugInfo{ message: "Could not determine valid halt locations for this request. Please consider using instruction level stepping.".to_string(), pc_at_error: program_counter, - }) + }) } } @@ -389,7 +336,7 @@ ) -> Result<(CoreStatus, u64), DebugError> { Ok(if target_address < program_counter { // We are not able to calculate a step_out_address. Notify the user to try something else. - return Err(DebugError::NoValidHaltLocation { + return Err(DebugError::IncompleteDebugInfo { message: "Unable to determine target address for this step request. Please try a different form of stepping.".to_string(), pc_at_error: program_counter, }); @@ -450,10 +397,12 @@ } /// In some cases, we need to single-step the core, until ONE of the following conditions are met: -/// - We reach the `target_address_range.end()` (inclusive) -/// - We reach an address that is not in the sequential range of `target_address_range` (inclusive), i.e. we stepped to some kind of branch instruction. -/// - We reach some other legitimate halt point (e.g. the user tries to step past a series of statements, but there is another breakpoint active in that "gap") -/// - We encounter an error (e.g. the core locks up) +/// - We reach the `target_address_range.end()` +/// - We reach an address that is not in the sequential range of `target_address_range`, +/// i.e. we stepped to some kind of branch instruction, or diversion to an interrupt handler. +/// - We reach some other legitimate halt point (e.g. the user tries to step past a series of statements, +/// but there is another breakpoint active in that "gap") +/// - We encounter an error (e.g. the core locks up). fn step_to_address( target_address_range: RangeInclusive<u64>, core: &mut impl CoreInterface, @@ -471,10 +420,15 @@ break; } // This is a recoverable error kind, and can be reported to the user higher up in the call stack. - other_halt_reason => return Err(DebugError::NoValidHaltLocation{message: format!("Target halted unexpectedly before we reached the destination address of a step operation: {other_halt_reason:?}"), pc_at_error: core.read_core_reg(core.program_counter().id())?.try_into()?}), + other_halt_reason => return Err(DebugError::IncompleteDebugInfo{message: + format!("Target halted unexpectedly before we reached the destination address of a step operation: {other_halt_reason:?}"), + pc_at_error: core.read_core_reg(core.program_counter().id())?.try_into()?} + ), }, // This is not a recoverable error, and will result in the debug session ending (we have no predicatable way of successfully continuing the session) - other_status => return Err(DebugError::Other(anyhow::anyhow!("Target failed to reach the destination address of a step operation: {:?}", other_status))), + other_status => return Err(DebugError::Other( + anyhow::anyhow!("Target failed to reach the destination address of a step operation: {:?}", other_status)) + ), } } Ok(( @@ -483,26 +437,3 @@ .try_into()?, )) } - -/// Find the compile unit at the current address. -fn get_compile_unit_info( - debug_info: &DebugInfo, - program_counter: u64, -) -> Result<&super::unit_info::UnitInfo, DebugError> { - for header in &debug_info.unit_infos { - match debug_info.dwarf.unit_ranges(&header.unit) { - Ok(mut ranges) => { - while let Ok(Some(range)) = ranges.next() { - if (range.begin <= program_counter) && (range.end > program_counter) { - return Ok(header); - } - } - } - Err(_) => continue, - }; - } - Err(DebugError::NoValidHaltLocation{ - message: "The specified source location does not have any debug information available. Please consider using instruction level stepping.".to_string(), - pc_at_error: program_counter, - }) -}
diff --git a/probe-rs/src/debug/function_die.rs b/probe-rs/src/debug/function_die.rs index aea47b7..cdfb3f6 100644 --- a/probe-rs/src/debug/function_die.rs +++ b/probe-rs/src/debug/function_die.rs
@@ -121,8 +121,6 @@ column, file: Some(file), directory: Some(directory), - low_pc: Some(self.low_pc as u32), - high_pc: Some(self.high_pc as u32), }) }
diff --git a/probe-rs/src/debug/mod.rs b/probe-rs/src/debug/mod.rs index ca75cde..7223e34 100644 --- a/probe-rs/src/debug/mod.rs +++ b/probe-rs/src/debug/mod.rs
@@ -16,7 +16,7 @@ /// Target Register definitions, expanded from [`crate::core::registers::CoreRegister`] to include unwind specific information. pub mod registers; /// The source statement information used while identifying haltpoints for debug stepping and breakpoints. -pub(crate) mod source_statement; +pub(crate) mod source_instructions; /// The stack frame information used while unwinding the stack from a specific program counter. pub mod stack_frame; /// Information about a Unit in the debug information. @@ -27,7 +27,8 @@ pub mod variable_cache; pub use self::{ - debug_info::*, debug_step::SteppingMode, registers::*, stack_frame::StackFrame, variable::*, + debug_info::*, debug_step::SteppingMode, registers::*, source_instructions::SourceLocation, + source_instructions::VerifiedBreakpoint, stack_frame::StackFrame, variable::*, variable_cache::VariableCache, }; use crate::{core::Core, MemoryInterface}; @@ -38,7 +39,6 @@ use std::{ io, num::NonZeroU32, - path::PathBuf, str::Utf8Error, sync::atomic::{AtomicU32, Ordering}, vec, @@ -71,10 +71,12 @@ /// An int could not be created from the given string. #[error(transparent)] IntConversion(#[from] std::num::TryFromIntError), - /// Errors encountered while determining valid halt locations for breakpoints and stepping. - /// These are distinct from other errors because they terminate the current step, and result in a user message, but they do not interrupt the rest of the debug session. + /// Errors encountered while determining valid locations for memory addresses involved in actions like + /// setting breakpoints and/or stepping through source code. + /// These are distinct from other errors because they gracefully terminate the current action, + /// and result in a user message, but they do not interrupt the rest of the debug session. #[error("{message} @program_counter={:#010X}.", pc_at_error)] - NoValidHaltLocation { + IncompleteDebugInfo { /// A message that can be displayed to the user to help them make an informed recovery choice. message: String, /// The value of the program counter for which a halt was requested. @@ -111,6 +113,15 @@ } } +impl From<u64> for ColumnType { + fn from(column: u64) -> Self { + match column { + 0 => ColumnType::LeftEdge, + _ => ColumnType::Column(column), + } + } +} + /// Object reference as defined in the DAP standard. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] pub enum ObjectRef { @@ -171,64 +182,6 @@ ObjectRef::Valid(NonZeroU32::new(key).unwrap()) } -fn serialize_typed_path<S>(path: &Option<TypedPathBuf>, serializer: S) -> Result<S::Ok, S::Error> -where - S: serde::Serializer, -{ - match path { - Some(path) => serializer.serialize_str(&path.to_string_lossy()), - None => serializer.serialize_none(), - } -} - -/// A specific location in source code. -#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize)] -pub struct SourceLocation { - /// The line number in the source file with zero based indexing. - pub line: Option<u64>, - /// The column number in the source file with zero based indexing. - pub column: Option<ColumnType>, - /// The file name of the source file. - pub file: Option<String>, - /// The directory of the source file. - #[serde(serialize_with = "serialize_typed_path")] - pub directory: Option<TypedPathBuf>, - /// The address of the first instruction associated with the source code - pub low_pc: Option<u32>, - /// The address of the first location past the last instruction associated with the source code - pub high_pc: Option<u32>, -} - -impl SourceLocation { - /// The full path of the source file, combining the `directory` and `file` fields. - /// If the path does not resolve to an existing file, an error is returned. - pub fn combined_path(&self) -> Result<PathBuf, DebugError> { - let combined_path = self.combined_typed_path(); - - if let Some(native_path) = combined_path.and_then(|p| PathBuf::try_from(p).ok()) { - if native_path.exists() { - return Ok(native_path); - } - } - - Err(DebugError::Other(anyhow::anyhow!( - "Unable to find source file for directory {:?} and file {:?}", - self.directory, - self.file - ))) - } - - /// Get the full path of the source file - pub fn combined_typed_path(&self) -> Option<TypedPathBuf> { - let combined_path = self - .directory - .as_ref() - .and_then(|dir| self.file.as_ref().map(|file| dir.join(file))); - - combined_path - } -} - /// If file information is available, it returns `Some(directory:PathBuf, file_name:String)`, otherwise `None`. fn extract_file( debug_info: &DebugInfo,
diff --git a/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__RP2040__full_unwind.snap b/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__RP2040__full_unwind.snap index a332206..535605a 100644 --- a/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__RP2040__full_unwind.snap +++ b/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__RP2040__full_unwind.snap
@@ -9,8 +9,6 @@ Column: 13 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 268468400 - high_pc: 268468588 registers: - core_register: id: 0 @@ -234,8 +232,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 268468400 - high_pc: 268468588 registers: - core_register: id: 0 @@ -451,8 +447,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 268468400 - high_pc: 268468588 registers: - core_register: id: 0 @@ -668,8 +662,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 268468400 - high_pc: 268468588 registers: - core_register: id: 0 @@ -885,8 +877,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 268468400 - high_pc: 268468588 registers: - core_register: id: 0 @@ -1102,8 +1092,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 268468400 - high_pc: 268468588 registers: - core_register: id: 0 @@ -1319,8 +1307,6 @@ Column: 5 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 268465168 - high_pc: 268468400 registers: - core_register: id: 0 @@ -4154,8 +4140,6 @@ Column: 54 file: RP2040.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src/thumbv6m-none-eabi - low_pc: 268436744 - high_pc: 268437052 registers: - core_register: id: 0 @@ -4360,8 +4344,6 @@ Column: 1 file: RP2040.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src/thumbv6m-none-eabi - low_pc: 268436732 - high_pc: 268436742 registers: - core_register: id: 0
diff --git a/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__atsamd51p19a__full_unwind.snap b/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__atsamd51p19a__full_unwind.snap index 056b552..d105a44 100644 --- a/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__atsamd51p19a__full_unwind.snap +++ b/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__atsamd51p19a__full_unwind.snap
@@ -9,8 +9,6 @@ Column: 5 file: main.c directory: "C:\\_Hobby\\probe-rs-test-c-firmware" - low_pc: 5172 - high_pc: 5272 registers: - core_register: id: 0 @@ -246,8 +244,6 @@ Column: 1 file: main.c directory: "C:\\_Hobby\\probe-rs-test-c-firmware" - low_pc: 5272 - high_pc: 5376 registers: - core_register: id: 0 @@ -465,8 +461,6 @@ Column: 9 file: main.c directory: "C:\\_Hobby\\probe-rs-test-c-firmware" - low_pc: 5376 - high_pc: 5488 registers: - core_register: id: 0 @@ -685,8 +679,6 @@ Column: 15 file: startup_samd51.c directory: "C:\\_Hobby\\probe-rs-test-c-firmware\\Atmel\\Device_Startup" - low_pc: 2304 - high_pc: 2464 registers: - core_register: id: 0
diff --git a/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__nRF52833_xxAA__full_unwind.snap b/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__nRF52833_xxAA__full_unwind.snap index d74474a..da27721 100644 --- a/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__nRF52833_xxAA__full_unwind.snap +++ b/probe-rs/src/debug/snapshots/probe_rs__debug__debug_info__test__nRF52833_xxAA__full_unwind.snap
@@ -9,8 +9,6 @@ Column: 13 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 7284 - high_pc: 7494 registers: - core_register: id: 0 @@ -234,8 +232,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 7284 - high_pc: 7494 registers: - core_register: id: 0 @@ -451,8 +447,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 7284 - high_pc: 7494 registers: - core_register: id: 0 @@ -668,8 +662,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 7284 - high_pc: 7494 registers: - core_register: id: 0 @@ -885,8 +877,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 7284 - high_pc: 7494 registers: - core_register: id: 0 @@ -1102,8 +1092,6 @@ Column: 9 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 7284 - high_pc: 7494 registers: - core_register: id: 0 @@ -1319,8 +1307,6 @@ Column: 5 file: common_testing_code.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src - low_pc: 3816 - high_pc: 7284 registers: - core_register: id: 0 @@ -4154,8 +4140,6 @@ Column: 10 file: nRF52833_xxAA.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src/thumbv7em-none-eabihf - low_pc: 1344 - high_pc: 1666 registers: - core_register: id: 0 @@ -4361,8 +4345,6 @@ Column: 1 file: nRF52833_xxAA.rs directory: /Users/jacknoppe/dev/debug/probe-rs-debugger-test/src/thumbv7em-none-eabihf - low_pc: 1334 - high_pc: 1344 registers: - core_register: id: 0
diff --git a/probe-rs/src/debug/source_instructions.rs b/probe-rs/src/debug/source_instructions.rs new file mode 100644 index 0000000..cac071f --- /dev/null +++ b/probe-rs/src/debug/source_instructions.rs
@@ -0,0 +1,405 @@ +use super::{ + unit_info::{self, UnitInfo}, + ColumnType, DebugError, DebugInfo, +}; +use gimli::LineSequence; +use std::{ + fmt::{Debug, Formatter}, + num::NonZeroU64, + ops::Range, + path::PathBuf, +}; +use typed_path::TypedPathBuf; + +/// Capture the required information when a breakpoint is set based on a requested source location. +/// It is possible that the requested source location cannot be resolved to a valid instruction address, +/// in which case the first 'valid' instruction address will be used, and the source location will be +/// updated to reflect the actual source location, not the requested source location. +#[derive(Clone, Debug)] +pub struct VerifiedBreakpoint { + /// The address in target memory, where the breakpoint can be set. + pub address: u64, + /// If the breakpoint request was for a specific source location, then this field will contain the resolved source location. + pub source_location: SourceLocation, +} + +fn serialize_typed_path<S>(path: &Option<TypedPathBuf>, serializer: S) -> Result<S::Ok, S::Error> +where + S: serde::Serializer, +{ + match path { + Some(path) => serializer.serialize_str(&path.to_string_lossy()), + None => serializer.serialize_none(), + } +} + +/// A specific location in source code. +/// Each unique line, column, file and directory combination is a unique source location. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize)] +pub struct SourceLocation { + /// The line number in the source file with zero based indexing. + pub line: Option<u64>, + /// The column number in the source file with zero based indexing. + pub column: Option<ColumnType>, + /// The file name of the source file. + pub file: Option<String>, + /// The directory of the source file. + #[serde(serialize_with = "serialize_typed_path")] + pub directory: Option<TypedPathBuf>, +} + +impl SourceLocation { + /// Resolve debug information for a [`InstructionLocation`] and create a [`SourceLocation`]. + pub(crate) fn from_instruction_location( + debug_info: &DebugInfo, + program_unit: &unit_info::UnitInfo, + instruction_location: &InstructionLocation, + ) -> Option<SourceLocation> { + let line_program = program_unit.unit.line_program.as_ref()?; + let file_entry = line_program + .header() + .file(instruction_location.file_index)?; + debug_info + .find_file_and_directory(&program_unit.unit, line_program.header(), file_entry) + .map(|(file, directory)| SourceLocation { + line: instruction_location.line.map(std::num::NonZeroU64::get), + column: Some(instruction_location.column), + file, + directory, + }) + } + + /// The full path of the source file, combining the `directory` and `file` fields. + /// If the path does not resolve to an existing file, an error is returned. + pub fn combined_path(&self) -> Result<PathBuf, DebugError> { + let combined_path = self.combined_typed_path(); + + if let Some(native_path) = combined_path.and_then(|p| PathBuf::try_from(p).ok()) { + if native_path.exists() { + return Ok(native_path); + } + } + + Err(DebugError::Other(anyhow::anyhow!( + "Unable to find source file for directory {:?} and file {:?}", + self.directory, + self.file + ))) + } + + /// Get the full path of the source file + pub fn combined_typed_path(&self) -> Option<TypedPathBuf> { + let combined_path = self + .directory + .as_ref() + .and_then(|dir| self.file.as_ref().map(|file| dir.join(file))); + + combined_path + } +} + +/// Keep track of all the instruction locations required to satisfy the operations of [`SteppingMode`]. +/// This is a list of target instructions, belonging to a [`gimli::LineSequence`], +/// and filters it to only user code instructions (no prologue code, and no non-statement instructions), +/// so that we are left only with what DWARF terms as "recommended breakpoint location". +pub struct InstructionSequence<'debug_info> { + /// The `address_range.start` is the starting address of the program counter for which this sequence is valid, + /// and allows us to identify target instruction locations where the program counter lies inside the prologue. + /// The `address_range.end` is the first address that is not covered by this sequence within the line number program, + /// and allows us to identify when stepping over a instruction location would result in leaving a sequence. + pub(crate) address_range: Range<u64>, + // NOTE: Use Vec as a container, because we will have relatively few statements per sequence, and we need to maintain the order. + pub(crate) instructions: Vec<InstructionLocation>, + // The following private fields are required to resolve the source location information for + // each instruction location. + debug_info: &'debug_info DebugInfo, + program_unit: &'debug_info UnitInfo, +} + +impl Debug for InstructionSequence<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + writeln!( + f, + "Instruction Sequence with address range: {:#010x} - {:#010x}", + self.address_range.start, self.address_range.end + )?; + for instruction_location in &self.instructions { + writeln!(f, "\t{instruction_location:?}")?; + } + Ok(()) + } +} + +impl<'debug_info> InstructionSequence<'debug_info> { + /// Extract all the instruction locations, belonging to the active sequence (i.e. the sequence that contains the `program_counter`). + pub(crate) fn for_address( + debug_info: &'debug_info DebugInfo, + program_counter: u64, + ) -> Result<Self, DebugError> { + let program_unit = debug_info.compile_unit_info(program_counter)?; + let (offset, address_size) = if let Some(line_program) = + program_unit.unit.line_program.clone() + { + ( + line_program.header().offset(), + line_program.header().address_size(), + ) + } else { + return Err(DebugError::IncompleteDebugInfo{ + message: "The specified source location does not have any line_program information available. Please consider using instruction level stepping.".to_string(), + pc_at_error: program_counter, + }); + }; + + // Get the sequences of rows from the CompleteLineProgram at the given program_counter. + let incomplete_line_program = + debug_info + .debug_line_section + .program(offset, address_size, None, None)?; + let (complete_line_program, line_sequences) = incomplete_line_program.sequences()?; + + // Get the sequence of rows that belongs to the program_counter. + let Some(line_sequence) = line_sequences.iter().find(|line_sequence| { + line_sequence.start <= program_counter && program_counter < line_sequence.end + }) else { + return Err(DebugError::IncompleteDebugInfo{ + message: "The specified source location does not have any line information available. Please consider using instruction level stepping.".to_string(), + pc_at_error: program_counter, + }); + }; + let program_language = program_unit.get_language(); + let mut sequence_rows = complete_line_program.resume_from(line_sequence); + + // We have enough information to create the InstructionSequence. + let mut instruction_sequence = InstructionSequence { + address_range: line_sequence.start..line_sequence.end, + instructions: Vec::new(), + debug_info, + program_unit, + }; + let mut prologue_completed = false; + let mut previous_row: Option<gimli::LineRow> = None; + while let Ok(Some((_, row))) = sequence_rows.next_row() { + // Don't do anything until we are at least at the prologue_end() of a function. + if row.prologue_end() { + prologue_completed = true; + } + + // For GNU C, it is known that the `DW_LNS_set_prologue_end` is not set, so we employ the same heuristic as GDB to determine when the prologue is complete. + // For other C compilers in the C99/11/17 standard, they will either set the `DW_LNS_set_prologue_end` or they will trigger this heuristic also. + // See https://gcc.gnu.org/legacy-ml/gcc-patches/2011-03/msg02106.html + if !prologue_completed + && matches!( + program_language, + gimli::DW_LANG_C99 | gimli::DW_LANG_C11 | gimli::DW_LANG_C17 + ) + { + if let Some(prev_row) = previous_row { + if row.end_sequence() + || (row.is_stmt() + && (row.file_index() == prev_row.file_index() + && (row.line() != prev_row.line() || row.line().is_none()))) + { + prologue_completed = true; + } + } + } + + if !prologue_completed { + log_row_eval(line_sequence, program_counter, row, " inside prologue>"); + previous_row = Some(*row); + continue; + } else { + log_row_eval(line_sequence, program_counter, row, " after prologue>"); + } + + // The end of the sequence is not a valid halt location, + // nor is it a valid instruction in the current sequence. + if row.end_sequence() { + // Mark the previous instruction as the last valid instruction in the sequence. + if let Some(previous_instruction) = instruction_sequence.instructions.last_mut() { + previous_instruction.is_sequence_exit = true; + } + break; + } + + if row.is_stmt() { + instruction_sequence.add(row, previous_row.as_ref()); + } + } + + if instruction_sequence.len() == 0 { + Err(DebugError::IncompleteDebugInfo{ + message: "Could not find valid instruction locations for this address. Consider using instruction level stepping.".to_string(), + pc_at_error: program_counter, + }) + } else { + tracing::trace!( + "Instruction location for pc={:#010x}\n{:?}", + program_counter, + instruction_sequence + ); + Ok(instruction_sequence) + } + } + + /// Identifying the instruction locations for a specific location (path, line, colunmn) is a bit more complex, + /// compared to the `for_address()` method, due to a few factors: + /// - We need to find the correct program instructions, which may be in any of the compilation + /// units of the current program. + /// - The debug information may not contain data for the "requested source location", e.g. + /// - DWARFv5 standard, section 6.2, allows omissions based on certain conditions. In this case, + /// we need to find the closest "relevant" source location that has valid debug information. + /// - The requested location may not be a valid source location, e.g. when the + /// debug information has been optimized away. In this case we will return an appropriate error. + #[allow(dead_code)] // temporary, while this PR is a WIP + pub(crate) fn for_source_location( + _path: &TypedPathBuf, + _line: u64, + _column: Option<u64>, + ) -> Result<Self, DebugError> { + Err(DebugError::Other(anyhow::anyhow!("TODO"))) + } + + /// Add a instruction location to the list. + pub(crate) fn add(&mut self, row: &gimli::LineRow, previous_row: Option<&gimli::LineRow>) { + let mut instruction_location = InstructionLocation::from(row); + if let Some(prev_row) = previous_row { + if row.line().is_none() + && prev_row.line().is_some() + && row.file_index() == prev_row.file_index() + && prev_row.column() == row.column() + { + // Workaround the line number issue (if recorded as 0 in the DWARF, then gimli reports it as None). + // For debug purposes, it makes more sense to be the same as the previous line, which almost always + // has the same file index and column value. + // This prevents the debugger from jumping to the top of the file unexpectedly. + instruction_location.line = prev_row.line(); + } + } + self.instructions.push(instruction_location); + } + + /// Get the number of instruction locations in the list. + pub(crate) fn len(&self) -> usize { + self.instructions.len() + } + + /// Return the first valid breakpoint location of the statement that is greater than OR equal to `address`. + /// e.g., if the `address` is the current program counter, and the return value will be the next valid halt address + /// in the current sequence. A result of `None` indicates that the next valid halt address is outside the current sequence. + pub(crate) fn get_first_breakpoint( + &self, + address: u64, + ) -> Result<VerifiedBreakpoint, DebugError> { + // Note: The `address_range` captures address range the prologue, in addition to the valid instructions in the sequence. + if self.address_range.contains(&address) { + if let Some(valid_breakpoint) = self + .instructions + .iter() + .find(|instruction_location| instruction_location.address >= address) + .and_then(|instruction_location| { + SourceLocation::from_instruction_location( + self.debug_info, + self.program_unit, + instruction_location, + ) + .map(|source_location| VerifiedBreakpoint { + address: instruction_location.address, + source_location, + }) + }) + { + tracing::debug!( + "Found valid breakpoint for address: {:#010x} : {valid_breakpoint:?}", + &address + ); + return Ok(valid_breakpoint); + } + } + Err(DebugError::IncompleteDebugInfo{ + message: "Could not determine valid halt locations for this request. Please consider using instruction level stepping.".to_string(), + pc_at_error: address, + }) + } +} + +#[derive(Clone)] +/// - A [`InstructionLocation`] filters and maps [`gimli::LineRow`] entries to be used for determining valid halt points. +/// - Each [`InstructionLocation`] maps to a single machine instruction on target. +/// - For establishing valid halt locations (breakpoint or stepping), we are only interested, +/// in the [`InstructionLocation`]'s that represent DWARF defined `statements`, +/// which are not part of the prologue or epilogue. +/// - A line of code in a source file may contain multiple instruction locations, in which case +/// a new [`InstructionLocation`] with unique `column` is created. +/// - A [`InstructionSequence`] is a series of contiguous [`InstructionLocation`]'s. +pub(crate) struct InstructionLocation { + pub(crate) address: u64, + pub(crate) file_index: u64, + pub(crate) line: Option<NonZeroU64>, + pub(crate) column: ColumnType, + /// Indicates that this instruction location is either the beginning of an epilogue, + /// or the last valid instruction in the sequence. + pub(crate) is_sequence_exit: bool, +} + +impl Debug for InstructionLocation { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Instruction @ {:010x}, on line={:04} col={:05} f={:02}, is_sequence_exit={:?}", + &self.address, + match &self.line { + Some(line) => line.get(), + None => 0, + }, + match &self.column { + ColumnType::LeftEdge => 0, + ColumnType::Column(column) => column.to_owned(), + }, + &self.file_index, + &self.is_sequence_exit, + )?; + Ok(()) + } +} + +impl From<&gimli::LineRow> for InstructionLocation { + fn from(line_row: &gimli::LineRow) -> Self { + InstructionLocation { + address: line_row.address(), + file_index: line_row.file_index(), + line: line_row.line(), + column: line_row.column().into(), + is_sequence_exit: line_row.epilogue_begin(), + } + } +} + +/// Helper function to avoid code duplication when logging of information during row evaluation. +fn log_row_eval( + active_sequence: &LineSequence<super::GimliReader>, + pc: u64, + row: &gimli::LineRow, + status: &str, +) { + tracing::trace!("Sequence: line={:04} col={:05} f={:02} addr={:#010X} stmt={:5} ep={:5} es={:5} eb={:5} : {:#010X}<={:#010X}<{:#010X} : {}", + match row.line() { + Some(line) => line.get(), + None => 0, + }, + match row.column() { + gimli::ColumnType::LeftEdge => 0, + gimli::ColumnType::Column(column) => column.get(), + }, + row.file_index(), + row.address(), + row.is_stmt(), + row.prologue_end(), + row.end_sequence(), + row.epilogue_begin(), + active_sequence.start, + pc, + active_sequence.end, + status); +}
diff --git a/probe-rs/src/debug/source_statement.rs b/probe-rs/src/debug/source_statement.rs deleted file mode 100644 index a57593d..0000000 --- a/probe-rs/src/debug/source_statement.rs +++ /dev/null
@@ -1,303 +0,0 @@ -use super::{unit_info::UnitInfo, DebugError, DebugInfo}; -use gimli::{ColumnType, LineSequence}; -use std::{ - fmt::{Debug, Formatter}, - num::NonZeroU64, - ops::Range, -}; - -/// Keep track of all the source statements required to satisfy the operations of [`SteppingMode`]. - -pub struct SourceStatements { - // NOTE: Use Vec as a container, because we will have relatively few statements per sequence, and we need to maintain the order. - pub(crate) statements: Vec<SourceStatement>, -} - -impl Debug for SourceStatements { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - for statement in &self.statements { - writeln!(f, "{statement:?}")?; - } - Ok(()) - } -} - -impl SourceStatements { - /// Extract all the source statements from the `program_unit`, starting at the `program_counter`. - /// Note:: In the interest of efficiency, for the case of SteppingMode::Breakpoint, this method will return as soon as it finds a valid halt point, and the result will only include the source statements between program_counter and the first valid haltpoint (inclusive). - pub(crate) fn new( - debug_info: &DebugInfo, - program_unit: &UnitInfo, - program_counter: u64, - ) -> Result<Self, DebugError> { - let mut source_statements = SourceStatements { - statements: Vec::new(), - }; - let (complete_line_program, active_sequence) = - get_program_info_at_pc(debug_info, program_unit, program_counter)?; - let mut sequence_rows = complete_line_program.resume_from(&active_sequence); - let program_language = program_unit.get_language(); - let mut prologue_completed = false; - let mut source_statement: Option<SourceStatement> = None; - while let Ok(Some((_, row))) = sequence_rows.next_row() { - if let Some(source_row) = source_statement.as_mut() { - if source_row.line.is_none() - && row.line().is_some() - && row.file_index() == source_row.file_index - && row.column() == source_row.column - { - // Workaround the line number issue (it is recorded as None in the DWARF when for debug purposes, it makes more sense to be the same as the previous line). - source_row.line = row.line(); - } - } else { - // Start tracking the source statement using this row. - source_statement = Some(SourceStatement::from(row)); - } - - // Don't do anything until we are at least at the prologue_end() of a function. - if row.prologue_end() { - prologue_completed = true; - } - // For GNU C, it is known that the `DW_LNS_set_prologue_end` is not set, so we employ the same heuristic as GDB to determine when the prologue is complete. - // For other C compilers in the C99/11/17 standard, they will either set the `DW_LNS_set_prologue_end` or they will trigger this heuristic also. - // See https://gcc.gnu.org/legacy-ml/gcc-patches/2011-03/msg02106.html - if !prologue_completed - && matches!( - program_language, - gimli::DW_LANG_C99 | gimli::DW_LANG_C11 | gimli::DW_LANG_C17 - ) - { - if let Some(source_row) = source_statement.as_mut() { - if row.end_sequence() - || (row.is_stmt() - && (row.file_index() == source_row.file_index - && (row.line() != source_row.line || row.line().is_none()))) - { - prologue_completed = true; - } - } - } - - if !prologue_completed { - log_row_eval(&active_sequence, program_counter, row, " inside prologue>"); - continue; - } else { - log_row_eval(&active_sequence, program_counter, row, " after prologue>"); - } - - // Notes about the process of building the source statement: - // 1. Start a new (and close off the previous) source statement, when we encounter end of sequence OR change of file/line/column. - // 2. The starting range of the first source statement will always be greater than or equal to the program_counter. - // 3. The values in the `source_statement` are only updated before we exit the current iteration of the loop, so that we can retroactively close off and store the source statement that belongs to previous `rows`. - // 4. The debug_info sometimes has a `None` value for the `row.line` that was started in the previous row, in which case we need to carry the previous row `line` number forward. GDB ignores this fact, and it shows up during debug as stepping to the top of the file (line 0) unexpectedly. - - if let Some(source_row) = source_statement.as_mut() { - // Update the instruction_range.end value. - source_row.instruction_range = source_row.low_pc()..row.address(); - - if row.end_sequence() - || (row.is_stmt() && row.address() > source_row.low_pc()) - || !(row.file_index() == source_row.file_index - && (row.line() == source_row.line || row.line().is_none()) - && row.column() == source_row.column) - { - if source_row.low_pc() >= program_counter { - // We need to close off the "current" source statement and add it to the list. - source_row.sequence_range = program_counter..active_sequence.end; - source_statements.add(source_row.clone()); - } - - if row.end_sequence() { - // If we hit the end of the sequence, we can get out of here. - break; - } - source_statement = Some(SourceStatement::from(row)); - } else if row.address() == program_counter { - // If we encounter the program_counter after the prologue, then we need to use this address as the low_pc, or else we run the risk of setting a breakpoint before the current program counter. - source_row.instruction_range = row.address()..row.address(); - } - } - } - - if source_statements.len() == 0 { - Err(DebugError::NoValidHaltLocation{ - message: "Could not find valid source statements for this address. Consider using instruction level stepping.".to_string(), - pc_at_error: program_counter, - }) - } else { - tracing::trace!( - "Source statements for pc={:#010x}\n{:?}", - program_counter, - source_statements - ); - Ok(source_statements) - } - } - - /// Add a new source statement to the list. - pub(crate) fn add(&mut self, statement: SourceStatement) { - self.statements.push(statement); - } - - /// Get the number of source statements in the list. - pub(crate) fn len(&self) -> usize { - self.statements.len() - } -} - -#[derive(Clone)] -/// Keep track of the boundaries of a source statement inside [`gimli::LineSequence`]. -/// The `file_index`, `line` and `column` fields from a [`gimli::LineRow`] are used to identify the source statement UNIQUELY in a sequence. -/// Terminology note: -/// - An `instruction` maps to a single machine instruction on target. -/// - A `row` (a [`gimli::LineRow`]) describes the role of an `instruction` in the context of a `sequence`. -/// - A `source_statement` is a range of rows where the addresses of the machine instructions are increasing, but not necessarily contiguous. -/// - A line of code in a source file may contain multiple source statements, in which case a new source statement with unique `column` is created. -/// - The [`gimli::LineRow`] entries for a source statement does not have to be contiguous where they appear in a [`gimli::LineSequence`] -/// - A `sequence`( [`gimli::LineSequence`] ) is a series of contiguous `rows`/`instructions`(may contain multiple `source_statement`'s). -pub(crate) struct SourceStatement { - /// The first address of the statement where row.is_stmt() is true. - pub(crate) is_stmt: bool, - pub(crate) file_index: u64, - pub(crate) line: Option<NonZeroU64>, - pub(crate) column: ColumnType, - /// The range of instruction addresses associated with a source statement. - /// The `address_range.start` is the address of the first instruction which is greater than or equal to the program_counter and not inside the prologue. - /// The `address_range.end` is the address of the row of the next the sequence, i.e. not part of this statement. - pub(crate) instruction_range: Range<u64>, - /// The `sequence_range.start` is the address of the program counter for which this sequence is valid, and allows us to identify target source statements where the program counter lies inside the prologue. - /// The `sequence_range.end` is the address of the first byte after the end of a sequence, and allows us to identify when stepping over a source statement would result in leaving a sequence. - pub(crate) sequence_range: Range<u64>, -} - -impl Debug for SourceStatement { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!( - f, - "\tStatement={:05} on line={:04} col={:05} f={:02}, Range: {:#010x}-{:#010x} --> Sequence Range: {:#010x}-{:#010x}", - &self.is_stmt, - match &self.line { - Some(line) => line.get(), - None => 0, - }, - match &self.column { - gimli::ColumnType::LeftEdge => 0, - gimli::ColumnType::Column(column) => column.get(), - }, - &self.file_index, - &self.instruction_range.start, - &self.instruction_range.end, - &self.sequence_range.start, - &self.sequence_range.end, - )?; - Ok(()) - } -} - -impl SourceStatement { - /// Return the first valid halt address of the statement that is greater than or equal to `address`. - pub(crate) fn get_first_halt_address(&self, address: u64) -> Option<u64> { - if self.instruction_range.start == address - || (self.sequence_range.start..self.instruction_range.end).contains(&address) - { - Some(self.low_pc()) - } else { - None - } - } - - /// Get the low_pc of this source_statement. - pub(crate) fn low_pc(&self) -> u64 { - self.instruction_range.start - } -} - -impl From<&gimli::LineRow> for SourceStatement { - fn from(line_row: &gimli::LineRow) -> Self { - SourceStatement { - is_stmt: line_row.is_stmt(), - file_index: line_row.file_index(), - line: line_row.line(), - column: line_row.column(), - instruction_range: line_row.address()..line_row.address(), - sequence_range: line_row.address()..line_row.address(), - } - } -} - -// Overriding clippy, as this is a private helper function. -#[allow(clippy::type_complexity)] -/// Resolve the relevant program row data for the given program counter. -fn get_program_info_at_pc( - debug_info: &DebugInfo, - program_unit: &UnitInfo, - program_counter: u64, -) -> Result< - ( - gimli::CompleteLineProgram< - gimli::EndianReader<gimli::LittleEndian, std::rc::Rc<[u8]>>, - usize, - >, - gimli::LineSequence<gimli::EndianReader<gimli::LittleEndian, std::rc::Rc<[u8]>>>, - ), - DebugError, -> { - let (offset, address_size) = if let Some(line_program) = program_unit.unit.line_program.clone() - { - ( - line_program.header().offset(), - line_program.header().address_size(), - ) - } else { - return Err(DebugError::NoValidHaltLocation{ - message: "The specified source location does not have any line_program information available. Please consider using instruction level stepping.".to_string(), - pc_at_error: program_counter, - }); - }; - - // Get the sequences of rows from the CompleteLineProgram at the given program_counter. - let incomplete_line_program = - debug_info - .debug_line_section - .program(offset, address_size, None, None)?; - let (complete_line_program, line_sequences) = incomplete_line_program.sequences()?; - - // Get the sequence of rows that belongs to the program_counter. - if let Some(active_sequence) = line_sequences.iter().find(|line_sequence| { - line_sequence.start <= program_counter && program_counter < line_sequence.end - }) { - Ok((complete_line_program, active_sequence.clone())) - } else { - Err(DebugError::NoValidHaltLocation{ - message: "The specified source location does not have any line information available. Please consider using instruction level stepping.".to_string(), - pc_at_error: program_counter, - }) - } -} - -/// Helper function to avoid code duplication when logging of information during row evaluation. -fn log_row_eval( - active_sequence: &LineSequence<super::GimliReader>, - pc: u64, - row: &gimli::LineRow, - status: &str, -) { - tracing::trace!("Sequence row {:#010X}<={:#010X}<{:#010X}: addr={:#010X} stmt={:5} ep={:5} es={:5} line={:04} col={:05} f={:02} : {}", - active_sequence.start, - pc, - active_sequence.end, - row.address(), - row.is_stmt(), - row.prologue_end(), - row.end_sequence(), - match row.line() { - Some(line) => line.get(), - None => 0, - }, - match row.column() { - gimli::ColumnType::LeftEdge => 0, - gimli::ColumnType::Column(column) => column.get(), - }, - row.file_index(), - status); -}
diff --git a/probe-rs/src/debug/unit_info.rs b/probe-rs/src/debug/unit_info.rs index d5b1921..c1f6072 100644 --- a/probe-rs/src/debug/unit_info.rs +++ b/probe-rs/src/debug/unit_info.rs
@@ -2225,7 +2225,7 @@ Ok(evaluation.resume_with_memory(val)?) } -trait RangeExt { +pub(crate) trait RangeExt { fn contains(self, addr: u64) -> bool; }
diff --git a/probe-rs/tests/source_location.rs b/probe-rs/tests/source_location.rs index 4b31a63..2241cf9 100644 --- a/probe-rs/tests/source_location.rs +++ b/probe-rs/tests/source_location.rs
@@ -94,8 +94,6 @@ column: Some(*col), directory: Some(dir.clone()), file: Some(file.to_owned()), - low_pc: Some(0x80006DE), - high_pc: Some(0x8000E0C), }), di.get_source_location(*addr) );