<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 4, 2014 at 3:15 PM, Artyom Skrobov <span dir="ltr"><<a href="mailto:Artyom.Skrobov@arm.com" target="_blank">Artyom.Skrobov@arm.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"><div lang="EN-GB" link="blue" vlink="purple"><div><p class="MsoNormal">
<span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">If I’m not able to reproduce the issue, how do you expect me to see if I fixed it?</span></p></div></div></blockquote><div><br></div><div>Fair enough. On the other hand, compiling an average open-source project nowadays isn't that complicated. With the one I pointed you to, it's a matter of:</div>
<div> <span style="color:rgb(0,0,0)">svn checkout -r 2229 <a href="http://dynamorio.googlecode.com/svn/trunk/">http://dynamorio.googlecode.com/svn/trunk/</a> dynamorio && cd dynamorio && cmake . && make all </span></div>
<div>This configures the project and generates all missing headers. But if you prefer to look at the preprocessed source, I'm sending it to you. Please see options.i.gz. If sending an attachment to the list doesn't work, let me know and I'll send it to you directly.</div>
<div> </div><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"><div lang="EN-GB" link="blue" vlink="purple"><p class="MsoNormal">
<span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Please run clang -E in your environment that has all headers that options.c requires, creating a single isolated source file that shows the problem.</span></p>
</div></blockquote><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"><div lang="EN-GB" link="blue" vlink="purple">
<div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">A fix would need a regression test to go in, anyway.<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p><div style="border-style:solid none none;border-top-color:rgb(181,196,223);border-top-width:1pt;padding:3pt 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span lang="EN-US" style="font-size:10pt;font-family:Tahoma,sans-serif"> Alexander Kornienko [mailto:<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>] <br>
<b>Sent:</b> 04 August 2014 14:03</span></p><div><div class="h5"><br><b>To:</b> Artyom Skrobov<br><b>Cc:</b> Hal Finkel; Ted Kremenek; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a> Commits<br>
<b>Subject:</b> Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses<u></u><u></u></div></div><p></p></div><div><div class="h5"><p class="MsoNormal"><u></u> <u></u></p><div><div><div>
<p class="MsoNormal">On Mon, Aug 4, 2014 at 2:31 PM, Artyom Skrobov <<a href="mailto:Artyom.Skrobov@arm.com" target="_blank">Artyom.Skrobov@arm.com</a>> wrote:<u></u><u></u></p><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">May I ask you for an isolated source file that would reproduce this issue?</span> Actually, I've sent you the link to the file. Getting a compilable copy locally is a matter of:</p>
<p class="MsoNormal"> svn checkout -r 2229 <a href="http://dynamorio.googlecode.com/svn/trunk/">http://dynamorio.googlecode.com/svn/trunk/</a> dynamorio && cd dynamorio && cmake . && make all </p>
</div><div><br></div><blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">options.c seems to require a whole tree of headers, including event_strings.h which is not present in the repository at all.</span><u></u><u></u></p>
</div></blockquote><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">I think, the issue is obvious: you've taken two different versions of the code and dropped one of them on the floor. Apparently, they were different for a reason. The fix could be as easy as adding back the version from LiveVariables.cpp (as another derived class, for example, once you already have inheritance there), and using it in LiveVariables.<u></u><u></u></p>
</div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">If you want to go further and understand the reason why they should be different, and what exactly breaks, the code I pointed at is probably the best I can provide. You can try looking at revision 2229: <a href="https://code.google.com/p/dynamorio/source/browse/trunk/core/options.c?r=2229" target="_blank">https://code.google.com/p/dynamorio/source/browse/trunk/core/options.c?r=2229</a>, it should be the closest one to the version I found the issue on.<u></u><u></u></p>
</div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">But please first send a patch to fix the issue in the simpliest manner.<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p>
</div><div><p class="MsoNormal">Thank you!<u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p></div><blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm">
<div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> </span><u></u><u></u></p>
<div style="border-style:solid none none;border-top-color:rgb(181,196,223);border-top-width:1pt;padding:3pt 0cm 0cm"><p class="MsoNormal"><b><span lang="EN-US" style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span lang="EN-US" style="font-size:10pt;font-family:Tahoma,sans-serif"> Alexander Kornienko [mailto:<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>] <br>
<b>Sent:</b> 04 August 2014 13:25<br><b>To:</b> Artyom Skrobov<br><b>Cc:</b> Hal Finkel; Ted Kremenek; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a> Commits</span><u></u><u></u></p><div>
<div><p class="MsoNormal"><br><b>Subject:</b> Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses<u></u><u></u></p></div></div></div><div><div><p class="MsoNormal"> <u></u><u></u></p>
<div><p class="MsoNormal">On Mon, Aug 4, 2014 at 2:22 PM, Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>> wrote:<u></u><u></u></p><div><p class="MsoNormal">This patch makes the <span style="color:black">deadcode.DeadStores analyzer hang on this file: </span><a href="https://code.google.com/p/dynamorio/source/browse/trunk/core/options.c" target="_blank">https://code.google.com/p/dynamorio/source/browse/trunk/core/options.c</a><u></u><u></u></p>
<div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">The relevant part of stack trace looks like this:<u></u><u></u></p></div><div><pre><span style="color:black"> 0x00e189ed: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::balanceTree(llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*, clang::Stmt const*, llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*)</span><u></u><u></u></pre>
<pre><span style="color:black"> 0x00e18d2e: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::add_internal(clang::Stmt const*, llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*)</span><u></u><u></u></pre>
<pre><span style="color:black"> 0x00e18ec5: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::add_internal(clang::Stmt const*, llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*)</span><u></u><u></u></pre>
<pre><span style="color:black"> 0x00e1b292: llvm::ImutAVLFactory<llvm::ImutContainerInfo<clang::Stmt const*> >::add(llvm::ImutAVLTree<llvm::ImutContainerInfo<clang::Stmt const*> >*, clang::Stmt const*)</span><u></u><u></u></pre>
<pre><span style="color:black"> 0x00e1f2d1: clang::LiveVariables::computeLiveness(clang::AnalysisDeclContext&, bool)</span><u></u><u></u></pre><pre><span style="color:black"> 0x00673969: void clang::ento::check::ASTCodeBody::_checkBody<(anonymous namespace)::DeadStoresChecker>(void*, clang::Decl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&)</span><u></u><u></u></pre>
<div><div><p class="MsoNormal">Did you notice that the DataflowWorklist class was a bit different in these two classes? <u></u><u></u></p></div></div></div></div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">
I meant, "files" (<span style="font-size:10pt;font-family:Arial,sans-serif">UninitializedValues.cpp and LiveVariables.cpp)</span>, not "classes". <u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p>
</div><blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin:5pt 0cm 5pt 4.8pt"><div><p class="MsoNormal">Notably, the dequeue method and initialization of the <span style="font-size:10pt;font-family:Arial,sans-serif">enqueuedBlocks bit-vector.</span><u></u><u></u></p>
</div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal"><span style="font-family:Arial,sans-serif">Please fix or revert the patch.</span><u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p>
</div><div><p class="MsoNormal"><span style="font-family:Arial,sans-serif">Thank you!</span><u></u><u></u></p><div><p class="MsoNormal"> <u></u><u></u></p><div><p class="MsoNormal">On Tue, Jul 29, 2014 at 6:27 PM, Artyom Skrobov <<a href="mailto:Artyom.Skrobov@arm.com" target="_blank">Artyom.Skrobov@arm.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">Hal, thank you for the suggestion! I've expanded that comment four-fold in r214183.<br><br>Ted, thank you for reviewing the original patch, and nov worries it took a while.<br>You might want to also check that the comments I've added when committing r214064 are correct -- although they're essentially a rephrasing of comments from your own emails.<u></u><u></u></p>
<div><p class="MsoNormal" style="margin-bottom:12pt"><br><br>-----Original Message-----<br>From: Hal Finkel [mailto:<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>]<br>Sent: 28 July 2014 13:12<br>To: Artyom Skrobov<br>
Cc: <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>Subject: Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses<br><br><br>We should have a description here of what this code does, not just where it's used. One can get some idea by reading the comments above DataflowWorklist::enqueueSuccessors in the source file, but it is not clear that gives a complete picture.<br>
<br> -Hal<br><br><br><br><u></u><u></u></p></div><div><div><p class="MsoNormal">_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p></div></div></div><p class="MsoNormal"><u></u> </p></div></div></blockquote>
</div></div></div></div></blockquote></div></div></div></div></div></div></div></blockquote></div>
</div></div>