PVS-Studio

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
saso
Veteran
Posts: 1924
Joined: Fri May 16, 2014 1:14 pm
Contact:

PVS-Studio

Post by saso »

PVS-Studio Analyzer - Static Code Analysis for C, C++, C# and Java
https://www.viva64.com/en/pvs-studio/

Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities
https://www.viva64.com/en/b/0592/

Free PVS-Studio for open source projects
https://www.viva64.com/en/b/0600/

Getting Started with the PVS-Studio Static Analyzer for Visual C++
https://www.viva64.com/en/b/0642/

Analysis of commits and pull requests in Travis CI, Buddy and AppVeyor using PVS-Studio
https://www.viva64.com/en/b/0679/

Static Analysis: From Getting Started to Integration
https://www.viva64.com/en/b/0753/

How to Get Nice Error Reports Using SARIF
https://www.viva64.com/en/b/0798/

How to write more reliable code - Egor Bredikhin - Meeting C++ 2018
https://www.youtube.com/watch?v=KpdSFZOQCl8

PVS-Studio in 2019
https://www.youtube.com/watch?v=FkfMGqxIR-I

Top 10 Bugs Found in C++ Projects in 2020
https://www.viva64.com/en/b/0784/

Intermodular analysis of C++ projects in PVS-Studio
https://pvs-studio.com/en/blog/posts/cpp/0851/

Why we need dynamic code analysis: the example of the PVS-Studio project
https://pvs-studio.com/en/blog/posts/cpp/0868/

Classification of PVS-Studio warnings according to OWASP Top 10 Web Application Security Risks
https://pvs-studio.com/en/pvs-studio/sast/owasptopten/

What's New in PVS-Studio in the Second Half of 2021
https://www.youtube.com/watch?v=dpPa3R3wFds

PVS-Studio: static code analysis technology
https://pvs-studio.com/en/blog/posts/0908/

PVS-Studio evolution: data flow analysis for related variables
https://pvs-studio.com/en/blog/posts/csharp/0942/

PVS-Studio's data flow analysis untangles more and more related variables
https://pvs-studio.com/en/blog/posts/csharp/0976/

Intermodular analysis of C and C++ projects in detail. Part 1
https://pvs-studio.com/en/blog/posts/cpp/0963/

Intermodular analysis of C and C++ projects in detail. Part 2
https://pvs-studio.com/en/blog/posts/cpp/0965/

What static analysis cannot find
https://pvs-studio.com/en/blog/posts/1037/

How static analysis works
https://pvs-studio.com/en/blog/posts/1048/

60 terrible tips for a C++ developer
https://pvs-studio.com/en/blog/posts/cpp/1053/

Common patterns of typos in programming
https://pvs-studio.com/en/blog/posts/cpp/1064/

Simple, yet easy-to-miss errors in code
https://pvs-studio.com/en/blog/posts/cpp/1068/

FreeCAD and undefined behavior in C++ code: meditation for developers
https://pvs-studio.com/en/blog/posts/cpp/1072/

New Year's Eve show: Top 10 errors in C and C++ projects in 2023
https://pvs-studio.com/en/blog/posts/cpp/1092/

Shall we check pointer for NULL before calling free function?
https://pvs-studio.com/en/blog/posts/cpp/1100/

OOP in real-life cases
https://pvs-studio.com/en/blog/posts/java/1103/

Why it is bad idea to check result of malloc call with assert
https://pvs-studio.com/en/blog/posts/cpp/1104/

Parable of null pointer for indolent C programmers
https://pvs-studio.com/en/blog/posts/cpp/1107/

How not to check array size in C++
https://pvs-studio.com/en/blog/posts/cpp/1112/

Static analyzer nudges you to write clean code
https://pvs-studio.com/en/blog/posts/cpp/1115/

A general post about the security and code quality tools https://forum.freecadweb.org/viewtopic. ... 02#p274069
Last edited by saso on Tue Apr 16, 2024 10:29 am, edited 26 times in total.
User avatar
saso
Veteran
Posts: 1924
Joined: Fri May 16, 2014 1:14 pm
Contact:

Re: PVS-Studio

Post by saso »

I have created an analysis of the FreeCAD code (build 0.18.15892) with PVS-Studio. The report is available in an html format to all developers that are interested to have a look at it...

As always, take care of the possible false positives with this reports, some discussions about that can also be found in the below links for the LibreOffice report...

Here are a few links to a similar report that was done for the LibreOffice
https://www.viva64.com/en/b/0586/
https://bugs.documentfoundation.org/sho ... ?id=120703
https://cgit.freedesktop.org/libreoffic ... grep&q=pvs
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: PVS-Studio

Post by wmayer »

In the recent weeks I was working from time to time on many of the reported issues. So far I have fixed a good amount of them and pushed the changes to the repository, some are in a local branch and will be pushed after the v0.18 release and still around 100 are left to be fixed.

At the moment there are only two issues with priority "High". These are:
TaskProjGroup.cpp:405 and TaskProjGroup.cpp:407

Code: Select all


const char * TaskProjGroup::viewChkIndexToCStr(int index)
{
    //   Third Angle:  FTL  T  FTRight
    //                  L   F   Right   Rear
    //                 FBL  B  FBRight
    //
    //   First Angle:  FBRight  B  FBL
    //                  Right   F   L  Rear
    //                 FTRight  T  FTL
    assert (multiView != NULL);

    bool thirdAngle = multiView->usedProjectionType().isValue("Third Angle");
    switch(index) {
        case 0: return (thirdAngle ? "FrontTopLeft" : "FrontBottomRight");
        case 1: return (thirdAngle ? "Top" : "Bottom");
        case 2: return (thirdAngle ? "FrontTopRight" : "FrontBottomLeft");
        case 3: return (thirdAngle ? "Left" : "Right");
        case 4: return (thirdAngle ? "Front" : "Front");
        case 5: return (thirdAngle ? "Right" : "Left");
        case 6: return (thirdAngle ? "Rear" : "Rear");
        case 7: return (thirdAngle ? "FrontBottomLeft" : "FrontTopRight");
        case 8: return (thirdAngle ? "Bottom" : "Top");
        case 9: return (thirdAngle ? "FrontBottomRight" : "FrontTopLeft");
        default: return NULL;
    }
}
PVS is complaining about these two lines:

Code: Select all

case 4: return (thirdAngle ? "Front" : "Front");

Code: Select all

case 6: return (thirdAngle ? "Rear" : "Rear");
wandererfan wrote: ping
Is this on purpose that for the ? : operator in both cases the same value is returned or is it a copy-paste error?
User avatar
wandererfan
Veteran
Posts: 6320
Joined: Tue Nov 06, 2012 5:42 pm
Contact:

Re: PVS-Studio

Post by wandererfan »

wmayer wrote: Sat Mar 09, 2019 12:13 pm PVS is complaining about these two lines:

Code: Select all

case 4: return (thirdAngle ? "Front" : "Front");

Code: Select all

case 6: return (thirdAngle ? "Rear" : "Rear");
wandererfan wrote: ping
Is this on purpose that for the ? : operator in both cases the same value is returned or is it a copy-paste error?
The code is functionally correct.
Front and Rear views occupy the same position in both FirstAngle and ThirdAngle conventions. All the other views move between First and Third.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: PVS-Studio

Post by wmayer »

OK, thanks for the clarification.
User avatar
saso
Veteran
Posts: 1924
Joined: Fri May 16, 2014 1:14 pm
Contact:

Re: PVS-Studio

Post by saso »

Happy to hear that the report was useful... Both thumbs up for fixing the issues this fast! :shock:
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: PVS-Studio

Post by abdullah »

saso wrote: Mon Mar 11, 2019 10:10 pm Happy to hear that the report was useful... Both thumbs up for fixing the issues this fast! :shock:
I have been looking to what Werner has pushed and I was very impressed with the detection capabilities of this tool.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: PVS-Studio

Post by wmayer »

abdullah wrote: Wed Mar 13, 2019 3:19 pm
saso wrote: Mon Mar 11, 2019 10:10 pm Happy to hear that the report was useful... Both thumbs up for fixing the issues this fast! :shock:
I have been looking to what Werner has pushed and I was very impressed with the detection capabilities of this tool.
It found many issues other code checking tools didn't find. However, there are a lot of false-positives and you have to look at all issues very carefully to avoid to make any regressions.

Other detected issues are formal correct but when following the advices it could happen that it will break the code when changing something in the class interface. So, it's best not to touch the relevant code.

So far I presumably have fixed around 30% (maybe more) of all issues.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: PVS-Studio

Post by wmayer »

abdullah wrote: Wed Mar 13, 2019 3:19 pm
saso wrote: Mon Mar 11, 2019 10:10 pm Happy to hear that the report was useful... Both thumbs up for fixing the issues this fast! :shock:
I have been looking to what Werner has pushed and I was very impressed with the detection capabilities of this tool.
At the moment there are left 15 issues I will have a look at. If you have some time then I will appreciate it if you can check the issues in CommandConstraints.cpp:4632, ..., with the description: Possible incorrect order of arguments passed to 'makeTangentToArcOfHyperbolaviaNewPoint' function: 'geom2' and 'geom1'.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: PVS-Studio

Post by abdullah »

wmayer wrote: Wed Mar 13, 2019 3:53 pm
abdullah wrote: Wed Mar 13, 2019 3:19 pm
saso wrote: Mon Mar 11, 2019 10:10 pm Happy to hear that the report was useful... Both thumbs up for fixing the issues this fast! :shock:
I have been looking to what Werner has pushed and I was very impressed with the detection capabilities of this tool.
It found many issues other code checking tools didn't find. However, there are a lot of false-positives and you have to look at all issues very carefully to avoid to make any regressions.

Other detected issues are formal correct but when following the advices it could happen that it will break the code when changing something in the class interface. So, it's best not to touch the relevant code.

So far I presumably have fixed around 30% (maybe more) of all issues.
Thanks for letting me know this. Because you filter the false positives, I only see the good part of the tool.

It is good that I am trying to learn form the good part too ;)
Post Reply