Skip to content

Commit fd4762b

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 c06be2e commit fd4762b

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
@@ -589,6 +589,7 @@ impl DebugSession {
589589
supports_conditional_breakpoints: Some(true),
590590
supports_configuration_done_request: Some(true),
591591
supports_data_breakpoints: Some(true),
592+
supports_data_breakpoint_bytes: Some(true),
592593
supports_delayed_stack_trace_loading: Some(true),
593594
supports_disassemble_request: Some(true),
594595
supports_evaluate_for_hovers: Some(self.evaluate_for_hovers),
@@ -1040,52 +1041,138 @@ impl DebugSession {
10401041
&mut self,
10411042
args: DataBreakpointInfoArguments,
10421043
) -> Result<DataBreakpointInfoResponseBody, Error> {
1043-
let container_handle = handles::from_i64(args.variables_reference.ok_or("variables_reference")?)?;
1044-
let container = self.var_refs.get(container_handle).expect("Invalid variables reference");
1045-
let child = match container {
1046-
Container::SBValue(container) => container.child_member_with_name(&args.name),
1047-
Container::Locals(frame) => frame.find_variable(&args.name),
1048-
Container::Globals(frame) => frame.find_value(&args.name, ValueType::VariableGlobal),
1049-
Container::Statics(frame) => frame.find_value(&args.name, ValueType::VariableStatic),
1050-
_ => None,
1051-
};
1052-
if let Some(child) = child {
1053-
let addr = child.load_address();
1054-
if addr != lldb::INVALID_ADDRESS {
1055-
let size = child.byte_size();
1056-
if self.is_valid_watchpoint_size(size) {
1057-
let data_id = format!("{}/{}", addr, size);
1058-
let desc = child.name().unwrap_or("");
1044+
if let Some(variables_reference) = args.variables_reference {
1045+
let container_handle = handles::from_i64(variables_reference)?;
1046+
let container = self.var_refs.get(container_handle).expect("Invalid variables reference");
1047+
let child = match container {
1048+
Container::SBValue(container) => container.child_member_with_name(&args.name),
1049+
Container::Locals(frame) => frame.find_variable(&args.name),
1050+
Container::Globals(frame) => frame.find_value(&args.name, ValueType::VariableGlobal),
1051+
Container::Statics(frame) => frame.find_value(&args.name, ValueType::VariableStatic),
1052+
_ => None,
1053+
};
1054+
if let Some(child) = child {
1055+
let addr = child.load_address();
1056+
if addr != lldb::INVALID_ADDRESS {
1057+
let size = child.byte_size();
1058+
if self.is_valid_watchpoint_size(size) {
1059+
let data_id = format!("{}/{}", addr, size);
1060+
let desc = child.name().unwrap_or("");
1061+
Ok(DataBreakpointInfoResponseBody {
1062+
data_id: Some(data_id),
1063+
access_types: Some(vec![
1064+
DataBreakpointAccessType::Read,
1065+
DataBreakpointAccessType::Write,
1066+
DataBreakpointAccessType::ReadWrite,
1067+
]),
1068+
description: format!("{} bytes at {:X} ({})", size, addr, desc),
1069+
..Default::default()
1070+
})
1071+
} else {
1072+
Ok(DataBreakpointInfoResponseBody {
1073+
data_id: None,
1074+
description: "Invalid watchpoint size.".into(),
1075+
..Default::default()
1076+
})
1077+
}
1078+
} else {
10591079
Ok(DataBreakpointInfoResponseBody {
1060-
data_id: Some(data_id),
1080+
data_id: None,
1081+
description: "This variable doesn't have an address.".into(),
1082+
..Default::default()
1083+
})
1084+
}
1085+
} else {
1086+
Ok(DataBreakpointInfoResponseBody {
1087+
data_id: None,
1088+
description: "Variable not found.".into(),
1089+
..Default::default()
1090+
})
1091+
}
1092+
} else {
1093+
let frame = match args.frame_id {
1094+
Some(frame_id) => {
1095+
let handle = handles::from_i64(frame_id)?;
1096+
match self.var_refs.get(handle) {
1097+
Some(Container::StackFrame(ref frame)) => {
1098+
// If they had used `frame select` command after the last stop,
1099+
// use currently selected frame from frame's thread, instead of the frame itself.
1100+
if self.selected_frame_changed {
1101+
Some(frame.thread().selected_frame())
1102+
} else {
1103+
Some(frame.clone())
1104+
}
1105+
}
1106+
_ => {
1107+
None
1108+
}
1109+
}
1110+
}
1111+
None => None,
1112+
};
1113+
if args.as_address.unwrap_or(false) {
1114+
// name is an address
1115+
let addr = parse_int::parse::<u64>(&args.name).unwrap_or(lldb::INVALID_ADDRESS);
1116+
let size = args.bytes.unwrap_or(self.target.address_byte_size() as i64) as usize;
1117+
if addr == lldb::INVALID_ADDRESS || !SBAddress::from_load_address(addr, &self.target).is_valid() {
1118+
Ok(DataBreakpointInfoResponseBody {
1119+
data_id: None,
1120+
description: format!("Invalid address {}", addr),
1121+
..Default::default()
1122+
})
1123+
} else if self.is_valid_watchpoint_size(size) {
1124+
Ok(DataBreakpointInfoResponseBody {
1125+
data_id: None,
1126+
description: format!("Invalid size {} for watchpoint", size),
1127+
..Default::default()
1128+
})
1129+
} else {
1130+
Ok(DataBreakpointInfoResponseBody {
1131+
data_id: Some(format!("{}/{}", addr, size)),
10611132
access_types: Some(vec![
10621133
DataBreakpointAccessType::Read,
10631134
DataBreakpointAccessType::Write,
10641135
DataBreakpointAccessType::ReadWrite,
10651136
]),
1066-
description: format!("{} bytes at {:X} ({})", size, addr, desc),
1137+
description: format!("{} bytes at {:X}", size, addr),
10671138
..Default::default()
10681139
})
1140+
}
1141+
} else {
1142+
// Otherwise name is an expression
1143+
let expr = &args.name;
1144+
let result = self.evaluate_user_supplied_expr(expr, frame)?;
1145+
let addr = result.load_address();
1146+
if addr != lldb::INVALID_ADDRESS {
1147+
let size = args.bytes.unwrap_or(result.byte_size() as i64) as usize;
1148+
if self.is_valid_watchpoint_size(size) {
1149+
let data_id = format!("{}/{}", addr, size);
1150+
let desc = result.name().unwrap_or(expr);
1151+
Ok(DataBreakpointInfoResponseBody {
1152+
data_id: Some(data_id),
1153+
access_types: Some(vec![
1154+
DataBreakpointAccessType::Read,
1155+
DataBreakpointAccessType::Write,
1156+
DataBreakpointAccessType::ReadWrite,
1157+
]),
1158+
description: format!("{} bytes at {:X} ({})", size, addr, desc),
1159+
..Default::default()
1160+
})
1161+
} else {
1162+
Ok(DataBreakpointInfoResponseBody {
1163+
data_id: None,
1164+
description: format!("Expression '{}' results in invalid watchpoint size: {}.", expr, size),
1165+
..Default::default()
1166+
})
1167+
}
10691168
} else {
10701169
Ok(DataBreakpointInfoResponseBody {
10711170
data_id: None,
1072-
description: "Invalid watchpoint size.".into(),
1171+
description: "This variable doesn't have an address.".into(),
10731172
..Default::default()
10741173
})
10751174
}
1076-
} else {
1077-
Ok(DataBreakpointInfoResponseBody {
1078-
data_id: None,
1079-
description: "This variable doesn't have an address.".into(),
1080-
..Default::default()
1081-
})
10821175
}
1083-
} else {
1084-
Ok(DataBreakpointInfoResponseBody {
1085-
data_id: None,
1086-
description: "Variable not found.".into(),
1087-
..Default::default()
1088-
})
10891176
}
10901177
}
10911178

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,20 @@ impl super::DebugSession {
533533
}
534534
}
535535

536+
pub fn evaluate_user_supplied_expr(
537+
&self,
538+
expression: &str,
539+
frame: Option<SBFrame>,
540+
) -> Result<SBValue, Error> {
541+
// The format spec is ignored and dropped, because the purpose of this
542+
// fn is to just return the SBValue to get references for things like
543+
// data breakpoints
544+
let (pp_expr, _format_spec) =
545+
expressions::prepare_with_format(expression, self.default_expr_type).map_err(blame_user)?;
546+
self.evaluate_expr_in_frame(&pp_expr, frame.as_ref())
547+
}
548+
549+
536550
// Evaluates expr in the context of frame (or in global context if frame is None)
537551
// Returns expressions.Value or SBValue on success, SBError on failure.
538552
pub(super) 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
@@ -164,7 +164,21 @@ impl SBTarget {
164164
let mut error = SBError::new();
165165
let wp = cpp!(unsafe [self as "SBTarget*", addr as "addr_t", size as "size_t",
166166
read as "bool", write as "bool", mut error as "SBError"] -> SBWatchpoint as "SBWatchpoint" {
167-
return self->WatchAddress(addr, size, read, write, error);
167+
168+
//
169+
// The LLDB API WatchAddress is a wrapper for
170+
// WatchpointCreateByAddress, but it's bugged and ignores what you
171+
// put in 'modify', meaning it always stops even for read-only
172+
// breakpoing requests. Fortunately, the implementation is trivial,
173+
// so we can just call WatchpointCreateByAddress directly.
174+
//
175+
SBWatchpointOptions options = {};
176+
options.SetWatchpointTypeRead(read);
177+
if (write) {
178+
options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify);
179+
}
180+
181+
return self->WatchpointCreateByAddress(addr, size, options, error);
168182
});
169183
if error.is_success() {
170184
Ok(wp)

0 commit comments

Comments
 (0)