[Lldb-commits] [PATCH] Fix showing children of synthetic providers for void*

[Enrico Granata]

> On Jun 22, 2014, at 5:10 PM, Bruce Mitchener <bruce.mitchener at gmail.com> wrote:
> 
> On Sun, Jun 22, 2014 at 11:29 PM, Enrico Granata <egranata at apple.com> 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
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140623/4fc43d63/attachment.html>