Re: [lldb-dev] hang bug in lldb-mi -var-update
> On Aug 25, 2017, at 4:15 PM, Jim Ingham wrote:
>
> If that’s the expectation, you have to obey it... Xcode is pretty careful
> to only act on the elements that were visible in a view, which made the
> locals view much less heavy-weight. But that took some work on their end.
>
> More to the point, this doesn’t seem to be code that should be in the MI
> layer. Shouldn’t this be a method on the SBValue? If there was something
> tricky that you could get wrong, it would be better to centralize it.
>
> As Greg says, this shouldn’t be the default “has changed” behavior, but still
> it seems like it should be an SBValue method.
We could possibly add:
bool SBValue::GetValueDidChange(bool check_children);
as a new API. I still say this is a potentially very expensive operation that
most if not all debuggers should avoid, but if they want to debug really really
slowly under some circumstances, I don't really mind. There should be a strong
warning on this method that things can get expensive.
Your fix of not recursing through pointers needs to also do the same for any
references (l value, r value, and C++ 11 &&). So your fix is correct if you
wish to keep the lldb-mi paradigm of checking all children, just add references
to the list of things to not check.
Just for fun, make an std::vector with a million entries, change the last
one, and single step over it and wait to see how long this code takes just to
make sure you really want to check that many items.
Greg
>
> Jim
>
>> On Aug 25, 2017, at 3:49 PM, Ted Woodward via lldb-dev
>> wrote:
>>
>> The spec says that's what it should do. From
>> https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Variable-Objects.html :
>>
>> "Reevaluate the expressions corresponding to the variable object name and
>> all its direct and indirect children, and return the list of variable
>> objects whose values have changed;"
>>
>> Also, our Eclipse guy gets grumpy when it doesn't :-)
>>
>> Ted
>>
>> --
>> Qualcomm Innovation Center, Inc.
>> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>> Linux Foundation Collaborative Project
>>
>>> -Original Message-----
>>> From: Greg Clayton [mailto:clayb...@gmail.com]
>>> Sent: Friday, August 25, 2017 5:00 PM
>>> To: Ted Woodward
>>> Cc: lldb-dev@lists.llvm.org
>>> Subject: Re: [lldb-dev] hang bug in lldb-mi -var-update
>>>
>>> lldb-mi should never be checking the children. This is never a good idea
>> due to
>>> performance. What happens when you have an array with a million entries?
>>> Long delay. Aggregate types should never say they changed. Only SBValue
>>> objects that have values should claim to change.
>>>
>>> Greg
>>>
>>>
>>>> On Aug 25, 2017, at 10:42 AM, Ted Woodward via lldb-dev >> d...@lists.llvm.org> wrote:
>>>>
>>>> I found a hang in lldb-mi's -var-update. It checks to see if a var
>>>> changed, then it checks each of the children recursively. If a child
>>>> is a pointer back to a parent, as in this case:
>>>>
>>>> struct complex_type
>>>> {
>>>> int i;
>>>> struct { long l; } inner;
>>>> struct complex_type *complex_ptr;
>>>> };
>>>>
>>>> void
>>>> var_update_test(void)
>>>> {
>>>> struct complex_type complx_array[2];
>>>>
>>>> complx_array[0].i = 4;
>>>> complx_array[0].inner.l = 4;
>>>> complx_array[0].complex_ptr = &complx_array[1];
>>>> complx_array[1].i = 5;
>>>> complx_array[1].inner.l = 5;
>>>> complx_array[1].complex_ptr = &complx_array[0];
>>>>
>>>>
>>>>
>>>> the code in CMICmdCmdVarUpdate::ExamineSBValueForChange will get into
>>>> an infinite loop.
>>>>
>>>> const MIuint nChildren = vrwValue.GetNumChildren(); for (MIuint i =
>>>> 0; i < nChildren; ++i) {
>>>> lldb::SBValue member = vrwValue.GetChildAtIndex(i);
>>>> if (!member.IsValid())
>>>>continue;
>>>>
>>>> if (member.GetValueDidChange()) {
>>>>vrwbChanged = true;
>>>>return MIstatus::success;
>>>> } else if (ExamineSBValueForChange(member, vrwbChanged) &&
>>> vrwbChanged)
>>>>// Handle composite types (i.e. struct or arrays)
>>>>return MIstatus::success;
>>>> }
>>>>
>>>> I've got a patch that disables checking a pointer's children. I'll put
>>>> it up on phabricator today.
>>>>
>>>> Ted
>>>>
>>>> --
>>>> Qualcomm Innovation Center, Inc.
>>>> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>>> a Linux Foundation Collaborative Project
>>>>
>>>>
>>>> ___
>>>> lldb-dev mailing list
>>>> lldb-dev@lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>
>>
>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] hang bug in lldb-mi -var-update
If that’s the expectation, you have to obey it... Xcode is pretty careful to
only act on the elements that were visible in a view, which made the locals
view much less heavy-weight. But that took some work on their end.
More to the point, this doesn’t seem to be code that should be in the MI layer.
Shouldn’t this be a method on the SBValue? If there was something tricky that
you could get wrong, it would be better to centralize it.
As Greg says, this shouldn’t be the default “has changed” behavior, but still
it seems like it should be an SBValue method.
Jim
> On Aug 25, 2017, at 3:49 PM, Ted Woodward via lldb-dev
> wrote:
>
> The spec says that's what it should do. From
> https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Variable-Objects.html :
>
> "Reevaluate the expressions corresponding to the variable object name and
> all its direct and indirect children, and return the list of variable
> objects whose values have changed;"
>
> Also, our Eclipse guy gets grumpy when it doesn't :-)
>
> Ted
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>> -Original Message-
>> From: Greg Clayton [mailto:clayb...@gmail.com]
>> Sent: Friday, August 25, 2017 5:00 PM
>> To: Ted Woodward
>> Cc: lldb-dev@lists.llvm.org
>> Subject: Re: [lldb-dev] hang bug in lldb-mi -var-update
>>
>> lldb-mi should never be checking the children. This is never a good idea
> due to
>> performance. What happens when you have an array with a million entries?
>> Long delay. Aggregate types should never say they changed. Only SBValue
>> objects that have values should claim to change.
>>
>> Greg
>>
>>
>>> On Aug 25, 2017, at 10:42 AM, Ted Woodward via lldb-dev > d...@lists.llvm.org> wrote:
>>>
>>> I found a hang in lldb-mi's -var-update. It checks to see if a var
>>> changed, then it checks each of the children recursively. If a child
>>> is a pointer back to a parent, as in this case:
>>>
>>> struct complex_type
>>> {
>>> int i;
>>> struct { long l; } inner;
>>> struct complex_type *complex_ptr;
>>> };
>>>
>>> void
>>> var_update_test(void)
>>> {
>>> struct complex_type complx_array[2];
>>>
>>> complx_array[0].i = 4;
>>> complx_array[0].inner.l = 4;
>>> complx_array[0].complex_ptr = &complx_array[1];
>>> complx_array[1].i = 5;
>>> complx_array[1].inner.l = 5;
>>> complx_array[1].complex_ptr = &complx_array[0];
>>>
>>>
>>>
>>> the code in CMICmdCmdVarUpdate::ExamineSBValueForChange will get into
>>> an infinite loop.
>>>
>>> const MIuint nChildren = vrwValue.GetNumChildren(); for (MIuint i =
>>> 0; i < nChildren; ++i) {
>>> lldb::SBValue member = vrwValue.GetChildAtIndex(i);
>>> if (!member.IsValid())
>>> continue;
>>>
>>> if (member.GetValueDidChange()) {
>>> vrwbChanged = true;
>>> return MIstatus::success;
>>> } else if (ExamineSBValueForChange(member, vrwbChanged) &&
>> vrwbChanged)
>>> // Handle composite types (i.e. struct or arrays)
>>> return MIstatus::success;
>>> }
>>>
>>> I've got a patch that disables checking a pointer's children. I'll put
>>> it up on phabricator today.
>>>
>>> Ted
>>>
>>> --
>>> Qualcomm Innovation Center, Inc.
>>> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>> a Linux Foundation Collaborative Project
>>>
>>>
>>> ___
>>> lldb-dev mailing list
>>> lldb-dev@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] hang bug in lldb-mi -var-update
The spec says that's what it should do. From
https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Variable-Objects.html :
"Reevaluate the expressions corresponding to the variable object name and
all its direct and indirect children, and return the list of variable
objects whose values have changed;"
Also, our Eclipse guy gets grumpy when it doesn't :-)
Ted
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
> -Original Message-
> From: Greg Clayton [mailto:clayb...@gmail.com]
> Sent: Friday, August 25, 2017 5:00 PM
> To: Ted Woodward
> Cc: lldb-dev@lists.llvm.org
> Subject: Re: [lldb-dev] hang bug in lldb-mi -var-update
>
> lldb-mi should never be checking the children. This is never a good idea
due to
> performance. What happens when you have an array with a million entries?
> Long delay. Aggregate types should never say they changed. Only SBValue
> objects that have values should claim to change.
>
> Greg
>
>
> > On Aug 25, 2017, at 10:42 AM, Ted Woodward via lldb-dev d...@lists.llvm.org> wrote:
> >
> > I found a hang in lldb-mi's -var-update. It checks to see if a var
> > changed, then it checks each of the children recursively. If a child
> > is a pointer back to a parent, as in this case:
> >
> > struct complex_type
> > {
> >int i;
> >struct { long l; } inner;
> >struct complex_type *complex_ptr;
> > };
> >
> > void
> > var_update_test(void)
> > {
> >struct complex_type complx_array[2];
> >
> >complx_array[0].i = 4;
> >complx_array[0].inner.l = 4;
> >complx_array[0].complex_ptr = &complx_array[1];
> >complx_array[1].i = 5;
> >complx_array[1].inner.l = 5;
> >complx_array[1].complex_ptr = &complx_array[0];
> >
> >
> >
> > the code in CMICmdCmdVarUpdate::ExamineSBValueForChange will get into
> > an infinite loop.
> >
> > const MIuint nChildren = vrwValue.GetNumChildren(); for (MIuint i =
> > 0; i < nChildren; ++i) {
> >lldb::SBValue member = vrwValue.GetChildAtIndex(i);
> >if (!member.IsValid())
> > continue;
> >
> >if (member.GetValueDidChange()) {
> > vrwbChanged = true;
> > return MIstatus::success;
> >} else if (ExamineSBValueForChange(member, vrwbChanged) &&
> vrwbChanged)
> > // Handle composite types (i.e. struct or arrays)
> > return MIstatus::success;
> > }
> >
> > I've got a patch that disables checking a pointer's children. I'll put
> > it up on phabricator today.
> >
> > Ted
> >
> > --
> > Qualcomm Innovation Center, Inc.
> > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
> >
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] hang bug in lldb-mi -var-update
lldb-mi should never be checking the children. This is never a good idea due to
performance. What happens when you have an array with a million entries? Long
delay. Aggregate types should never say they changed. Only SBValue objects that
have values should claim to change.
Greg
> On Aug 25, 2017, at 10:42 AM, Ted Woodward via lldb-dev
> wrote:
>
> I found a hang in lldb-mi's -var-update. It checks to see if a var changed,
> then it checks each of the children recursively. If a child is a pointer
> back to a parent, as in this case:
>
> struct complex_type
> {
>int i;
>struct { long l; } inner;
>struct complex_type *complex_ptr;
> };
>
> void
> var_update_test(void)
> {
>struct complex_type complx_array[2];
>
>complx_array[0].i = 4;
>complx_array[0].inner.l = 4;
>complx_array[0].complex_ptr = &complx_array[1];
>complx_array[1].i = 5;
>complx_array[1].inner.l = 5;
>complx_array[1].complex_ptr = &complx_array[0];
>
>
>
> the code in CMICmdCmdVarUpdate::ExamineSBValueForChange will get into an
> infinite loop.
>
> const MIuint nChildren = vrwValue.GetNumChildren();
> for (MIuint i = 0; i < nChildren; ++i) {
>lldb::SBValue member = vrwValue.GetChildAtIndex(i);
>if (!member.IsValid())
> continue;
>
>if (member.GetValueDidChange()) {
> vrwbChanged = true;
> return MIstatus::success;
>} else if (ExamineSBValueForChange(member, vrwbChanged) && vrwbChanged)
> // Handle composite types (i.e. struct or arrays)
> return MIstatus::success;
> }
>
> I've got a patch that disables checking a pointer's children. I'll put it up
> on phabricator today.
>
> Ted
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
[lldb-dev] hang bug in lldb-mi -var-update
I found a hang in lldb-mi's -var-update. It checks to see if a var changed,
then it checks each of the children recursively. If a child is a pointer
back to a parent, as in this case:
struct complex_type
{
int i;
struct { long l; } inner;
struct complex_type *complex_ptr;
};
void
var_update_test(void)
{
struct complex_type complx_array[2];
complx_array[0].i = 4;
complx_array[0].inner.l = 4;
complx_array[0].complex_ptr = &complx_array[1];
complx_array[1].i = 5;
complx_array[1].inner.l = 5;
complx_array[1].complex_ptr = &complx_array[0];
the code in CMICmdCmdVarUpdate::ExamineSBValueForChange will get into an
infinite loop.
const MIuint nChildren = vrwValue.GetNumChildren();
for (MIuint i = 0; i < nChildren; ++i) {
lldb::SBValue member = vrwValue.GetChildAtIndex(i);
if (!member.IsValid())
continue;
if (member.GetValueDidChange()) {
vrwbChanged = true;
return MIstatus::success;
} else if (ExamineSBValueForChange(member, vrwbChanged) && vrwbChanged)
// Handle composite types (i.e. struct or arrays)
return MIstatus::success;
}
I've got a patch that disables checking a pointer's children. I'll put it up
on phabricator today.
Ted
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
