diff --git a/adapter/adapter-protocol/src/debugAdapterProtocol.json b/adapter/adapter-protocol/src/debugAdapterProtocol.json index 90a36a04..e7c7aa09 100644 --- a/adapter/adapter-protocol/src/debugAdapterProtocol.json +++ b/adapter/adapter-protocol/src/debugAdapterProtocol.json @@ -1386,6 +1386,18 @@ "name": { "type": "string", "description": "The name of the Variable's child to obtain data breakpoint information for.\nIf variablesReference isn't provided, this can be an expression." + }, + "frameId": { + "type": "integer", + "description": "When `name` is an expression, evaluate it in the scope of this stack frame. If not specified, the expression is evaluated in the global scope. When `variablesReference` is specified, this property has no effect." + }, + "asAddress": { + "type": "boolean", + "description": "If `true`, the `name` is a memory address and the debugger should interpret it as a decimal value, or hex value if it is prefixed with `0x`. Clients may set this property only if the `supportsDataBreakpointBytes` capability is true." + }, + "bytes": { + "type": "integer", + "description": "If specified, a debug adapter should return information for the range of memory extending `bytes` number of bytes from the address or variable specified by `name`. Breakpoints set using the resulting data ID should pause on data access anywhere within that range. Clients may set this property only if the `supportsDataBreakpointBytes` capability is true." } }, "required": [ "name" ] @@ -3145,7 +3157,11 @@ "supportsSingleThreadExecutionRequests": { "type": "boolean", "description": "The debug adapter supports the 'singleThread' property on the execution requests ('continue', 'next', 'stepIn', 'stepOut', 'reverseContinue', 'stepBack')." - } + }, + "supportsDataBreakpointBytes": { + "type": "boolean", + "description": "The debug adapter supports the `asAddress` and `bytes` fields in the `dataBreakpointInfo` request." + } } }, diff --git a/adapter/codelldb/src/debug_session.rs b/adapter/codelldb/src/debug_session.rs index db8df08c..f21b99be 100644 --- a/adapter/codelldb/src/debug_session.rs +++ b/adapter/codelldb/src/debug_session.rs @@ -589,6 +589,7 @@ impl DebugSession { supports_conditional_breakpoints: Some(true), supports_configuration_done_request: Some(true), supports_data_breakpoints: Some(true), + supports_data_breakpoint_bytes: Some(true), supports_delayed_stack_trace_loading: Some(true), supports_disassemble_request: Some(true), supports_evaluate_for_hovers: Some(self.evaluate_for_hovers), @@ -1040,20 +1041,20 @@ impl DebugSession { &mut self, args: DataBreakpointInfoArguments, ) -> Result { - let container_handle = handles::from_i64(args.variables_reference.ok_or("variables_reference")?)?; - let container = self.var_refs.get(container_handle).expect("Invalid variables reference"); - let child = match container { - Container::SBValue(container) => container.child_member_with_name(&args.name), - Container::Locals(frame) => frame.find_variable(&args.name), - Container::Globals(frame) => frame.find_value(&args.name, ValueType::VariableGlobal), - Container::Statics(frame) => frame.find_value(&args.name, ValueType::VariableStatic), - _ => None, - }; - if let Some(child) = child { - let addr = child.load_address(); - if addr != lldb::INVALID_ADDRESS { - let size = child.byte_size(); - if self.is_valid_watchpoint_size(size) { + if let Some(variables_reference) = args.variables_reference { + let container_handle = handles::from_i64(variables_reference)?; + let container = self.var_refs.get(container_handle).expect("Invalid variables reference"); + let child = match container { + Container::SBValue(container) => container.child_member_with_name(&args.name), + Container::Locals(frame) => frame.find_variable(&args.name), + Container::Globals(frame) => frame.find_value(&args.name, ValueType::VariableGlobal), + Container::Statics(frame) => frame.find_value(&args.name, ValueType::VariableStatic), + _ => None, + }; + if let Some(child) = child { + let addr = child.load_address(); + if addr != lldb::INVALID_ADDRESS { + let size = args.bytes.unwrap_or(child.byte_size() as i64) as usize; let data_id = format!("{}/{}", addr, size); let desc = child.name().unwrap_or(""); Ok(DataBreakpointInfoResponseBody { @@ -1069,38 +1070,87 @@ impl DebugSession { } else { Ok(DataBreakpointInfoResponseBody { data_id: None, - description: "Invalid watchpoint size.".into(), + description: "This variable doesn't have an address.".into(), ..Default::default() }) } } else { Ok(DataBreakpointInfoResponseBody { data_id: None, - description: "This variable doesn't have an address.".into(), + description: "Variable not found.".into(), ..Default::default() }) } } else { - Ok(DataBreakpointInfoResponseBody { - data_id: None, - description: "Variable not found.".into(), - ..Default::default() - }) - } - } - - fn is_valid_watchpoint_size(&self, size: usize) -> bool { - let addr_size = self.target.address_byte_size(); - match addr_size { - 4 => match size { - 1 | 2 | 4 => true, - _ => false, - }, - 8 => match size { - 1 | 2 | 4 | 8 => true, - _ => false, - }, - _ => true, // No harm in allowing to set an invalid watchpoint, other than user confusion. + let frame = match args.frame_id { + Some(frame_id) => { + let handle = handles::from_i64(frame_id)?; + match self.var_refs.get(handle) { + Some(Container::StackFrame(ref frame)) => { + // If they had used `frame select` command after the last stop, + // use currently selected frame from frame's thread, instead of the frame itself. + if self.selected_frame_changed { + Some(frame.thread().selected_frame()) + } else { + Some(frame.clone()) + } + } + _ => { + None + } + } + } + None => None, + }; + if args.as_address.unwrap_or(false) { + // name is an address + let addr = parse_int::parse::(&args.name).unwrap_or(lldb::INVALID_ADDRESS); + let size = args.bytes.unwrap_or(self.target.address_byte_size() as i64) as usize; + if addr == lldb::INVALID_ADDRESS || !SBAddress::from_load_address(addr, &self.target).is_valid() { + Ok(DataBreakpointInfoResponseBody { + data_id: None, + description: format!("Invalid address {}", addr), + ..Default::default() + }) + } else { + Ok(DataBreakpointInfoResponseBody { + data_id: Some(format!("{}/{}", addr, size)), + access_types: Some(vec![ + DataBreakpointAccessType::Read, + DataBreakpointAccessType::Write, + DataBreakpointAccessType::ReadWrite, + ]), + description: format!("{} bytes at {:X}", size, addr), + ..Default::default() + }) + } + } else { + // Otherwise name is an expression + let expr = &args.name; + let result = self.evaluate_user_supplied_expr(expr, frame)?; + let addr = result.load_address(); + if addr != lldb::INVALID_ADDRESS { + let size = args.bytes.unwrap_or(result.byte_size() as i64) as usize; + let data_id = format!("{}/{}", addr, size); + let desc = result.name().unwrap_or(expr); + Ok(DataBreakpointInfoResponseBody { + data_id: Some(data_id), + access_types: Some(vec![ + DataBreakpointAccessType::Read, + DataBreakpointAccessType::Write, + DataBreakpointAccessType::ReadWrite, + ]), + description: format!("{} bytes at {:X} ({})", size, addr, desc), + ..Default::default() + }) + } else { + Ok(DataBreakpointInfoResponseBody { + data_id: None, + description: "This variable doesn't have an address.".into(), + ..Default::default() + }) + } + } } } diff --git a/adapter/codelldb/src/debug_session/variables.rs b/adapter/codelldb/src/debug_session/variables.rs index 935ae87f..e39ff85b 100644 --- a/adapter/codelldb/src/debug_session/variables.rs +++ b/adapter/codelldb/src/debug_session/variables.rs @@ -533,6 +533,20 @@ impl super::DebugSession { } } + pub fn evaluate_user_supplied_expr( + &self, + expression: &str, + frame: Option, + ) -> Result { + // The format spec is ignored and dropped, because the purpose of this + // fn is to just return the SBValue to get references for things like + // data breakpoints + let (pp_expr, _format_spec) = + expressions::prepare_with_format(expression, self.default_expr_type).map_err(blame_user)?; + self.evaluate_expr_in_frame(&pp_expr, frame.as_ref()) + } + + // Evaluates expr in the context of frame (or in global context if frame is None) // Returns expressions.Value or SBValue on success, SBError on failure. pub(super) fn evaluate_expr_in_frame( diff --git a/adapter/lldb/src/sb/sbtarget.rs b/adapter/lldb/src/sb/sbtarget.rs index 4e3642e6..ac37a50d 100644 --- a/adapter/lldb/src/sb/sbtarget.rs +++ b/adapter/lldb/src/sb/sbtarget.rs @@ -164,7 +164,21 @@ impl SBTarget { let mut error = SBError::new(); let wp = cpp!(unsafe [self as "SBTarget*", addr as "addr_t", size as "size_t", read as "bool", write as "bool", mut error as "SBError"] -> SBWatchpoint as "SBWatchpoint" { - return self->WatchAddress(addr, size, read, write, error); + + // + // The LLDB API WatchAddress is a wrapper for + // WatchpointCreateByAddress, but it's bugged and ignores what you + // put in 'modify', meaning it always stops even for read-only + // breakpoing requests. Fortunately, the implementation is trivial, + // so we can just call WatchpointCreateByAddress directly. + // + SBWatchpointOptions options = {}; + options.SetWatchpointTypeRead(read); + if (write) { + options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify); + } + + return self->WatchpointCreateByAddress(addr, size, options, error); }); if error.is_success() { Ok(wp)