Skip to content

Commit 3598f3d

Browse files
anthony-kolesovtkrasnukha
authored andcommitted
Report struct/union/array members in -var-update response
Fixes #25. Before this commit, when LLDB-MI would get a -var-update request for a complex variable (struct, union, array), it would analyze if value changed and if it is, then it would return a changelist for this this value only, which in case of struct and unions means `{...}`. Child member value were not reported even when changed - MI client would need to request them individually. This is different from GDB behavior, where complex object is not reported at all, but child values are reported. This was causing troubles for Eclipse Variables view which was written under assumption that -var-update for a complex variable returns changelist for all children - as a result values of struct fields were not updated when changed, view continued to show obsolete value. The fix is complicated by the fact that response should include children only if they have been listed with -var-list-children. What is more, according to GDB MI docs [1] range specified to -var-list-children shouldn't affect -var-update, instead -var-set-update-range should be used to specify range of children to be reported in -var-update response, however LLDB-MI doesn't support the -var-set-update-range request at all - so instead this commit implementation relies on -var-list-children. It is also not clear how to efficiently identify which children have been listed and which not - complex var objects don't contain links to children, instead they exist in a flat namespace, where children-parent relationship is established through a name. Therefore in this commit child will be added to the changelist if it's value changed and there is an existing var object with the same name. Probably it would be faster to iterate through known var objects and compare their names with a name of a root object, but CMICmnLLDBDebugSessionInfoVarObj doesn't provide an interface to iterate through objects and changing this class was out of scope for my fix - fix relies only on existing public functions. Additional noticable problem is recursion, if children objects contain a pointer to the parent object, however LLDB-MI doesn't has to stop it itself. A user may "open up" loop of parent/children in the infinite fashion as much as they want - LLDB-MI and Eclipse GUI would just print as many nested levels as needed. Note that CMICmdCmdVarUpdate::ExamineSBValueForChange explicitly halts on pointers and references, because it scans through SBValue children and doesn't know which children will be printed or not - structure with unlisted children still will be reported as one that changed its value. The one case where this code still doesn't work, if we had construct like struct S { int i; } struct S s = { 1 }; struct S *ps = NULL; Then a variable will be created for ps->i, and because ps is NULL, that variable will have a printed value of `??`. If later application does ps = &s; Then ps->i will effectively change its value, but this will not be reported in the changelist, because SBValue::GetValueDidChange() returns false for this value. Apparantly change from "invalid" value to a valid one is not considered a "value change" in API, but this disagreese how this should be represented in the Eclipse GUI. The only way to fix this in MI, I think is to compare "old" and "new" string value in MI, which means need to store old string value - and that is not very effective, so I don't want to try to fix this problem in this commit. As far as I observe this is specific to cases with NULL pointer, if pointer was pointing to a valid structure, than started to point to another - changes in children values will be properly reported by SBValue, and this will make their way into the changelist. [1] https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Variable-Objects.html#The-_002dvar_002dlist_002dchildren-Command
1 parent 626900d commit 3598f3d

File tree

2 files changed

+81
-10
lines changed

2 files changed

+81
-10
lines changed

src/MICmdCmdVar.cpp

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,78 @@ bool CMICmdCmdVarUpdate::ParseArgs() {
356356
return ParseValidateCmdOptions();
357357
}
358358

359+
//++
360+
// Details: Print an SBValue or its children into the changelist response. This
361+
// is a recursive function. Note that user code may contain infinite
362+
// recursion, if a structure contains a pointer to itself (directly or
363+
// indirectly) - but this code can't really break this recursion,
364+
// because user still may open up elements in variables view into many
365+
// levels of nesting. In practice recursion is stopped by the user who
366+
// at some point stops expanding nested elements, and this would mean
367+
// that at some point complex variable will not have its children
368+
// listed with -var-list-children and that will stop recursion.
369+
// Return: MIstatus::success - Function succeeded.
370+
// MIstatus::failure - Function failed.
371+
//--
372+
bool CMICmdCmdVarUpdate::PrintValue(
373+
CMICmnLLDBDebugSessionInfo::VariableInfoFormat_e eVarInfoFormat,
374+
lldb::SBValue &value, const CMIUtilString &valueName) {
375+
376+
bool bPrintedChildren = false;
377+
if (value.MightHaveChildren()) {
378+
// Scan children recursively and print those that changed.
379+
const MIuint nChildren = value.GetNumChildren();
380+
for (MIuint i = 0; i < nChildren; i++) {
381+
lldb::SBValue child = value.GetChildAtIndex(i);
382+
383+
// Did this child changed its value?
384+
bool childValueChanged = false;
385+
if (!ExamineSBValueForChange(child, childValueChanged))
386+
return MIstatus::failure;
387+
if (!childValueChanged)
388+
continue;
389+
390+
// rValue.GetNumChildren() returns all members, but this function should
391+
// print only those that has been listed by -var-list-children or
392+
// -var-set-update-range. LLDB-MI doens't support the latter, though.
393+
// I don't know any better way to check if member has been listed than
394+
// to attempt to get the member by its name.
395+
const CMIUtilString childName(
396+
GetMemberName(valueName, CMICmnLLDBUtilSBValue(child).GetName(), i));
397+
CMICmnLLDBDebugSessionInfoVarObj childVarObj;
398+
if (!CMICmnLLDBDebugSessionInfoVarObj::VarObjGet(childName, childVarObj))
399+
continue;
400+
401+
PrintValue(eVarInfoFormat, child, childName);
402+
bPrintedChildren = true;
403+
}
404+
}
405+
406+
if (!bPrintedChildren ||
407+
(value.GetType().GetTypeFlags() & lldb::eTypeIsPointer &&
408+
value.GetValueDidChange())) {
409+
// Print scalar values or complex value if its children were not printed.
410+
// Pointer to a structure is also printed if its own value changed
411+
// (structure is printed as `{...}`, while pointer to a structure is printed
412+
// as an address value).
413+
const bool bPrintValue =
414+
((eVarInfoFormat ==
415+
CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_AllValues) ||
416+
(eVarInfoFormat ==
417+
CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_SimpleValues &&
418+
value.GetNumChildren() == 0));
419+
const CMIUtilString strValue(
420+
CMICmnLLDBDebugSessionInfoVarObj::GetValueStringFormatted(
421+
value, CMICmnLLDBDebugSessionInfoVarObj::eVarFormat_Natural));
422+
const CMIUtilString strInScope(value.IsInScope() ? "true" : "false");
423+
424+
MIFormResponse(valueName, bPrintValue ? strValue.c_str() : nullptr,
425+
strInScope);
426+
}
427+
428+
return MIstatus::success;
429+
}
430+
359431
//++
360432
// Details: The invoker requires this function. The command does work in this
361433
// function.
@@ -417,16 +489,7 @@ bool CMICmdCmdVarUpdate::Execute() {
417489

418490
if (m_bValueChanged) {
419491
varObj.UpdateValue();
420-
const bool bPrintValue(
421-
(eVarInfoFormat ==
422-
CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_AllValues) ||
423-
(eVarInfoFormat ==
424-
CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_SimpleValues &&
425-
rValue.GetNumChildren() == 0));
426-
const CMIUtilString strValue(bPrintValue ? varObj.GetValueFormatted() : "");
427-
const CMIUtilString strInScope(rValue.IsInScope() ? "true" : "false");
428-
MIFormResponse(rVarObjName, bPrintValue ? strValue.c_str() : nullptr,
429-
strInScope);
492+
return PrintValue(eVarInfoFormat, rValue, rVarObjName);
430493
}
431494

432495
return MIstatus::success;
@@ -524,6 +587,11 @@ void CMICmdCmdVarUpdate::MIFormResponse(const CMIUtilString &vrStrVarName,
524587
//--
525588
bool CMICmdCmdVarUpdate::ExamineSBValueForChange(lldb::SBValue &vrwValue,
526589
bool &vrwbChanged) {
590+
// Note SBValue::GetValueDidChange() returns false if value changes from
591+
// invalid (for example if it represents a field of a structure, and
592+
// structure is pointed at with a NULL pointer) to a valid value, which is not
593+
// a desired result for -var-update changelist - it will miss case of
594+
// invalid-to-valid change.
527595
if (vrwValue.GetValueDidChange()) {
528596
vrwbChanged = true;
529597
return MIstatus::success;

src/MICmdCmdVar.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ class CMICmdCmdVarUpdate : public CMICmdBase {
115115
// Methods:
116116
private:
117117
bool ExamineSBValueForChange(lldb::SBValue &vrwValue, bool &vrwbChanged);
118+
bool
119+
PrintValue(CMICmnLLDBDebugSessionInfo::VariableInfoFormat_e eVarInfoFormat,
120+
lldb::SBValue &value, const CMIUtilString &valueName);
118121
void MIFormResponse(const CMIUtilString &vrStrVarName,
119122
const char *const vpValue,
120123
const CMIUtilString &vrStrScope);

0 commit comments

Comments
 (0)