Hello, everyone! My name is Stas, I am an engineer of DevOps Tooling team at Align Technology. In this article, I'm going to talk briefly about our company's experience of integrating static code analysis, based on PVS-Studio analyzer, into the development process.
A year ago we considered adopting static analysis in our company. We had used a number of static analysis tools before to analyze our products, including C/C++ projects, so we thought it would be interesting to try a new tool to address a familiar task. We were looking for something better than the integrated analyzer in VS, Cppcheck, or the one that comes with Sonar.
As it always happens with undertakings like this, it started with an action group (consisting of one developer whose initiative was generally supported by the manager). He came to our team and said he would like to give it a try. We contacted PVS-Studio developers, and they suggested that we use the free demo version of the tool. Our developer ran it on his code and made a presentation for his colleagues, demonstrating examples of bugs that PVS-Studio can detect. Moreover, he showed that serious errors can also be found in relatively fresh code. The presentation was a great success; everyone was amused by the mistakes found in their colleagues' and their own code. The camp of enthusiasts grew larger and, what's especially nice, attracted the managers as well.
After that the managers (together with the enthusiastic developers) calculated the ROI of adopting the product. How exactly the calculations were done is a subject for another article, which will probably appear later. Anyway, we purchased PVS-Studio, and our team was given the task to integrate static analysis and the bug-fixing mechanism into the product development process.
This is how we did it:
The process of eliminating detected issues advances in an abrupt fashion and is driven mainly by enthusiasts. Every now and then, some of them will find time to eliminate bugs of a particular type. It's better that way because warnings can be usually eliminated without changing the code logic, so, suspicious fragments can be easily refactored. Besides, a large portion of warnings can be eliminated using one or several more or less standard techniques, thus eliminating one warning pattern throughout the project at a time is more efficient than, say, dealing with all the diverse warnings in one file.
To make it easier for our enthusiasts, we integrated PVS-Studio into the CI process. We use Atlassian Bamboo as a CI server, so I will be using the terms accepted in this tool when describing the integration process. We created special build plans scheduled for automatic launches - one plan per project.
Through the command-line interface, a build plan initiates an analysis of the main project in the standalone mode for the current code version. The analysis process takes quite a while (2-4 hours); that's why we run it not sooner than once a day. After the analysis is finished, PVS-Studio generates a plog-file, which is in fact an xml file with a list of found issues. This file goes through some processing (namely, absolute paths are replaced with relative ones) and then is converted, through xslt-conversion, into an xUnit-report, which Bamboo can understand and visualize. Out of every warning type, we make one unit test with a list of all the files this warning was triggered by. After the first launch, all the warnings are sent to quarantine (i.e. builds are considered to be successful when the tests are failed).
Once a certain warning pattern gets fixed, the developer releases the corresponding test from the quarantine. Then, if a warning we considered to be fixed in the whole project is suddenly triggered again, we get a failed build and take measures.
This is what a freshly generated warning looks like:
xUnit typically has a three-level hierarchy: TestSuite as a union of TestCases; each TestCase contains from 0 (for a passed test) to many Failures. Analysis results are translated in the following way.
When integrating PVS-Studio analyzer into our CI, we faced with some complications.
Firstly, there were troubles with the license. We had purchased a site-license for 30 users. According to the licensing policy, one installation on one particular operating system "takes away" one license. Since we have lots of builds and projects, there are lots of build agents too - we are now approaching the limit of 100 agents (there are about 90 of them currently). Not all of them are used for building C++ code, of course, but we do have a lot of those. It was impossible to have PVS-Studio installed on all of them, so we had to assign a few agents for that job. As a result, if these assigned agents are busy at the time the analysis process starts, it has to wait until they are free and then occupies them for half of the day. It leads to health-check builds, which start after every commit, constantly being short of agents. Moreover, it hinders automatic deployment of the agents (we constantly have to keep count of currently used licenses, which is not an easy task without a license server).
Secondly, there are troubles with getting access to the documentation for diagnostics. The only place you can get them is the documentation section at the PVS-Studio developers' website. However, we haven't managed to implement an automatic parser for that page yet as the page makeup is too awkward for correct parsing and changes a bit every time. In addition, it's somewhat difficult to implement automatic centralized update of PVS-Studio and the documentation on all the agents (it can be dealt with, of course, but we haven't got down to it yet). So, we update manually for now.
Here are a few examples of bugs found by the analyzer, i.e. when the process worked successfully with new code.
V544. It is odd that the value 'X' of HRESULT type is compared with 'Y'
static HRESULT findSomething(const X& x);
// later in the code
if (findSomething(x) == -1)Minor issue: style
V501: There are identical sub-expressions to the left and to the right of the 'foo' operator
assert(leftPoints.size() == leftPoints.size());Minor issue: incorrect assert — no production impact
if (upper && upper)
return something;Major issue: bug
template<class T>
void operator () ( const char* name, const T& t1, const T& t2 )
{
if( t1 != t1 )
{
diff = true;
}
}Major issue: bug
V674: The expression contains a suspicious mix of integer and real types.
double precision = getSettingInt("Model", "Precision", 1e-6);Major issue: bug
V655: The strings were concatenated but are not utilized. Consider inspecting the expression.
switch (someEnum)
{
case someCondition:
err + " is null ";
break;V596: The object was created but it is not being used. The 'throw' keyword could be missing.
if (open(...))
{
std::runtime_error("Can not open database");
}Minor issue: low-probability bug
V557: Array overrun is possible.
float Jx[3];
...
for (int i=0; i<4; i++){
for (int j=i; j<4; j++){
value += Jx[i]*Jx[j];
}
}We'd like to point out that the PVS-Studio package now includes PlogConverter utility. This tool comes as a set of source files and allows you to convert .plog into .html, .txt, .csv, or any other format. With its help, you can quickly convert the plog-file into your favorite format. For the PlogConverter description, see the corresponding documentation section.
The original article was published at habrahabr.ru, in the blog of Align Technology company. Align Technology Inc is one of the world's leading developers of innovative medical devices, based in the Silicon Valley. The company manufactures and successfully sells a unique product - a device called Invisalign, which allows patients to achieve a perfect smile in a simple and aesthetically accepted way (without wearing traditional dental braces).