Skip to content

Commit bafd86a

Browse files
puremourningpuremourning
committed
Add full support for data breakpoints
Currently, we only provode support for data breakpoints where the request is for a child of some container using variables_reference. However, the spec includes support for: - an arbitrary expression, and - an arbitrary address, and - an optional size in bytes. The later two are supported via a capability "supportsDataBreakpointBytes". We add support for all of these options. It's fairly trivial to evaluate an expression and do esentially what we do now for children (find the load address, check its size). But we can also accept a size, so long as it's valid watchpoint size. Similarly for addresses, if 'asAddress' is supplied we can just try to use it. It will probably fail if the address is actually invalid, but that's sort of the point.
1 parent fe108b5 commit bafd86a

File tree

4 files changed

+164
-33
lines changed

4 files changed

+164
-33
lines changed

‎adapter/adapter-protocol/src/debugAdapterProtocol.json‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1386,6 +1386,18 @@
13861386
"name": {
13871387
"type": "string",
13881388
"description": "The name of the Variable's child to obtain data breakpoint information for.\nIf variablesReference isn't provided, this can be an expression."
1389+
},
1390+
"frameId": {
1391+
"type": "integer",
1392+
"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."
1393+
},
1394+
"asAddress": {
1395+
"type": "boolean",
1396+
"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."
1397+
},
1398+
"bytes": {
1399+
"type": "integer",
1400+
"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."
13891401
}
13901402
},
13911403
"required": [ "name" ]
@@ -3145,7 +3157,11 @@
31453157
"supportsSingleThreadExecutionRequests": {
31463158
"type": "boolean",
31473159
"description": "The debug adapter supports the 'singleThread' property on the execution requests ('continue', 'next', 'stepIn', 'stepOut', 'reverseContinue', 'stepBack')."
3148-
}
3160+
},
3161+
"supportsDataBreakpointBytes": {
3162+
"type": "boolean",
3163+
"description": "The debug adapter supports the `asAddress` and `bytes` fields in the `dataBreakpointInfo` request."
3164+
}
31493165
}
31503166
},
31513167

‎adapter/codelldb/src/debug_session.rs‎

Lines changed: 118 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,7 @@ impl DebugSession {
572572
support_terminate_debuggee: Some(true),
573573
supports_log_points: Some(true),
574574
supports_data_breakpoints: Some(true),
575+
supports_data_breakpoint_bytes: Some(true),
575576
supports_cancel_request: Some(true),
576577
supports_disassemble_request: Some(true),
577578
supports_stepping_granularity: Some(true),
@@ -1019,52 +1020,138 @@ impl DebugSession {
10191020
&mut self,
10201021
args: DataBreakpointInfoArguments,
10211022
) -> Result<DataBreakpointInfoResponseBody, Error> {
1022-
let container_handle = handles::from_i64(args.variables_reference.ok_or("variables_reference")?)?;
1023-
let container = self.var_refs.get(container_handle).expect("Invalid variables reference");
1024-
let child = match container {
1025-
Container::SBValue(container) => container.child_member_with_name(&args.name),
1026-
Container::Locals(frame) => frame.find_variable(&args.name),
1027-
Container::Globals(frame) => frame.find_value(&args.name, ValueType::VariableGlobal),
1028-
Container::Statics(frame) => frame.find_value(&args.name, ValueType::VariableStatic),
1029-
_ => None,
1030-
};
1031-
if let Some(child) = child {
1032-
let addr = child.load_address();
1033-
if addr != lldb::INVALID_ADDRESS {
1034-
let size = child.byte_size();
1035-
if self.is_valid_watchpoint_size(size) {
1036-
let data_id = format!("{}/{}", addr, size);
1037-
let desc = child.name().unwrap_or("");
1023+
if let Some(variables_reference) = args.variables_reference {
1024+
let container_handle = handles::from_i64(variables_reference)?;
1025+
let container = self.var_refs.get(container_handle).expect("Invalid variables reference");
1026+
let child = match container {
1027+
Container::SBValue(container) => container.child_member_with_name(&args.name),
1028+
Container::Locals(frame) => frame.find_variable(&args.name),
1029+
Container::Globals(frame) => frame.find_value(&args.name, ValueType::VariableGlobal),
1030+
Container::Statics(frame) => frame.find_value(&args.name, ValueType::VariableStatic),
1031+
_ => None,
1032+
};
1033+
if let Some(child) = child {
1034+
let addr = child.load_address();
1035+
if addr != lldb::INVALID_ADDRESS {
1036+
let size = child.byte_size();
1037+
if self.is_valid_watchpoint_size(size) {
1038+
let data_id = format!("{}/{}", addr, size);
1039+
let desc = child.name().unwrap_or("");
1040+
Ok(DataBreakpointInfoResponseBody {
1041+
data_id: Some(data_id),
1042+
access_types: Some(vec![
1043+
DataBreakpointAccessType::Read,
1044+
DataBreakpointAccessType::Write,
1045+
DataBreakpointAccessType::ReadWrite,
1046+
]),
1047+
description: format!("{} bytes at {:X} ({})", size, addr, desc),
1048+
..Default::default()
1049+
})
1050+
} else {
1051+
Ok(DataBreakpointInfoResponseBody {
1052+
data_id: None,
1053+
description: "Invalid watchpoint size.".into(),
1054+
..Default::default()
1055+
})
1056+
}
1057+
} else {
1058+
Ok(DataBreakpointInfoResponseBody {
1059+
data_id: None,
1060+
description: "This variable doesn't have an address.".into(),
1061+
..Default::default()
1062+
})
1063+
}
1064+
} else {
1065+
Ok(DataBreakpointInfoResponseBody {
1066+
data_id: None,
1067+
description: "Variable not found.".into(),
1068+
..Default::default()
1069+
})
1070+
}
1071+
} else {
1072+
let frame = match args.frame_id {
1073+
Some(frame_id) => {
1074+
let handle = handles::from_i64(frame_id)?;
1075+
match self.var_refs.get(handle) {
1076+
Some(Container::StackFrame(ref frame)) => {
1077+
// If they had used `frame select` command after the last stop,
1078+
// use currently selected frame from frame's thread, instead of the frame itself.
1079+
if self.selected_frame_changed {
1080+
Some(frame.thread().selected_frame())
1081+
} else {
1082+
Some(frame.clone())
1083+
}
1084+
}
1085+
_ => {
1086+
None
1087+
}
1088+
}
1089+
}
1090+
None => None,
1091+
};
1092+
if args.as_address.unwrap_or(false) {
1093+
// name is an address
1094+
let addr = parse_int::parse::<u64>(&args.name).unwrap_or(lldb::INVALID_ADDRESS);
1095+
let size = args.bytes.unwrap_or( self.target.address_byte_size() as i64 ) as usize;
1096+
if addr == lldb::INVALID_ADDRESS || !SBAddress::from_load_address(addr, &self.target).is_valid() {
10381097
Ok(DataBreakpointInfoResponseBody {
1039-
data_id: Some(data_id),
1098+
data_id: None,
1099+
description: format!("Invalid address {}", addr),
1100+
..Default::default()
1101+
})
1102+
} else if self.is_valid_watchpoint_size(size) {
1103+
Ok(DataBreakpointInfoResponseBody {
1104+
data_id: None,
1105+
description: format!("Invalid size {} for watchpoint", size),
1106+
..Default::default()
1107+
})
1108+
} else {
1109+
Ok(DataBreakpointInfoResponseBody {
1110+
data_id: Some(format!("{}/{}", addr, size)),
10401111
access_types: Some(vec![
10411112
DataBreakpointAccessType::Read,
10421113
DataBreakpointAccessType::Write,
10431114
DataBreakpointAccessType::ReadWrite,
10441115
]),
1045-
description: format!("{} bytes at {:X} ({})", size, addr, desc),
1116+
description: format!("{} bytes at {:X}", size, addr),
10461117
..Default::default()
10471118
})
1119+
}
1120+
} else {
1121+
// Otherwise name is an expression
1122+
let expr = &args.name;
1123+
let result = self.evaluate_user_supplied_expr(expr, frame)?;
1124+
let addr = result.load_address();
1125+
if addr != lldb::INVALID_ADDRESS {
1126+
let size = args.bytes.unwrap_or(result.byte_size() as i64) as usize;
1127+
if self.is_valid_watchpoint_size(size) {
1128+
let data_id = format!("{}/{}", addr, size);
1129+
let desc = result.name().unwrap_or(expr);
1130+
Ok(DataBreakpointInfoResponseBody {
1131+
data_id: Some(data_id),
1132+
access_types: Some(vec![
1133+
DataBreakpointAccessType::Read,
1134+
DataBreakpointAccessType::Write,
1135+
DataBreakpointAccessType::ReadWrite,
1136+
]),
1137+
description: format!("{} bytes at {:X} ({})", size, addr, desc),
1138+
..Default::default()
1139+
})
1140+
} else {
1141+
Ok(DataBreakpointInfoResponseBody {
1142+
data_id: None,
1143+
description: format!("Expression '{}' results in invalid watchpoint size: {}.", expr, size),
1144+
..Default::default()
1145+
})
1146+
}
10481147
} else {
10491148
Ok(DataBreakpointInfoResponseBody {
10501149
data_id: None,
1051-
description: "Invalid watchpoint size.".into(),
1150+
description: "This variable doesn't have an address.".into(),
10521151
..Default::default()
10531152
})
10541153
}
1055-
} else {
1056-
Ok(DataBreakpointInfoResponseBody {
1057-
data_id: None,
1058-
description: "This variable doesn't have an address.".into(),
1059-
..Default::default()
1060-
})
10611154
}
1062-
} else {
1063-
Ok(DataBreakpointInfoResponseBody {
1064-
data_id: None,
1065-
description: "Variable not found.".into(),
1066-
..Default::default()
1067-
})
10681155
}
10691156
}
10701157

‎adapter/codelldb/src/debug_session/variables.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,20 @@ impl super::DebugSession {
517517
}
518518
}
519519

520+
pub fn evaluate_user_supplied_expr(
521+
&self,
522+
expression: &str,
523+
frame: Option<SBFrame>,
524+
) -> Result<SBValue, Error> {
525+
// The format spec is ignored and dropped, because the purpose of this
526+
// fn is to just return the SBValue to get references for things like
527+
// data breakpoints
528+
let (pp_expr, _format_spec) =
529+
expressions::prepare_with_format(expression, self.default_expr_type).map_err(as_user_error)?;
530+
self.evaluate_expr_in_frame(&pp_expr, frame.as_ref())
531+
}
532+
533+
520534
// Evaluates expr in the context of frame (or in global context if frame is None)
521535
// Returns expressions.Value or SBValue on success, SBError on failure.
522536
pub fn evaluate_expr_in_frame(

‎adapter/lldb/src/sb/sbtarget.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,21 @@ impl SBTarget {
157157
let mut error = SBError::new();
158158
let wp = cpp!(unsafe [self as "SBTarget*", addr as "addr_t", size as "size_t",
159159
read as "bool", write as "bool", mut error as "SBError"] -> SBWatchpoint as "SBWatchpoint" {
160-
return self->WatchAddress(addr, size, read, write, error);
160+
161+
//
162+
// The LLDB API WatchAddress is a wrapper for
163+
// WatchpointCreateByAddress, but it's bugged and ignores what you
164+
// put in 'modify', meaning it always stops even for read-only
165+
// breakpoing requests. Fortunately, the implementation is trivial,
166+
// so we can just call WatchpointCreateByAddress directly.
167+
//
168+
SBWatchpointOptions options = {};
169+
options.SetWatchpointTypeRead(read);
170+
if (write) {
171+
options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify);
172+
}
173+
174+
return self->WatchpointCreateByAddress(addr, size, options, error);
161175
});
162176
if error.is_success() {
163177
Ok(wp)

0 commit comments

Comments
 (0)