Re: [Qemu-devel] Can we make better use of Coverity?

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> On 21/01/2015 13:47, Markus Armbruster wrote:
>> I also use Coverity locally (requires a license) with a derived model
>> for GLib to increase scanning power.  Since last July, the number of
>> defects I get that way has increased from ~400 to ~700.  Not quite as
>> bad as it sounds, because ~100 of the new ones are DEADCODE.  Still, it
>> suggests we haven't made much progress in reducing the number of defects
>> to a manageable level.
>
> While I agree that the current frequency of scans is too low, things are
> not too bad.  When I do run a scan, I get 10-20 issues.  This is a
> volume that I can triage, and I can also (depending on the component)
> send the most egregious out to the maintainer or the author of the
> offending patch.  It takes me between an hour and two.
>
> There are "only" 221 outstanding defects on Coverity scan, most of which
> actually have never been triaged.  This means that maintainers are good
> at fixing bugs.  In fact, about 120 of these 221 defects are split
> between the "9p", "bt", "disas", "other", "slirp" and "user" components
> (i.e. the worst components + the catchall component).  None of the bad
> components are in active development; unfortunately, this means that
> 70-80 defects probably will never be fixed.
>
> Every now and then I refine the components, mostly by looking at defects
> in the "other" category.  This had the nice side effect of making
> "other" no longer one of the bad components; it's way below the average
> of the project.  As a rule of thumb, either we know something is bad, or
> we maintain it well.  Again, this is not bad news.
>
> QEMU is also using a GLib model on Coverity Scan, as well as a
> QEMU-specific model, which suggests one of the following:

What do you mean by "a GLib model"?  scripts/coverity-model.c?

> 1) your model is not tailored well to QEMU;

I use cov-analyze with

    -co BAD_FREE:allow_first_field:true
    --security
    --concurrency
    --derived-model-file $wherever/glib-2.38.2.xmldb
    --user-model-file scripts/coverity-model.xmldb

where coverity-model.xmldb is made with

    $ cov-make-library -of scripts/coverity-model.xmldb scripts/coverity-model.c

and glib-2.38.2.xmldb is made with

    $ cov-collect-models --dir cov -of glib-2.38.2.xmldb

after a cov-analyze run of a fresh compile of Fedora-20's GLib.

> 2) you are not weeding out false positives.

Guilty as charged.  The proper place to do that is the Scan service,
where all of us can profit.

> Between the model, the triaging, and the fixing efforts, our defect rate
> has gone down from 0.88 to 0.24 in a year, which I think is pretty good.
>  (We could probably it down to 0.15, it's hard to go below that).

As I said: "We've put in some effort, and we've gotten some mileage out
of it, but I feel we could get more."

>> Some of the new defects are avoidable.  For instance, we've added 16
>> MISSING_BREAK.  Probably just missing /* fall through */, but we can't
>> be sure without examining each case.  Patch review fail.
>
> Or just that we do not care.  Missing /* fall through */ should either
> be flagged by the compiler,

Unfortunately, gcc doesn't.  Relying on tools for this is fine, but
requires actual use of said tools.  Which this thread is about :)

>                             or treated as a bonus.  Detecting missing
> fall through comments is a waste of reviewer brain bandwidth.
>
>> At the other end of the spectrum, I see 36 new UNINIT defects.
>> 
>> I think we should scan much more regularly.  Once a week, full auto?
>> 
>> I further think we should send the e-mail report to the list, to have
>> more eyes on it.
>> 
>> Opinions?
>> 
>> 
>> [*] https://scan.coverity.com/projects/378