<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Apr 30, 2018, at 6:28 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Sorry - could we revisit this?<br class=""></div></div></blockquote><div><br class=""></div><div>Of course :-) </div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><br class="">I think my concerns expressed still haven't been resolved:<br class=""><br class="">"<span style="color:rgb(33,33,33)" class="">Could you check the revision history here? I'm pretty sure the first version of this I reviewed from Greg wasn't recursive - and then it became recursive at some point to handle something needed, but maybe those decisions need to be reexamined?" (& related comments back there)</span></div></div></blockquote><div><br class=""></div><div>Greg added the recursiveness in D40156 because he saw DWARFDie::getName() failing “in the wild”. I think it was for some Google project but I’d have to double check the mail thread. </div><div><br class=""></div><div>IIRC we all agreed that for “valid” DWARF we shouldn’t need the recursion. The question is whether we want to support the “invalid case or not, as it’s definitely being generated (but maybe not by clang?). If the answer is yes, then r331200 is strictly better than what we had before. Now we don’t crash if there’s a cycle. If the answer is no, we just need to go back to the non-recursive implementation. Personally I don’t have a strong opinion, as long as we don’t crash.</div><div><br class=""></div><div>Cheers,</div><div>Jonas </div><div><br class=""></div><blockquote type="cite" class=""><div class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, Apr 30, 2018 at 10:06 AM Jonas Devlieghere via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This revision was automatically updated to reflect the committed changes.<br class="">
Closed by commit rL331200: [DebugInfo] Prevent infinite recursion for malformed DWARF (authored by JDevlieghere, committed by ).<br class="">
<br class="">
Changed prior to commit:<br class="">
<a href="https://reviews.llvm.org/D43092?vs=144485&id=144582#toc" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D43092?vs=144485&id=144582#toc</a><br class="">
<br class="">
Repository:<br class="">
rL LLVM<br class="">
<br class="">
<a href="https://reviews.llvm.org/D43092" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D43092</a><br class="">
<br class="">
Files:<br class="">
llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp<br class="">
llvm/trunk/test/tools/llvm-dwarfdump/X86/invalid_abstract_origin.s<br class="">
<br class="">
</blockquote></div>
</div></blockquote></div><br class=""></body></html>