[cfe-dev] clang-analyzer NewDeleteLeaks : best way to silence a false positive

[Jonathan Roelofs via cfe-dev]

On 6/28/16 6:14 PM, Benoit Belley via cfe-dev wrote:
> Hi everyone,
>
> The clang static analyzer gives a false positive warning on the
> following program:
>
>     $ cat falseLeak.cpp
>     struct CtrlBlk {
>         explicit CtrlBlk() : m_useCount(1) {}
>
>         virtual void dispose();
>
>         void decUseCount() {
>             -- m_useCount;
>             if (m_useCount == 0) { dispose(); }
>         }
>
>         int m_useCount;
>     };
>
>     struct Ptr {
>         Ptr() { m_cntrlBlk = new CtrlBlk; }
>         ~Ptr() { m_cntrlBlk->decUseCount(); }
>         CtrlBlk* m_cntrlBlk;
>     };
>
>     void testCase() { Ptr px; }
>
>
>     $ clang-tidy -checks=* falseLeak.cpp
>     1 warning generated.
>     /Users/benoit/tmp/falseLeak.cpp:20:27: warning: Potential leak of
>     memory pointed to by 'px.m_cntrlBlk'
>     [clang-analyzer-cplusplus.NewDeleteLeaks]
>     void testCase() { Ptr px; }
>                               ^
>     /Users/benoit/tmp/falseLeak.cpp:20:23: note: Calling default
>     constructor for 'Ptr'
>     void testCase() { Ptr px; }
>                           ^
>     /Users/benoit/tmp/falseLeak.cpp:15:26: note: Memory is allocated
>         Ptr() { m_cntrlBlk = new CtrlBlk; }
>                              ^
>     /Users/benoit/tmp/falseLeak.cpp:20:23: note: Returning from default
>     constructor for 'Ptr'
>     void testCase() { Ptr px; }
>                           ^
>     /Users/benoit/tmp/falseLeak.cpp:20:27: note: Potential leak of
>     memory pointed to by 'px.m_cntrlBlk'
>     void testCase() { Ptr px; }
>                               ^
>
>
> Here’s my solution to silence up that false positive:
>
>     #ifdef __clang_analyzer__
>         /// \brief Escape a variable from the analysis of the clang
>     static analyzer
>         ///
>         /// This function takes a reference to a variable as an
>     argument. This
>         /// causes the static analyzer tool to notice that the variable
>     has escape
>         /// the scope of its analysis. It forces the static analyzer to
>     make very
>         /// conservative assumptions about the state of the variable.
>     This also
>         /// includes assumptions about any memory location referenced by
>     this
>         /// variable.
>         ///
>         /// This can be used to silence up false positives. In general,
>     it is
>         /// better to augment the information available to the static
>     analyzer
>         /// using attributes and/or assertions. Unfortunately, this
>     isn't always
>         /// possible. For example, it might be impossible to describe
>     that a given
>         /// object is destructed along all execution paths.
>         template <typename V> static void
>     escapeFromClangStaticAnalysis(V& escapedVar);
>     #endif
>
>     struct CtrlBlk {
>         explicit CtrlBlk() : m_useCount(1) {}
>
>         virtual void dispose();
>
>         void decUseCount() {
>             -- m_useCount;
>             if (m_useCount == 0) { dispose(); }
>     #ifdef __clang_analyzer__
>             escapeFromClangStaticAnalysis(m_useCount);
>     #endif
>         }
>
>         int m_useCount;
>     };
>
>     struct Ptr {
>         Ptr() { m_cntrlBlk = new CtrlBlk; }
>         ~Ptr() { m_cntrlBlk->decUseCount(); }
>         CtrlBlk* m_cntrlBlk;
>     };
>
>     void testCase() { Ptr px; }
>
>
> Would anyone have a better solution ?

Does dispose() do `delete this`?

If so, I think the first problem is that the analyzer cannot possibly 
see that it does in this example.

Secondly, while legal, that's a very strange pattern, and it prevents 
you from using CtrlBlk with new[], placement new, as a stack variable, 
etc (i.e. if you call dispose(), you're only allowed to use it with 
plain old new).


Jon

>
> Thanks,
> Benoit
>
> *Benoit Belley*
>
> Sr Principal Developer
>
> M&E-Product Development Group
>
>
>
> *MAIN* +1 514 393 1616
>
> *DIRECT* +1 438 448 6304
>
> *FAX* +1 514 393 0110
>
>
>
> Twitter <http://twitter.com/autodesk>
>
> Facebook <https://www.facebook.com/Autodesk>
>
>
>
> *Autodesk, Inc.*
>
> 10 Duke Street
>
> Montreal, Quebec, Canada H3C 2L7
>
> www.autodesk.com <http://www.autodesk.com/>
>
>
>
> Description: Email_Signature_Logobar
>
>
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded