<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><blockquote type="cite"><div>On Jun 22, 2014, at 5:10 PM, Bruce Mitchener <<a href="mailto:bruce.mitchener@gmail.com">bruce.mitchener@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Jun 22, 2014 at 11:29 PM, Enrico Granata <span dir="ltr"><<a href="mailto:egranata@apple.com" target="_blank">egranata@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Bruce,<br>
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"<br>
<br>
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.<br>
</blockquote><div><br></div><div>That isn't the case with my patch as I understand things.</div><div><br></div><div>Looking at the code, but stripped of comments:</div><div><br></div><div>bool</div><div>ValueObjectPrinter::ShouldPrintChildren (bool is_failed_description,</div>
<div> uint32_t& curr_ptr_depth)</div><div>{</div><div> const bool is_ref = IsRef ();</div><div> const bool is_ptr = IsPtr ();</div><div><br></div><div> if (is_failed_description || m_curr_depth < options.m_max_depth)</div>
<div> {</div><div> bool print_children = false;</div><div> if (is_ptr || is_ref)<br></div><div> {</div><div> AddressType ptr_address_type;<br></div><div> if (m_valobj->GetPointerValue (&ptr_address_type) == 0)</div>
<div> return false;</div><div><br></div><div> else if (is_ref && m_curr_depth == 0 && curr_ptr_depth == 0)</div><div> {</div><div> curr_ptr_depth = 1;<br>
</div><div> }</div><div><br></div><div> print_children = (curr_ptr_depth > 0);</div><div> }</div><div><br></div><div> TypeSummaryImpl* entry = GetSummaryFormatter();</div><div><br></div>
<div> return (print_children || !entry || entry->DoesPrintChildren(m_valobj) || m_summary.empty());</div><div> }</div><div> return false;</div><div>} </div><div><br></div><div>You can see that my change is squarely within the depth check, so it won't allow infinite recursion. </div></div></div></div></div></blockquote><div><br></div><div>Well, it actually will - which is most likely a bug in the way we handle pointer depths now, but consider the following:</div><div><div>class SThingChildren:</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>def __init__(self,valobj,*args):</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>self.valobj = valobj</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>def num_children(self):</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>return 1</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>def get_child_index(self,name):</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>return 0</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>def update(self):</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>return False</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>def has_children(self):</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>return True</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>def get_child_at_index(self,index):</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>return self.valobj</div><div><br></div><div>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</div><div>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</div><div>Then in PrintChild(), curr_ptr_depth is 0, so it doesn’t get decreased!</div><div><br></div><div>Essentially this works with our current code because a cur_ptr_depth of 0 always means “don’t print”</div><div><br></div></div><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> If m_curr_depth >= options.m_max_depth, it will still return false.</div>
<div><br></div><div>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.</div><div><br></div></div></div></div></div></blockquote><div><br></div><div>Which is a good thing, but as-is this patch opens the door to problems</div><div><br></div><div>The best thing I could come up that avoids the issue is:</div><div><br></div><div><div style="margin: 0px;"><font face="Menlo"><span style="font-size: 11px;">diff --git a/source/DataFormatters/ValueObjectPrinter.cpp b/source/DataFormatters/ValueObjectPrinter.cpp<br>index be78ed4..2389c3f 100644<br>--- a/source/DataFormatters/ValueObjectPrinter.cpp<br>+++ b/source/DataFormatters/ValueObjectPrinter.cpp<br>@@ -530,6 +530,10 @@ ValueObjectPrinter::ShouldPrintChildren (bool is_failed_description,<br> // otherwise we can end up with infinite recursion...<br> curr_ptr_depth = 1;<br> }<br>+ else if (is_ptr && m_curr_depth == 0 && curr_ptr_depth == 0 && options.m_max_ptr_depth > 0)<br>+ {<br>+ curr_ptr_depth = 1;<br>+ }<br> <br> return (curr_ptr_depth > 0);<br> }<br></span></font><br></div><div style="margin: 0px;">If you want to pursue this avenue, you may want to test with something along these lines and then resubmit</div></div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> - Bruce</div>
<div><br></div></div></div></div>
</div></blockquote></div><br></body></html>