<div dir="ltr">Hi all,<div><br></div><div><span>I'm running clang's static analyzer on a C++ codebase at Google. I saw a roughly a 50% false positive rate which stemmed from the analyzer not recognizing temporary object destructors: this issue is discussed in some length in another thread, which mentions a similar error rate on the Chromium codebase: <a class="cremed" href="http://comments.gmane.org/gmane.comp.compilers.clang.devel/33901">http://comments.gmane.org/gmane.comp.compilers.clang.devel/33901</a></span></div>

<div><br></div><div><span>Starting from Pavel's work which was reverted in <a class="cremed" href="http://llvm-reviews.chandlerc.com/rL186925">http://llvm-reviews.chandlerc.com/rL186925</a> , I've put together a new patch (see attachment) that attempts to fix temporary object destructor handling.</span></div>

<div><span><br></span></div><div><span>This new patch fixes all of the new regression tests added after Pavel's change was reverted, notably including <a class="cremed" href="http://llvm-reviews.chandlerc.com/rL187133">http://llvm-reviews.chandlerc.com/rL187133</a> . I've also fixed some other crashes, including a crash when processing an array of temporary objects use in a C++11 initializer_list, covered by a new regression test in </span>cfe/test/Analysis/dtor-cxx11.cpp . And most importantly, running clang with this patch eliminates the 50% false positive rate I saw previously (from ~800 warnings to ~400 across the ~1700 file codebase).</div>

<div><span><br></span></div><div><span><br></span></div><div><span>Now for the bad news:</span></div><div><span><br></span></div><div><span>This patch introduces a new regression which wasn't covered by existing tests: named temporaries declared and used within if statements are considered dead while they're still being used, which results in "</span>Undefined or garbage value returned to caller" errors. I've added regression tests to test/Analysis/dtor.cpp to cover this case, which currently fail. I've also updated test/Analysis/temp-obj-dtors-cfg-output.cpp with relevant CFG dumps to try to debug the problem. This new false positive is much nosier than the ones this patch fixes: the only advantage to the current patch as-is is that the garbage return value warnings are emitted in a small collection of header files, making them much easier to ignore en masse.</div>

<div><br></div><div>I don't have any compiler experience, so I'm moving slowly in the clang codebase and could use some help understanding where to look next. I've mostly been handling each crash or error as I find it, but I don't have a high level understanding of the impact or context of my change. In particular, I don't know how to read the CFG dumps I've generated, so I'm not sure where things are going wrong. Ted, Jordan, and Anna: Manuel Klimek mentioned that you've looked into this issue at length. Do have any advice on what I'm doing wrong, or could you suggest other approaches I might be able to try? Anything you can think of that can speed up my search for a fix would be greatly appreciated.</div>

<div><br></div><div>If we can get this patch working, it should address the following issues:</div><div><a href="http://llvm.org/bugs/show_bug.cgi?id=15599" class="cremed">http://llvm.org/bugs/show_bug.cgi?id=15599</a> </div>

<div><span><a class="cremed" href="http://llvm.org/bugs/show_bug.cgi?id=16664">http://llvm.org/bugs/show_bug.cgi?id=16664</a></span></div><div><span></span><span><a class="cremed" href="http://llvm.org/bugs/show_bug.cgi?id=18159">http://llvm.org/bugs/show_bug.cgi?id=18159</a> (not sure, this bug is referenced by a newly fixed test in </span>test/Analysis/temporaries.cpp)</div>

<div><br></div><div>Thanks for your help,<br clear="all"><div><span style="color:rgb(153,153,153)">-Alex McCarthy</span><br></div>
</div></div>