Re: GC/GTY and Coverity "resource leaks" (was Re: Daily Coverity analysis on gcc)

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2017-06-26 at 15:14 -0400, David Malcolm wrote:
> On Mon, 2017-06-26 at 06:36 -0700, Sylvestre Ledru wrote:
> > Over the last few weeks, working with Jonathan, I have been running
> > daily analysis of Coverity (a proprietary static analyzer tool)
> > It is running on a Debian amd64 with the options:
> > --with-gnu-as --with-gnu-ld --disable-bootstrap 
> > --enable-languages=jit,c,c++,fortran,lto,objc  --enable-host-shared
> > 
> > Coverity finds about 4300 defects. The defect density is 1.67 for
> > 2.5
> > millions line of code.
> > 
> > As a comparison, the llvm toolchain (llvm, clang, lldb, etc) has
> > 3223
> > defects (0.62 of defect density for 5.1M loc).
> > 
> > Most of the defect are detected as resource leaks (1681 defects). I
> > had
> > a quick look and many of them are actual issues (in many cases
> > minor), I
> > took the
> > opportunity to fix some of them.
> > 
> > To access to the list of results, you can apply
> > https://scan.coverity.com/projects/gcc, please try to prove that
> > you
> > are
> > a gcc contributor.
> > 
> > Thanks,
> > Sylvestre
> 
> Thanks.
> 
> The most recent one, and thus the first one I looked at (CID=1412982)
> was a supposed "Resource Leak" in the get_cast_suggestion function I
> introduced in r249461, where the local variable "trial" is supposedly
> leaked (leading to 3 issues within Coverity overall, if I'm reading
> things right, for the 3 ways in which a non-NULL value can be
> generated
> and then fall out of scope).
> 
> But this is a tree, and will eventually be GC-ed.
> 
> So is there a way to teach Coverity about GTY and our garbage
> collector?  I'm wondering how many of the resource leak defects are
> similar false positives.
> 
> Dave

Partially answering my own question, in:
  https://scan.coverity.com/models
lists various primitives that can be used in "models", which appear to
be small fragments of fake implementations of functions that the
analyzer can't see.

One of them is:
  __coverity_escape__
which "Models a function that saves its argument (for example, in a
global variable) so it can be freed later. The analysis will not report
a resource leak on such an argument once it escapes."

Although it's not in a "model", perhaps there's a way of hooking that
up to our GC-allocation functions so that the checker "knows" that the
memory can be reclaimed at some later time?

Similarly, there's a reference at that URL to "code annotations", one
of which is:

// coverity[-alloc]

which will suppress the check that the returned pointer is freed. 

Digging into the report in question, it appears to be using the
implementation of ggc_internal_alloc from ggc-none.c, which is a thin
wrapper around xmalloc.

Hence perhaps we need something like the following (untested), to
persuade Coverity not to check for explicit frees of GC-allocated
blocks?

Thoughts?

--- a/gcc/ggc-none.c
+++ b/gcc/ggc-none.c
@@ -40,6 +40,7 @@ ggc_round_alloc_size (size_t requested_size)
   return requested_size;
 }
 
+// coverity[-alloc]
 void *
 ggc_internal_alloc (size_t size, void (*f)(void *), size_t, size_t
                    MEM_STAT_DECL)
@@ -48,6 +49,7 @@ ggc_internal_alloc (size_t size, void (*f)(void *), size_t, size_t
   return xmalloc (size);
 }
 
+// coverity[-alloc]
 void *
 ggc_internal_cleared_alloc (size_t size, void (*f)(void *), size_t, size_t
                            MEM_STAT_DECL)
@@ -56,6 +58,7 @@ ggc_internal_cleared_alloc (size_t size, void (*f)(void *), size_t, size_t
   return xcalloc (size, 1);
 }
 
+// coverity[-alloc]
 void *
 ggc_realloc_stat (void *x, size_t size MEM_STAT_DECL)
 {