> On Jun 22, 2014, at 5:10 PM, Bruce Mitchener <[email protected]>
> wrote:
>
> On Sun, Jun 22, 2014 at 11:29 PM, Enrico Granata <[email protected]> wrote:
> Bruce,
> your patch seems to essentially implement the algorithm "display children of
> pointer types if within the pointer depth OR there is a formatter that wants
> them shown"
>
> That's somewhat worrisome. The reason for having a pointer depth is that
> pointers potentially introduce recursion - i.e. you can have a pointer that
> points to itself. Limiting how deep you look into pointer values has the
> benefit that lldb will not hang in such situations trying to display an i
> infinitely recursive structure - which is especially useful if the infinite
> recursion was accidental and unexpected.
>
> That isn't the case with my patch as I understand things.
>
> Looking at the code, but stripped of comments:
>
> bool
> ValueObjectPrinter::ShouldPrintChildren (bool is_failed_description,
> uint32_t& curr_ptr_depth)
> {
> const bool is_ref = IsRef ();
> const bool is_ptr = IsPtr ();
>
> if (is_failed_description || m_curr_depth < options.m_max_depth)
> {
> bool print_children = false;
> if (is_ptr || is_ref)
> {
> AddressType ptr_address_type;
> if (m_valobj->GetPointerValue (&ptr_address_type) == 0)
> return false;
>
> else if (is_ref && m_curr_depth == 0 && curr_ptr_depth == 0)
> {
> curr_ptr_depth = 1;
> }
>
> print_children = (curr_ptr_depth > 0);
> }
>
> TypeSummaryImpl* entry = GetSummaryFormatter();
>
> return (print_children || !entry ||
> entry->DoesPrintChildren(m_valobj) || m_summary.empty());
> }
> return false;
> }
>
> You can see that my change is squarely within the depth check, so it won't
> allow infinite recursion.
Well, it actually will - which is most likely a bug in the way we handle
pointer depths now, but consider the following:
class SThingChildren:
def __init__(self,valobj,*args):
self.valobj = valobj
def num_children(self):
return 1
def get_child_index(self,name):
return 0
def update(self):
return False
def has_children(self):
return True
def get_child_at_index(self,index):
return self.valobj
and attach this provider to a typedef of void* - with your patch we will allow
the children to be generated all the time when the user doesn’t pass a pointer
depth
Since the max_depth is 0, m_curr_depth is also 0, so the first check succeeds,
then even though print_children is false, we expand children
Then in PrintChild(), curr_ptr_depth is 0, so it doesn’t get decreased!
Essentially this works with our current code because a cur_ptr_depth of 0
always means “don’t print”
> If m_curr_depth >= options.m_max_depth, it will still return false.
>
> The only situation where my patch actually changes things with regards to
> pointer depth is that now, a pointer at depth 0 will result in checking the
> summary formatter.
>
Which is a good thing, but as-is this patch opens the door to problems
The best thing I could come up that avoids the issue is:
diff --git a/source/DataFormatters/ValueObjectPrinter.cpp
b/source/DataFormatters/ValueObjectPrinter.cpp
index be78ed4..2389c3f 100644
--- a/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/source/DataFormatters/ValueObjectPrinter.cpp
@@ -530,6 +530,10 @@ ValueObjectPrinter::ShouldPrintChildren (bool
is_failed_description,
// otherwise we can end up with infinite recursion...
curr_ptr_depth = 1;
}
+ else if (is_ptr && m_curr_depth == 0 && curr_ptr_depth == 0 &&
options.m_max_ptr_depth > 0)
+ {
+ curr_ptr_depth = 1;
+ }
return (curr_ptr_depth > 0);
}
If you want to pursue this avenue, you may want to test with something along
these lines and then resubmit
> - Bruce
>
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
