This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: GC/GTY and Coverity "resource leaks" (was Re: Daily Coverity analysis on gcc)
- From: David Malcolm <dmalcolm at redhat dot com>
- To: Sylvestre Ledru <sylvestre at debian dot org>, gcc at gcc dot gnu dot org
- Cc: Jonathan Wakely <jonathan dot wakely at gmail dot com>
- Date: Mon, 26 Jun 2017 16:27:43 -0400
- Subject: Re: GC/GTY and Coverity "resource leaks" (was Re: Daily Coverity analysis on gcc)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=dmalcolm at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 8C6D0C0587C0
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8C6D0C0587C0
- References: <793886b8-2fd8-e593-f065-2399412ba347@debian.org> <1498504452.7656.8.camel@redhat.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)
{