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

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

[Markus Armbruster]

We're using the Coverity Scan service[*].  We've put in some effort, and
we've gotten some mileage out of it, but I feel we could get more.

Judging from the report e-mail I have lying about, we're scanning about
once a month on average.  These reports cuts off after 20 new defects.
When there are more, which is common, people have to go to the web
dashboard to see them.  When I get one with ten, I may have a look, when
I get one "Showing 20 of 100 defect(s)", I despair of the task, and put
it off.

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.

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.

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

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

[Peter Maydell]

On 21 January 2015 at 12:47, Markus Armbruster <armbru@redhat.com> wrote:
> We're using the Coverity Scan service[*].  We've put in some effort, and
> we've gotten some mileage out of it, but I feel we could get more.
>
> Judging from the report e-mail I have lying about, we're scanning about
> once a month on average.  These reports cuts off after 20 new defects.
> When there are more, which is common, people have to go to the web
> dashboard to see them.  When I get one with ten, I may have a look, when
> I get one "Showing 20 of 100 defect(s)", I despair of the task, and put
> it off.

Right, but coverity reports lots of stuff, much of which is either
wrong or just not very important. The interesting stats here are:
(1) the "high impact outstanding" buglist: we have just 33 of these
(2) the per-component lists: where somebody's been working on the
    bug list for that component there are often not many bugs (there
    are just 2 outstanding for "arm", for instance)

> I think we should scan much more regularly.  Once a week, full auto?

I think a regular automated scan would be useful, yes.

> I further think we should send the e-mail report to the list, to have
> more eyes on it.

I agree that we'd benefit much more from more people seeing the
list of coverity reports.

-- PMM

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

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 21 January 2015 at 12:47, Markus Armbruster <armbru@redhat.com> wrote:
>> We're using the Coverity Scan service[*].  We've put in some effort, and
>> we've gotten some mileage out of it, but I feel we could get more.
>>
>> Judging from the report e-mail I have lying about, we're scanning about
>> once a month on average.  These reports cuts off after 20 new defects.
>> When there are more, which is common, people have to go to the web
>> dashboard to see them.  When I get one with ten, I may have a look, when
>> I get one "Showing 20 of 100 defect(s)", I despair of the task, and put
>> it off.
>
> Right, but coverity reports lots of stuff, much of which is either
> wrong or just not very important. The interesting stats here are:
> (1) the "high impact outstanding" buglist: we have just 33 of these
> (2) the per-component lists: where somebody's been working on the
>     bug list for that component there are often not many bugs (there
>     are just 2 outstanding for "arm", for instance)

I agree the sky is most definitely not falling.

The defect density is quite uneven (see appended table).  "arm" is in
good shape indeed, and the largest low-density component.  Top-scorers
are bt, slirp and 9pfs.  Figures; they feel barely maintained these
days.

>> I think we should scan much more regularly.  Once a week, full auto?
>
> I think a regular automated scan would be useful, yes.

Need a volunteer to script that.  Any takers?

>> I further think we should send the e-mail report to the list, to have
>> more eyes on it.
>
> I agree that we'd benefit much more from more people seeing the
> list of coverity reports.

I figure that's just a matter of creating a dummy member with the list
address.  Any objections?


Defect density by component, from
https://scan.coverity.com/projects/378?tab=overview

    Component Name  Line of Code    Defect density
    bt                4,610         1.74
    slirp             6,968         1.44
    9pfs              9,493         1.37
    user             32,263         0.68
    mips             34,321         0.52
    Other           390,967         0.51
    net              29,412         0.44
    lm32              2,836         0.35
    ui               43,771         0.32
    block            55,171         0.31
    ppc              50,323         0.28
    disas            38,362         0.26
    i386             36,786         0.22
    migration         5,249         0.19
    usb              26,524         0.19
    m68k              5,533         0.18
    s390             17,171         0.17
    sparc            14,677         0.14
    tricore           7,801         0.13
    pci              11,292         0.09
    scsi             14,521         0.07
    arm              69,085         0.01
    cris              6,341         0.00
    libcacard         3,779         0.00
    microblaze        3,482         0.00
    monitor          30,044         0.00
    nbd               1,714         0.00
    openrisc          3,102         0.00
    tcg              10,659         0.00
    trace             9,090         0.00
    unicore32         3,191         0.00
    xtensa            7,393         0.00

The size of "Other" shows that our component definitions could use a
little love, too :)

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

[Paolo Bonzini]

On 21/01/2015 14:58, Markus Armbruster wrote:
> Defect density by component, from
> https://scan.coverity.com/projects/378?tab=overview

Updated:

bt		4610	1.74	8
slirp		6968	1.44	10
9pfs		9493	1.37	13
user		32263	0.68	22
fpu		15355	0.59	9
(headers)	12323	0.57	7
mips		34321	0.52	18
net		27732	0.43	12
util		12668	0.39	5
lm32		2836	0.35	1
block		62844	0.35	22
ui		43828	0.34	15
ppc		49651	0.28	14
char		10703	0.28	3
disas		38362	0.26	10
i386		36786	0.22	8
migration	5249	0.19	1
usb		25647	0.19	5
m68k		5533	0.18	1
s390		17171	0.17	3
sparc		14788	0.14	2
tricore		7801	0.13	1
pci		11623	0.09	1
Other		297281	0.08	25
scsi		14521	0.07	1
audio		16304	0.06	1
arm		75435	0.03	2
Defect-free	93835	0.00	0
--------------------------------------------
Total		985931	0.22	220

Other is now smaller by 15%.  It will get even smaller as soon as the
list is refreshed: I've just created qemu-ga and xen components, as well
as moved more files out of Other (and into lm32, ppc and tcg).

Defect-free includes: alpha, cris, microblaze, openrisc, sh, unicore32,
xtensa, libcacard, monitor, nbd, tcg (though it will get a defect as
soon as the list is refreshed), trace.

There are a good number of subsystems that are really close to 0.

Paolo

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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 21/01/2015 14:58, Markus Armbruster wrote:
>> Defect density by component, from
>> https://scan.coverity.com/projects/378?tab=overview
>
> Updated:
>
> bt		4610	1.74	8
> slirp		6968	1.44	10
> 9pfs		9493	1.37	13
> user		32263	0.68	22
> fpu		15355	0.59	9
> (headers)	12323	0.57	7
> mips		34321	0.52	18
> net		27732	0.43	12
> util		12668	0.39	5
> lm32		2836	0.35	1
> block		62844	0.35	22
> ui		43828	0.34	15
> ppc		49651	0.28	14
> char		10703	0.28	3
> disas		38362	0.26	10
> i386		36786	0.22	8
> migration	5249	0.19	1
> usb		25647	0.19	5
> m68k		5533	0.18	1
> s390		17171	0.17	3
> sparc		14788	0.14	2
> tricore		7801	0.13	1
> pci		11623	0.09	1
> Other		297281	0.08	25
> scsi		14521	0.07	1
> audio		16304	0.06	1
> arm		75435	0.03	2
> Defect-free	93835	0.00	0
> --------------------------------------------
> Total		985931	0.22	220
>
> Other is now smaller by 15%.  It will get even smaller as soon as the
> list is refreshed: I've just created qemu-ga and xen components, as well
> as moved more files out of Other (and into lm32, ppc and tcg).

If you continue like this, the Coverity Scan service component list will
soon be more useful than MAINTAINERS ;)

> Defect-free includes: alpha, cris, microblaze, openrisc, sh, unicore32,
> xtensa, libcacard, monitor, nbd, tcg (though it will get a defect as
> soon as the list is refreshed), trace.
>
> There are a good number of subsystems that are really close to 0.

Thanks to the folks who put in the necessary work.  Much appreciated.

And special thank you to you, for your sustained effort with the
Coverity Scan service.

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

[Daniel P. Berrange]

On Wed, Jan 21, 2015 at 01:47:22PM +0100, Markus Armbruster wrote:
> We're using the Coverity Scan service[*].  We've put in some effort, and
> we've gotten some mileage out of it, but I feel we could get more.
> 
> Judging from the report e-mail I have lying about, we're scanning about
> once a month on average.  These reports cuts off after 20 new defects.
> When there are more, which is common, people have to go to the web
> dashboard to see them.  When I get one with ten, I may have a look, when
> I get one "Showing 20 of 100 defect(s)", I despair of the task, and put
> it off.
> 
> 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.
> 
> 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.
> 
> 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 agree that you need to scan much more regularly. Given the number of
patches QEMU merges, with only monthly scans you're creating a big job
for whoever has to deal with the monthly report because chances are
there will be alot of new stuff reported each month to wade through.

In libvirt we now have a coverity scan being run once a day, so when
we get new problems reported, the code in question is still fresh in
the mind of the reviewers & patch author. Daily scans also spread out
the workload much better. Only get a small number of new problems to
analyse a couple of times a week - never any real huge burden for the
person managing the coverity scan & more likely to get others to help
too if there's only a couple of things for them to look at instead of
a list of 700+ to wade through. I think these contribute to make it
practical for us to keep libvirt at zero coverity problems all the
time.

If you set the current 700 issues you have reported as your baseline,
then it is still practical to run coverity daily on QEMU. Just have
it report only new issues, ignoring the backlog, and ensure those all
get fixes posted the same day so the backlog doesn't grow. Deal with
the historical backlog of issues separately as time allows.

Also I'd suggest making "no new coverity issues" be a release blocker
item so people see you are taking it seriously and so be encouraged
to help out to ensure the release doesn't slip.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Markus Armbruster]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Wed, Jan 21, 2015 at 01:47:22PM +0100, Markus Armbruster wrote:
>> We're using the Coverity Scan service[*].  We've put in some effort, and
>> we've gotten some mileage out of it, but I feel we could get more.
>> 
>> Judging from the report e-mail I have lying about, we're scanning about
>> once a month on average.  These reports cuts off after 20 new defects.
>> When there are more, which is common, people have to go to the web
>> dashboard to see them.  When I get one with ten, I may have a look, when
>> I get one "Showing 20 of 100 defect(s)", I despair of the task, and put
>> it off.
>> 
>> 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.
>> 
>> 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.
>> 
>> 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 agree that you need to scan much more regularly. Given the number of
> patches QEMU merges, with only monthly scans you're creating a big job
> for whoever has to deal with the monthly report because chances are
> there will be alot of new stuff reported each month to wade through.
>
> In libvirt we now have a coverity scan being run once a day, so when
> we get new problems reported, the code in question is still fresh in
> the mind of the reviewers & patch author. Daily scans also spread out
> the workload much better. Only get a small number of new problems to
> analyse a couple of times a week - never any real huge burden for the
> person managing the coverity scan & more likely to get others to help
> too if there's only a couple of things for them to look at instead of
> a list of 700+ to wade through. I think these contribute to make it
> practical for us to keep libvirt at zero coverity problems all the
> time.

That's a pretty comfy place to be with Coverity.

> If you set the current 700 issues you have reported as your baseline,
> then it is still practical to run coverity daily on QEMU. Just have
> it report only new issues, ignoring the backlog, and ensure those all
> get fixes posted the same day so the backlog doesn't grow. Deal with
> the historical backlog of issues separately as time allows.
>
> Also I'd suggest making "no new coverity issues" be a release blocker
> item so people see you are taking it seriously and so be encouraged
> to help out to ensure the release doesn't slip.

I wasn't bold enough to suggest "daily", let alone "release blocker".
I'd be fine with either, but I'd also be fine with baby steps in the
direction.

As Paolo pointed out, a good part of the backlog is in code nobody cares
enough about to actually maintain, yet somebody cares just enough to
protest violently when anyone suggests to ditch it.

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

[Peter Maydell]

On 21 January 2015 at 15:55, Markus Armbruster <armbru@redhat.com> wrote:
> I wasn't bold enough to suggest "daily", let alone "release blocker".

I think the Coverity FAQ says we can't do more than 2
scans a week for a project of QEMU's size anyway...

-- PMM

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

[Paolo Bonzini]

On 21/01/2015 16:59, Peter Maydell wrote:
>> > I wasn't bold enough to suggest "daily", let alone "release blocker".
> I think the Coverity FAQ says we can't do more than 2
> scans a week for a project of QEMU's size anyway...

That's just a default.  You can ask them to be an exception, and QEMU is
probably high-profile enough to be granted one.

In fact, my impression is that they hardly have a reason not to grant
one, they just want to have sane defaults that let them scale easily and
protect against DOS.

Paolo

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

[Paolo Bonzini]

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:

1) your model is not tailored well to QEMU;

2) you are not weeding out false positives.

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

> 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, 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
> 

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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> 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

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

[Paolo Bonzini]

On 21/01/2015 15:57, Markus Armbruster wrote:
>> 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?

Yes.  It models g_malloc0 in a way that avoids a lot of false positives,
but still is able to flag leaks.

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

Yup.  So the numbers are off by a couple hundred or so, assuming 20%
false positive rate.

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

Definitely.  But we've gotten much more than "some mileage" IMO.

>>> 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 :)

Sure.  But even then, MISSING_BREAK is not the #1 reason to have
Coverity around. :)

Paolo

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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 21/01/2015 15:57, Markus Armbruster wrote:
>>> 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?
>
> Yes.  It models g_malloc0 in a way that avoids a lot of false positives,
> but still is able to flag leaks.

Understood.  It's hugely better than nothing, but it still can't make
Coverity see many relevant facts like GLib functions returning newly
allocated storage.  For that, you have to throw in a derived model, like
I do.  Unfortunately, the Scan service seems unable to do that.

>>> 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.
>
> Yup.  So the numbers are off by a couple hundred or so, assuming 20%
> false positive rate.

Yes.  I should've made that clear.

>>> 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."
>
> Definitely.  But we've gotten much more than "some mileage" IMO.

Conceded.

>>>> 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 :)
>
> Sure.  But even then, MISSING_BREAK is not the #1 reason to have
> Coverity around. :)

MISSING_BREAK is almost always a false positive.  But when it isn't, it
can be anything from harmless bug to security hole.

Regardless, I agree many other Coverity checkers are more valuable.

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

[Paolo Bonzini]

On 21/01/2015 17:05, Markus Armbruster wrote:
>>> >> What do you mean by "a GLib model"?  scripts/coverity-model.c?
>> >
>> > Yes.  It models g_malloc0 in a way that avoids a lot of false positives,
>> > but still is able to flag leaks.
> Understood.  It's hugely better than nothing,

Yes, I think between false positives and false negatives it affected
over 100 defects.

> but it still can't make
> Coverity see many relevant facts like GLib functions returning newly
> allocated storage.  For that, you have to throw in a derived model, like
> I do.  Unfortunately, the Scan service seems unable to do that.

Right.  You can also model the functions manually, like we do for
g_io_channel_unix_new:

typedef struct _GIOChannel GIOChannel;
GIOChannel *g_io_channel_unix_new(int fd)
{
    GIOChannel *c = g_malloc0(sizeof(GIOChannel));
    __coverity_escape__(fd);
    return c;
}

(This was done because of false positives when Coverity thought that fd
would leak at end of scope).

If you know some offenders which did cause us to leak memory in the
past, please do submit a patch to scripts/coverity-model.c.

BTW, thanks for starting this thread.  We already have like 6 new users
who will be able to see the defects and fix them!  That alone is very
much worthy!

Paolo

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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 21/01/2015 17:05, Markus Armbruster wrote:
>>>> >> What do you mean by "a GLib model"?  scripts/coverity-model.c?
>>> >
>>> > Yes.  It models g_malloc0 in a way that avoids a lot of false positives,
>>> > but still is able to flag leaks.
>> Understood.  It's hugely better than nothing,
>
> Yes, I think between false positives and false negatives it affected
> over 100 defects.

Yes.  RESOURCE_LEAK was pretty useless without it.

>> but it still can't make
>> Coverity see many relevant facts like GLib functions returning newly
>> allocated storage.  For that, you have to throw in a derived model, like
>> I do.  Unfortunately, the Scan service seems unable to do that.
>
> Right.  You can also model the functions manually, like we do for
> g_io_channel_unix_new:
>
> typedef struct _GIOChannel GIOChannel;
> GIOChannel *g_io_channel_unix_new(int fd)
> {
>     GIOChannel *c = g_malloc0(sizeof(GIOChannel));
>     __coverity_escape__(fd);
>     return c;
> }
>
> (This was done because of false positives when Coverity thought that fd
> would leak at end of scope).
>
> If you know some offenders which did cause us to leak memory in the
> past, please do submit a patch to scripts/coverity-model.c.

Comparing two local scans, one with and one without my derived model, I
think I can spot some gaps we could fill in coverity-model.c.  Stay
tuned...

> BTW, thanks for starting this thread.  We already have like 6 new users
> who will be able to see the defects and fix them!  That alone is very
> much worthy!

:)

I hope regular scanning reports to qemu-devel will get us even more.