We Checked the Android Source Code by PVS-Studio, or Nothing is Perfect

[Andrey Karpov]

Development of large complex projects is impossible without the use of programming techniques and tools helping to monitor the quality of the code. First, it requires a literate coding standard, code reviews, unit tests, static and dynamic code analyzers. All this helps to detect defects in code at the earliest stages of development. This article demonstrates the abilities of a PVS-Studio static analyzer in detecting bugs and security weaknesses in code of the Android operating system. We hope that the article will attract readers' attention to the methodology of static code analysis, and they will be willing to embed it in the process of developing their own projects.

Article: https://www.viva64.com/en/b/0579/

[Rick C. Hodgin]

On 8/1/2018 1:17 PM, Andrey Karpov wrote:
> Development of large complex projects is impossible without the use of programming techniques and tools helping to monitor the quality of the code. First, it requires a literate coding standard, code reviews, unit tests, static and dynamic code analyzers. All this helps to detect defects in code at the earliest stages of development. This article demonstrates the abilities of a PVS-Studio static analyzer in detecting bugs and security weaknesses in code of the Android operating system. We hope that the article will attract readers' attention to the methodology of static code analysis, and they will be willing to embed it in the process of developing their own projects.
>
> Article: https://www.viva64.com/en/b/0579/
>

I was very critical of your coming to comp.lang.c++ initially because
I viewed it as little more than an attempt to make money off your
product. However, since that time I've downloaded the trial version
and tried it out on my own software and was pleased beyond words with
how effective it is.

I give your product full props, would recommend it to everyone, and
would use it if I had the funds to do so.

My only complaint remains: PVS-Studio is too expensive for small
open-source project developers like me to easily obtain. And, there
are many of us on many projects. Our software is also important to
be bug free, but your decision to exclude us by your high starting
price is a barrier, and is the negative aspect of your product...
This is all in my personal opinion.

--
Rick C. Hodgin

[Paavo Helde]

On 1.08.2018 20:17, Andrey Karpov wrote:
> Development of large complex projects is impossible without the use of programming techniques and tools helping to monitor the quality of the code. First, it requires a literate coding standard, code reviews, unit tests, static and dynamic code analyzers. All this helps to detect defects in code at the earliest stages of development. This article demonstrates the abilities of a PVS-Studio static analyzer in detecting bugs and security weaknesses in code of the Android operating system. We hope that the article will attract readers' attention to the methodology of static code analysis, and they will be willing to embed it in the process of developing their own projects.
>
> Article: https://www.viva64.com/en/b/0579/

In the article I see you admit there are a lot of false positives, and
yet you claim there is nothing wrong with that and the software is OK.
In practice, the false positives are a huge deterrent for a user, if 99%
of diagnostics are false positives then the tool basically becomes
unusable (been there, done that, spending a full day analyzing
avalanches of diagnostics from a multithread-safety checker tool, only
to eventually figure out they were 100% false positives because the tool
didn't recognize boost::mutex as a mutex).

Quoting the article:

#if GENERIC_TARGET
const char alternative_config_path[] = "/data/nfc/";
#else
const char alternative_config_path[] = "";
#endif

CNxpNfcConfig& CNxpNfcConfig::GetInstance() {
....
if (alternative_config_path[0] != '\0') {
....
}
Here the analyzer issues a warning: V547 CWE-570 Expression
'alternative_config_path[0] != '\0'' is always false. phNxpConfig.cpp 401

The issue is that the GENERIC_TARGET macro is not defined
[...]
Unfortunately, nothing can be done with such situations.

[/end quote]

Of course something can be done. The analyzer can take into account the
presence of both preprocessor branches and see that in one of them the
check makes sense, so the warning is not warranted. Maybe it's not so
simple and maybe it would require a major redesign of the analyzer, but
definitely it can be done.

[David Brown]

It is certainly an interesting product, and one that we may consider at
my company. I don't like the restriction that the normal business
license is for either Windows /or/ Linux - we use both, and would prefer
a mixed license. But it may still be worth it.

And the articles that Andrey links to are sometimes good to read too.

However, even though the product is relevant to C++, the posts are spam
- they are unsolicited commercial adverts in a non-commercial newsgroup.
It would be a slightly different matter if Andrey were a regular
contributor here and took part in discussions about his product and
other C++ topics.

[Ian Collins]

It would be interesting seeing how it compares to other static analysis
tools for the same code. We are increasing our use of clang-tidy,
having used CppCheck for a couple of years. The latter is fast (at
least on Linux) but somewhat lacking in its analysis, the former slower
but way more thorough.

--
Ian.

[Richard]

[Please do not mail me a copy of your followup]

Paavo Helde <myfir...@osa.pri.ee> spake the secret code
<pjt74s$qhc$1...@dont-email.me> thusly:

>Quoting the article:
>
>#if GENERIC_TARGET
>const char alternative_config_path[] = "/data/nfc/";
>#else
>const char alternative_config_path[] = "";
>#endif
>
>CNxpNfcConfig& CNxpNfcConfig::GetInstance() {
> ....
> if (alternative_config_path[0] != '\0') {
> ....
>}
>Here the analyzer issues a warning: V547 CWE-570 Expression
>'alternative_config_path[0] != '\0'' is always false. phNxpConfig.cpp 401
>
>The issue is that the GENERIC_TARGET macro is not defined
>[...]
>Unfortunately, nothing can be done with such situations.
>
>[/end quote]
>
>Of course something can be done. The analyzer can take into account the
>presence of both preprocessor branches and see that in one of them the
>check makes sense, so the warning is not warranted. Maybe it's not so
>simple and maybe it would require a major redesign of the analyzer, but
>definitely it can be done.

If PVS Studio isn't yet supporting compile_commands.json for recording
this sort of thing (or scraping it out of the VS solution/project),
then that is a feature that is sorely needed.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
The Terminals Wiki <http://terminals-wiki.org>
The Computer Graphics Museum <http://computergraphicsmuseum.org>
Legalize Adulthood! (my blog) <http://legalizeadulthood.wordpress.com>

[David Brown]

CppCheck is fast enough that it can be integrated in the IDE, rather
than run as a separate pass. I haven't used clang-tidy much as yet, but
plan to do so.

My understanding (I haven't tried it at all) of PVS-Studio is that it is
a much more in-depth analysis and covers many things that I have not
heard of in other tools, such as its "copy paste" error detection.

[Jorgen Grahn]

On Wed, 2018-08-01, Paavo Helde wrote:
> On 1.08.2018 20:17, Andrey Karpov wrote:

...

>> Article: https://www.viva64.com/en/b/0579/
>
> In the article I see you admit there are a lot of false positives, and
> yet you claim there is nothing wrong with that and the software is OK.
> In practice, the false positives are a huge deterrent for a user, if 99%
> of diagnostics are false positives then the tool basically becomes
> unusable

Not if you can mark and track the false positives so you only have to
deal with them once. Coverity Prevent does that (via a web interface
which I found frustrating to use, some years ago).

I haven't looked into Andrey's product.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .

[Andrey Karpov]

> I was very critical of your coming to comp.lang.c++ initially because
> I viewed it as little more than an attempt to make money off your
> product. However, since that time I've downloaded the trial version
> and tried it out on my own software and was pleased beyond words with
> how effective it is.
>
> I give your product full props, would recommend it to everyone, and
> would use it if I had the funds to do so.
>
> My only complaint remains: PVS-Studio is too expensive for small
> open-source project developers like me to easily obtain. And, there
> are many of us on many projects. Our software is also important to
> be bug free, but your decision to exclude us by your high starting
> price is a barrier, and is the negative aspect of your product...
> This is all in my personal opinion.

Thank you very much. I am glad that you liked our tool. I’d like to draw your attention to the article "How to use PVS-Studio for Free" - https://www.viva64.com/en/b/0457/ . Perhaps, the described option of free licensing will be suitable for your tasks.

[Andrey Karpov]

> In the article I see you admit there are a lot of false positives, and
> yet you claim there is nothing wrong with that and the software is OK.
> In practice, the false positives are a huge deterrent for a user, if 99%
> of diagnostics are false positives then the tool basically becomes
> unusable (been there, done that, spending a full day analyzing
> avalanches of diagnostics from a multithread-safety checker tool, only
> to eventually figure out they were 100% false positives because the tool
> didn't recognize boost::mutex as a mutex).

I understand what you are saying. We are paying much attention to fighting against false positives: https://www.viva64.com/en/b/0488/

I’m saying that it's not crucial for two reasons. Firstly, I know that PVS-Studio can be configured and you can get very good results. I have the article on this topic: Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives - https://www.viva64.com/en/b/0523/

Secondly, PVS-Studio provides a mass warnings suppression mode with a help of a special base. This allows you to quickly integrate the tool into the development process and fix warnings, related only to the new or changed code. And you can fix technical debt (old bugs) during free time (which is always not enough :). Read more: Mass Suppression of Analyzer Messages (disable generation of analyzer messages for legacy code) - https://www.viva64.com/en/m/0032/

Generally speaking, the question about false positives is not that scary in case of using PVS-Studio. We have experience in getting to zero all false warnings and fixes of all real errors. An example of such work: https://www.unrealengine.com/en-US/blog/how-pvs-studio-team-improved-unreal-engines-code . This is a big, but very doable work.

[Andrey Karpov]

> It is certainly an interesting product, and one that we may consider at
> my company. I don't like the restriction that the normal business
> license is for either Windows /or/ Linux - we use both, and would prefer
> a mixed license. But it may still be worth it.
>
> And the articles that Andrey links to are sometimes good to read too.
>
> However, even though the product is relevant to C++, the posts are spam
> - they are unsolicited commercial adverts in a non-commercial newsgroup.
> It would be a slightly different matter if Andrey were a regular
> contributor here and took part in discussions about his product and
> other C++ topics.

As for licensing, please, write to our support. I think we can find a compromise solution.

I agree that such articles might look not very pretty. But I always try to write materials that will be useful and will educate programmers how to avoid many kinds of errors.

[Andrey Karpov]

> It would be interesting seeing how it compares to other static analysis
> tools for the same code. We are increasing our use of clang-tidy,
> having used CppCheck for a couple of years. The latter is fast (at
> least on Linux) but somewhat lacking in its analysis, the former slower
> but way more thorough.

I am no longer writing such comparisons articles:) . This is a very large and ungrateful work. Even though I’m writing such articles very seriously, there is always someone who says "it’s not true, you’ve deliberately picked projects to beat everyone else". And it is not clear how to object:).

Well, the comparison issue isn’t quite simple. I described my ideas about it in detail in the article "Why I Dislike Synthetic Tests": https://www.viva64.com/en/b/0471/

As for Clang, perhaps, you’ll be interested in these articles: https://www.viva64.com/en/b/0108/ , https://www.viva64.com/en/b/0155/ , https://www.viva64.com/en/b/0446/

[Rick C. Hodgin]

When I contacted your company previously (sup...@viva64.com), I was
responded to by someone named Evgeniy Ryzhkov (April 10, 2018).

I wrote this content:

-----[ Begin #1 ]-----
>Greetings.
>
>I just wanted to write and convey how thoroughly impressed I am with
>PVS-Studio. I downloaded the free trial and installed it. It found a half
>dozen legitimate bugs in my code, and over 50 cases where there's redundancy
>or an underflow using memcmp() or other such issues.
>
>I would love to use this tool in my organization. The price is outside of
>my range however, and I would like to request a version for a lower price,
>one that lets users only use the tool for so many minutes per day, for
>example, or so many minutes per week. Something like that.
>
>By creating this less expensive tool you would serve the needs of many more
>developers with smaller budgets. This would address many for-free open
>source projects, widening your user base and increasing sales to a portion
>of a potential target customer base. The full version would be sold as is
>and used as it is today for use at any time. The smaller version would
>suite the new customers.
>
>Please consider creating this lesser version for smaller organizations.
>Something around $240 per year, with $120 per year renewals. It would be
>a great benefit, and greatly appreciated.
>
>Thank you for making such a powerful tool.
>
>--
>Rick C. Hodgin
-----[ End #1 ]-----

He did not point me to the page you do. He responded with:

-----[ Begin #2 ]-----
>Hi Rick,
>
>Thanks for detailed feedback!
>
>Please read this post about our experience with low price project:
>https://www.viva64.com/en/b/0320/
-----[ End #2 ]-----

That page explains why you don't have a free version. I did not see
a link to the free version.

I replied with:

-----[ Begin #3 ]-----
>Your post said it was almost profitable, to the break even point.
>
>I did not know about your product. Others didn't either. There was still
>room for growth.
>
>In addition, I am not thinking another separate tool is required. Only
>PVS-Studio, but just a hobbled version for the lower amount. As I say,
>only so many minutes or analyzes per day or week, or even year.
>
>One product, multiple markets (you have one at $60/month, one at $30/month,
>and I'm asking for one at $20/month that is a much linited system). Call
>it PVS-Studio Light or something. Same code base, just some imposed
>limitations.
>
>You would benefit so many more developers and users. It would be to your
>credit. I urge you to give it some real consideration.
>
>--
>Rick C. Hodgin
-----[ End #3 ]-----

He replied with:

-----[ Begin #4 ]-----
>Thanks for your opinion.
>
>Evgeniy Ryzhkov
>OOO Program Verification Systems (Co Ltd)
-----[ End #4 ]-----

To be blunt:

I have held your company in a negative regard since that time because of
that exchange, seeing that you had no interest in helping out small or open
source projects. I thought your product was great, but the fact that you
were only having paid versions was inappropriate as you were pursuing money
ahead of helping developers, which is the wrong way to be.

I see now you do have a free version for open source projects. It would
have been nice to know that four months ago during this exchange with
Evgeniy.

I would like to suggest teaching your staff some additional steps toward
assisting potential customers, or toward those making an effort to help
you increase the widespread use of your product. I think your tool is
amazing and most C/C++ projects would benefit from it. I think your price
tag is too high, which is why I wrote what I did above, so that you could
sell to more people for less money for a lesser version.

In addition, I apologize for my harsh assessment of your company. I
based my conclusion on what information I had, and I see now it was the
wrong conclusion. I am sorry, and I apologize.

--
Rick C. Hodgin

[David Brown]

On 02/08/18 16:39, Andrey Karpov wrote:
>> It is certainly an interesting product, and one that we may
>> consider at my company. I don't like the restriction that the
>> normal business license is for either Windows /or/ Linux - we use
>> both, and would prefer a mixed license. But it may still be worth
>> it.
>>
>> And the articles that Andrey links to are sometimes good to read
>> too.
>>
>> However, even though the product is relevant to C++, the posts are
>> spam - they are unsolicited commercial adverts in a non-commercial
>> newsgroup. It would be a slightly different matter if Andrey were a
>> regular contributor here and took part in discussions about his
>> product and other C++ topics.
>
> As for licensing, please, write to our support. I think we can find
> a compromise solution.

Certainly I would do so when the time comes. First, I have to have a
closer look at it and see how it would work for us before bothering your
commercial people :-) But it is nice to hear that they might be flexible.

Our work is mostly with embedded systems, and mostly C at the moment
(with C++ growing). What sort of experience do you have with using your
tool in non-native environments for embedded devices?

This also means that there are a variety of IDE's involved - several
specialised Eclipse versions, some specialised MSVS-based IDEs, and a
few others. Pure command-line "make" is also vital, and the compilation
could be running on Windows desktops, Linux desktops, or (in the future)
Linux-based build servers.

>
> I agree that such articles might look not very pretty. But I always
> try to write materials that will be useful and will educate
> programmers how to avoid many kinds of errors.
>

I have no beef with the articles themselves - I have read a number, and
found things of interest in them. Obviously there will be a certain
amount of bias in them - you have a product to promote and want to show
it in the best light - but on the whole I have seen them as informative
and educational.

However, the fact remains that you are selling a product and the links
you post in this group are, in effect, adverts for a commercial product.
The fear of any discussion group is that once some sorts of commercial
posts are accepted, you are on a slippery slope and the group ends up
drowning in adverts.

This is an unmoderated group, so no one can give rules here or "ban" you
from posting, but your articles are in a grey area between information
and spam. If you were more involved in the group (and your replies now
help tremendously - you are clearly not a "hit and run" advertiser) then
in my mind at least, it would make your posts a more clearly positive
contribution. Obviously you are a knowledgeable and experienced C++
developer, and would be welcome in the group.

[Rick C. Hodgin]

I have one further apology to make. In searching further for the email
exchanges with Evgeniy, I came across this one dated February 2, 2017 (a
year earlier):

-----[ Begin #5 ]-----
>I am a single person working on multiple projects at the following url:
>https://github.com/RickCHodgin/libsf
>
>They are all public domain projects. I am interested in obtaining a
>temporary license to evaluate your product. And I would like to know
>what cost your product would be to someone like me, who is not selling
>software, but is creating software for anyone to download and use for
>free.
>
>Thank you,
>Rick C. Hodgin
-----[ End #5 ]-----

Evgeniy responded with:

-----[ Begin #6 ]-----
>Hi Rick,
>
>I think that you can use free license: http://www.viva64.com/en/b/0457/

>
>Evgeniy Ryzhkov
>OOO “Program Verification Systems” (Co Ltd)

-----[ End #6 ]-----

I did not reply to that email, and I'm not sure if I ever got it. If
I did, I either misread it or had forgotten about it since that time.

The full range of errors and misunderstandings here may have been on my
end, and if so I apologize greatly. It may have been all me, and I
feel very bad about that possibility.

I'm sorry for any harm or negativity I've caused you, your staff, or
your company.

--
Rick C. Hodgin

[Andrey Karpov]

> Our work is mostly with embedded systems, and mostly C at the moment
> (with C++ growing). What sort of experience do you have with using your
> tool in non-native environments for embedded devices?
>
> This also means that there are a variety of IDE's involved - several
> specialised Eclipse versions, some specialised MSVS-based IDEs, and a
> few others. Pure command-line "make" is also vital, and the compilation
> could be running on Windows desktops, Linux desktops, or (in the future)
> Linux-based build servers.

Yes, starting from the version 6.22, the PVS-Stidio analyzer supports a number of compilers for embedded systems. For example, here is the article about a check of the embedded operating system RT-Thread: https://www.viva64.com/en/b/0561/ by PVS-Studio.

Try to check your embedded projects. In any case, if something goes wrong, contact us and we will help you.

Now a few words about environments and running modes. The analyzer can be used very differently. That’s why I suggest to get acquainted with the documentation: https://www.viva64.com/en/m/ . I hope you will find in it the answers to many questions. For example, perhaps you the format full html report will be of interest for you: https://www.viva64.com/en/b/0539/

As for more specific and detailed questions, again you can contact our support: https://www.viva64.com/en/about-feedback/

[Andrey Karpov]

Hello Rick,
Great! I'm glad we came to mutual understanding :).

[David Brown]

On 03/08/18 10:41, Andrey Karpov wrote:
>> Our work is mostly with embedded systems, and mostly C at the
>> moment (with C++ growing). What sort of experience do you have
>> with using your tool in non-native environments for embedded
>> devices?
>>
>> This also means that there are a variety of IDE's involved -
>> several specialised Eclipse versions, some specialised MSVS-based
>> IDEs, and a few others. Pure command-line "make" is also vital,
>> and the compilation could be running on Windows desktops, Linux
>> desktops, or (in the future) Linux-based build servers.
>
> Yes, starting from the version 6.22, the PVS-Stidio analyzer supports
> a number of compilers for embedded systems. For example, here is the
> article about a check of the embedded operating system RT-Thread:
> https://www.viva64.com/en/b/0561/ by PVS-Studio.
>

Thank you for that link. I haven't read it yet, but I will certainly do
so. While IAR and Keil are popular compilers, far and away the most
popular embedded compiler is gcc. Of course you will have plenty of
support for normal native gcc, but embedded gcc versions may have extra
challenges. (I have not yet tried your software, so I can't say if this
is an issue or not.)

On a related note, does your tool support MISRA rule checking? I hate
MISRA - it is full of pointless and sometimes directly harmful rules -
but occasionally clients insist on it. Open source compilers can't give
MISRA warnings because the MISRA standards must be bought and licensed,
but closed-source compilers such as IAR and Keil handle them. If your
tool supports MISRA (and perhaps other common coding standards, such as
JSF), it might be an additional selling point.

> Try to check your embedded projects. In any case, if something goes
> wrong, contact us and we will help you.
>
> Now a few words about environments and running modes. The analyzer
> can be used very differently. That’s why I suggest to get acquainted
> with the documentation: https://www.viva64.com/en/m/ . I hope you
> will find in it the answers to many questions. For example, perhaps
> you the format full html report will be of interest for you:
> https://www.viva64.com/en/b/0539/
>
> As for more specific and detailed questions, again you can contact
> our support: https://www.viva64.com/en/about-feedback/
>

Thank you for your suggestions and information.

[Chris M. Thomasson]

On 8/1/2018 10:24 AM, Rick C. Hodgin wrote:
> On 8/1/2018 1:17 PM, Andrey Karpov wrote:
>> Development of large complex projects is impossible without the use of
>> programming techniques and tools helping to monitor the quality of the
>> code. First, it requires a literate coding standard, code reviews,
>> unit tests, static and dynamic code analyzers. All this helps to
>> detect defects in code at the earliest stages of development. This
>> article demonstrates the abilities of a PVS-Studio static analyzer in
>> detecting bugs and security weaknesses in code of the Android
>> operating system. We hope that the article will attract readers'
>> attention to the methodology of static code analysis, and they will be
>> willing to embed it in the process of developing their own projects.
>>
>> Article: https://www.viva64.com/en/b/0579/
>>
>
> I was very critical of your coming to comp.lang.c++ initially because
> I viewed it as little more than an attempt to make money off your
> product.  However, since that time I've downloaded the trial version
> and tried it out on my own software and was pleased beyond words with
> how effective it is.

Still have not tried it, but will now. Thanks Rick.

[Andrey Karpov]

> Thank you for that link. I haven't read it yet, but I will certainly do
> so. While IAR and Keil are popular compilers, far and away the most
> popular embedded compiler is gcc. Of course you will have plenty of
> support for normal native gcc, but embedded gcc versions may have extra
> challenges. (I have not yet tried your software, so I can't say if this
> is an issue or not.)
>
> On a related note, does your tool support MISRA rule checking? I hate
> MISRA - it is full of pointless and sometimes directly harmful rules -
> but occasionally clients insist on it. Open source compilers can't give
> MISRA warnings because the MISRA standards must be bought and licensed,
> but closed-source compilers such as IAR and Keil handle them. If your
> tool supports MISRA (and perhaps other common coding standards, such as
> JSF), it might be an additional selling point.

We support some types of gcc. Perhaps, if you try, everything will work and you'll be able to check your project. If something goes wrong, write to us, we'll give you advice or fine-tune the analyzer.

We also don't like this standard, but there is nothing to do...:) Right now we're working on MISRA...i.g. our analyzer will first support MISRA C and then MISRA C++. I don't know exactly about the dates, we've just started.
However, the PVS-Studio analyzer already detects some errors, described in MISRA and we'll only need to match the identifiers of our rules with the identifiers of the MISRA rules.