Static Analysis

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Static Analysis

Paul Ramsey
Hey Devs,

Are we interested in receiving static analysis reports (Coverity) on
the PostGIS code base?

The folks at CrunchyData are willing to stick-handle the bureaucracy
around getting Coverity account for the project and a system set up to
regularly pass the PostGIS code base through Coverity static analysis.
Coverity provides free (as in beer) accounts for open source projects,
so the actual Coverity "account" would be the PostGIS project's and
the PSC would control it.

Anyways, other than providing an annoying list of things we should do
(gah!) I see no downside to having some more information on our code
cleanliness/security. Unlike the transifex stuff, there'd be no
dependencies on a foreign system, since if Coverity ever shut off our
access we'd be no worse off than we are right now.

Thoughts?

P
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

Regina Obe
+1

-----Original Message-----
From: postgis-devel [mailto:[hidden email]] On Behalf Of Paul Ramsey
Sent: Thursday, May 05, 2016 2:07 PM
To: PostGIS Development Discussion <[hidden email]>
Subject: [postgis-devel] Static Analysis

Hey Devs,

Are we interested in receiving static analysis reports (Coverity) on the PostGIS code base?

The folks at CrunchyData are willing to stick-handle the bureaucracy around getting Coverity account for the project and a system set up to regularly pass the PostGIS code base through Coverity static analysis.
Coverity provides free (as in beer) accounts for open source projects, so the actual Coverity "account" would be the PostGIS project's and the PSC would control it.

Anyways, other than providing an annoying list of things we should do
(gah!) I see no downside to having some more information on our code cleanliness/security. Unlike the transifex stuff, there'd be no dependencies on a foreign system, since if Coverity ever shut off our access we'd be no worse off than we are right now.

Thoughts?

P
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel


_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

Bborie Park-2
In reply to this post by Paul Ramsey
+1 I'd love to have a run through...

On Thu, May 5, 2016 at 11:06 AM, Paul Ramsey <[hidden email]> wrote:
Hey Devs,

Are we interested in receiving static analysis reports (Coverity) on
the PostGIS code base?

The folks at CrunchyData are willing to stick-handle the bureaucracy
around getting Coverity account for the project and a system set up to
regularly pass the PostGIS code base through Coverity static analysis.
Coverity provides free (as in beer) accounts for open source projects,
so the actual Coverity "account" would be the PostGIS project's and
the PSC would control it.

Anyways, other than providing an annoying list of things we should do
(gah!) I see no downside to having some more information on our code
cleanliness/security. Unlike the transifex stuff, there'd be no
dependencies on a foreign system, since if Coverity ever shut off our
access we'd be no worse off than we are right now.

Thoughts?

P
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel


_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

Sandro Santilli-2
In reply to this post by Paul Ramsey
On May 5, 2016 8:06:42 PM CEST, Paul Ramsey <[hidden email]> wrote:

>Hey Devs,
>
>Are we interested in receiving static analysis reports (Coverity) on
>the PostGIS code base?
>
>The folks at CrunchyData are willing to stick-handle the bureaucracy
>around getting Coverity account for the project and a system set up to
>regularly pass the PostGIS code base through Coverity static analysis.
>Coverity provides free (as in beer) accounts for open source projects,
>so the actual Coverity "account" would be the PostGIS project's and
>the PSC would control it.
>
>Anyways, other than providing an annoying list of things we should do
>(gah!) I see no downside to having some more information on our code
>cleanliness/security. Unlike the transifex stuff, there'd be no
>dependencies on a foreign system, since if Coverity ever shut off our
>access we'd be no worse off than we are right now.
>
>Thoughts?
>
>P
>_______________________________________________
>postgis-devel mailing list
>[hidden email]
>http://lists.osgeo.org/mailman/listinfo/postgis-devel

+1
--strk;

Sent from device with lame keyboard. Please excuse my brevity.
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

Mark Cave-Ayland
In reply to this post by Paul Ramsey
On 05/05/16 19:06, Paul Ramsey wrote:

> Hey Devs,
>
> Are we interested in receiving static analysis reports (Coverity) on
> the PostGIS code base?
>
> The folks at CrunchyData are willing to stick-handle the bureaucracy
> around getting Coverity account for the project and a system set up to
> regularly pass the PostGIS code base through Coverity static analysis.
> Coverity provides free (as in beer) accounts for open source projects,
> so the actual Coverity "account" would be the PostGIS project's and
> the PSC would control it.
>
> Anyways, other than providing an annoying list of things we should do
> (gah!) I see no downside to having some more information on our code
> cleanliness/security. Unlike the transifex stuff, there'd be no
> dependencies on a foreign system, since if Coverity ever shut off our
> access we'd be no worse off than we are right now.
>
> Thoughts?

Definitely! I suspect that there will be a number of things to check
after the first scan which may take some time. Note that Coverity does
produce false positives, so perhaps some thought should be given to as
to whether we should aim for a "clean" scan to the point where we can
add annotations to the code for cases like these.


ATB,

Mark.

_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

Even Rouault-2
Le vendredi 06 mai 2016 10:55:50, Mark Cave-Ayland a écrit :

> On 05/05/16 19:06, Paul Ramsey wrote:
> > Hey Devs,
> >
> > Are we interested in receiving static analysis reports (Coverity) on
> > the PostGIS code base?
> >
> > The folks at CrunchyData are willing to stick-handle the bureaucracy
> > around getting Coverity account for the project and a system set up to
> > regularly pass the PostGIS code base through Coverity static analysis.
> > Coverity provides free (as in beer) accounts for open source projects,
> > so the actual Coverity "account" would be the PostGIS project's and
> > the PSC would control it.
> >
> > Anyways, other than providing an annoying list of things we should do
> > (gah!) I see no downside to having some more information on our code
> > cleanliness/security. Unlike the transifex stuff, there'd be no
> > dependencies on a foreign system, since if Coverity ever shut off our
> > access we'd be no worse off than we are right now.
> >
> > Thoughts?
>
> Definitely! I suspect that there will be a number of things to check
> after the first scan which may take some time. Note that Coverity does
> produce false positives, so perhaps some thought should be given to as
> to whether we should aim for a "clean" scan to the point where we can
> add annotations to the code for cases like these.

We have gone through the Coverity route for GDAL. Dealing with the results of
the first scan is likely to be painful (took months to clear the > 1300 initial
stock). There are some false positives, but not that much (10% ?). In general
you get false positives in portion of the code where the logic is convoluted
and you can get rid of most of them by simplifying the code a bit, or adding
extra conditions, and making it more obvious, including for humans.
For real false positives, you can add the following annotation before the line
/* coverity[name_of_the_violated_rule] */

The largest occurence in GDAL code base is /* coverity[tainted_data] */ , that
is due to using the value of an environment variable with no or little
checking. Likely not going to affect PostGIS.

We've also used Clang Static Analyzer. To be honest, it is far less powerful
than Coverity and has a high rate of false positives, and reworking the code
to make it happy can be really hard sometimes. It did uncover a couple of
things other tools didn't find though but the rate time invested to investigate
reports / bug founds is prettly low. If you want to give it a try, I'd
recommend only using it after cleaning all Coverity warnings, so as to limit
the number of things to look at.

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

nicklas

Interesting things. I want to dp my share of the clean up work.
I guess I will learn a lot from that.

Will most of the issues be isolated to source files, or special functions or will it be more about keeping things conseqent over the whole code base?

/Nicklas

Skickat från min Sony Xperia™-smartphone



---- Even Rouault skrev ----

Le vendredi 06 mai 2016 10:55:50, Mark Cave-Ayland a écrit :

> On 05/05/16 19:06, Paul Ramsey wrote:
> > Hey Devs,
> >
> > Are we interested in receiving static analysis reports (Coverity) on
> > the PostGIS code base?
> >
> > The folks at CrunchyData are willing to stick-handle the bureaucracy
> > around getting Coverity account for the project and a system set up to
> > regularly pass the PostGIS code base through Coverity static analysis.
> > Coverity provides free (as in beer) accounts for open source projects,
> > so the actual Coverity "account" would be the PostGIS project's and
> > the PSC would control it.
> >
> > Anyways, other than providing an annoying list of things we should do
> > (gah!) I see no downside to having some more information on our code
> > cleanliness/security. Unlike the transifex stuff, there'd be no
> > dependencies on a foreign system, since if Coverity ever shut off our
> > access we'd be no worse off than we are right now.
> >
> > Thoughts?
>
> Definitely! I suspect that there will be a number of things to check
> after the first scan which may take some time. Note that Coverity does
> produce false positives, so perhaps some thought should be given to as
> to whether we should aim for a "clean" scan to the point where we can
> add annotations to the code for cases like these.

We have gone through the Coverity route for GDAL. Dealing with the results of
the first scan is likely to be painful (took months to clear the > 1300 initial
stock). There are some false positives, but not that much (10% ?). In general
you get false positives in portion of the code where the logic is convoluted
and you can get rid of most of them by simplifying the code a bit, or adding
extra conditions, and making it more obvious, including for humans.
For real false positives, you can add the following annotation before the line
/* coverity[name_of_the_violated_rule] */

The largest occurence in GDAL code base is /* coverity[tainted_data] */ , that
is due to using the value of an environment variable with no or little
checking. Likely not going to affect PostGIS.

We've also used Clang Static Analyzer. To be honest, it is far less powerful
than Coverity and has a high rate of false positives, and reworking the code
to make it happy can be really hard sometimes. It did uncover a couple of
things other tools didn't find though but the rate time invested to investigate
reports / bug founds is prettly low. If you want to give it a try, I'd
recommend only using it after cleaning all Coverity warnings, so as to limit
the number of things to look at.

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel


_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

Paul Ramsey
OK, thanks all, I've asked Crunchy to go ahead and put the wheels in motion.
Dreading that first report,
P

On Fri, May 6, 2016 at 2:33 AM, Nicklas Aven <[hidden email]> wrote:

> Interesting things. I want to dp my share of the clean up work.
> I guess I will learn a lot from that.
>
> Will most of the issues be isolated to source files, or special functions or
> will it be more about keeping things conseqent over the whole code base?
>
> /Nicklas
>
> Skickat från min Sony Xperia™-smartphone
>
>
>
> ---- Even Rouault skrev ----
>
>
> Le vendredi 06 mai 2016 10:55:50, Mark Cave-Ayland a écrit :
>> On 05/05/16 19:06, Paul Ramsey wrote:
>> > Hey Devs,
>> >
>> > Are we interested in receiving static analysis reports (Coverity) on
>> > the PostGIS code base?
>> >
>> > The folks at CrunchyData are willing to stick-handle the bureaucracy
>> > around getting Coverity account for the project and a system set up to
>> > regularly pass the PostGIS code base through Coverity static analysis.
>> > Coverity provides free (as in beer) accounts for open source projects,
>> > so the actual Coverity "account" would be the PostGIS project's and
>> > the PSC would control it.
>> >
>> > Anyways, other than providing an annoying list of things we should do
>> > (gah!) I see no downside to having some more information on our code
>> > cleanliness/security. Unlike the transifex stuff, there'd be no
>> > dependencies on a foreign system, since if Coverity ever shut off our
>> > access we'd be no worse off than we are right now.
>> >
>> > Thoughts?
>>
>> Definitely! I suspect that there will be a number of things to check
>> after the first scan which may take some time. Note that Coverity does
>> produce false positives, so perhaps some thought should be given to as
>> to whether we should aim for a "clean" scan to the point where we can
>> add annotations to the code for cases like these.
>
> We have gone through the Coverity route for GDAL. Dealing with the results
> of
> the first scan is likely to be painful (took months to clear the > 1300
> initial
> stock). There are some false positives, but not that much (10% ?). In
> general
> you get false positives in portion of the code where the logic is convoluted
> and you can get rid of most of them by simplifying the code a bit, or adding
> extra conditions, and making it more obvious, including for humans.
> For real false positives, you can add the following annotation before the
> line
> /* coverity[name_of_the_violated_rule] */
>
> The largest occurence in GDAL code base is /* coverity[tainted_data] */ ,
> that
> is due to using the value of an environment variable with no or little
> checking. Likely not going to affect PostGIS.
>
> We've also used Clang Static Analyzer. To be honest, it is far less powerful
> than Coverity and has a high rate of false positives, and reworking the code
> to make it happy can be really hard sometimes. It did uncover a couple of
> things other tools didn't find though but the rate time invested to
> investigate
> reports / bug founds is prettly low. If you want to give it a try, I'd
> recommend only using it after cleaning all Coverity warnings, so as to limit
> the number of things to look at.
>
> --
> Spatialys - Geospatial professional services
> http://www.spatialys.com
> _______________________________________________
> postgis-devel mailing list
> [hidden email]
> http://lists.osgeo.org/mailman/listinfo/postgis-devel
>
>
> _______________________________________________
> postgis-devel mailing list
> [hidden email]
> http://lists.osgeo.org/mailman/listinfo/postgis-devel
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis

Paul Norman
In reply to this post by Paul Ramsey
On 5/5/2016 11:06 AM, Paul Ramsey wrote:

> Hey Devs,
>
> Are we interested in receiving static analysis reports (Coverity) on
> the PostGIS code base?
>
> The folks at CrunchyData are willing to stick-handle the bureaucracy
> around getting Coverity account for the project and a system set up to
> regularly pass the PostGIS code base through Coverity static analysis.
> Coverity provides free (as in beer) accounts for open source projects,
> so the actual Coverity "account" would be the PostGIS project's and
> the PSC would control it.
>
> Anyways, other than providing an annoying list of things we should do
> (gah!) I see no downside to having some more information on our code
> cleanliness/security. Unlike the transifex stuff, there'd be no
> dependencies on a foreign system, since if Coverity ever shut off our
> access we'd be no worse off than we are right now.

We've used Coverity with osm2pgsql, both when it was a C project and
with the new C++ code. Once you get it set up properly, there's a
reasonably low false positive rate. I suspect PostGIS will require a
modeling file (https://scan.coverity.com/tune) for some stuff
implemented in PostgreSQL.

I found Clang's static analysis more useful, but less comprehensive.
Another effort that was done at about the same time was to compile with
-Wall -Werror to enforce having zero warnings, and that caught a lot of
problems that I think Coverity would have also spotted.
_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel