[cfe-dev] [PATCH] Clang Static Analyzer support for temporary destructors

[Alex McCarthy]

Hi all,

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:
http://comments.gmane.org/gmane.comp.compilers.clang.devel/33901

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

This new patch fixes all of the new regression tests added after Pavel's
change was reverted, notably including
http://llvm-reviews.chandlerc.com/rL187133 . 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
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).


Now for the bad news:

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 "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.

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.

If we can get this patch working, it should address the following issues:
http://llvm.org/bugs/show_bug.cgi?id=15599
http://llvm.org/bugs/show_bug.cgi?id=16664
http://llvm.org/bugs/show_bug.cgi?id=18159 (not sure, this bug is
referenced by a newly fixed test in test/Analysis/temporaries.cpp)

Thanks for your help,
-Alex McCarthy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140302/8486810b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: temporary-destructors.patch
Type: application/octet-stream
Size: 18083 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140302/8486810b/attachment.obj>