At Clever Cloud, we like giving a try to new technologies when they're released, especially if it means safer software. When facebook recently announced the release of infer, we surely wanted to try it out!
The concept of static analysis
The idea behind infer is quite simple: it analyses your code before you actually run it and looks for common code issues or misconceptions which can lead to unsafe code.
You can think of it as some kind of compiler plugin which will do some stricter checks and produce more warnings than your compiler usually does.
The idea of static analysis is really great as you get to know some of your code's problems before even trying to run it. An issue with most static analyzers though is the false positives, those warnings you know shouldn't be there as you perfectly know that this problem is 100% impossible to happen.
Testing our code with infer
We tend to use a lot of different technologies to run our platform. From the languages supported by infer, Java was the most interesting to us. I decided to run a first pass on all of our Java projects.
After getting the code on github and following the installation instructions in INSTALL.md, I started with one of our core small pieces of software.
I didn't expect to see many defects but was afraid of getting a ton of false positives when I ran the test: infer -- mvn clean package. Turned out we got a couple of legitimate warnings about resource leaks when spawning a process and then waiting for it but not destroying it once done, and nothing more! No false positives at all.
For the record, those warnings looked like that:
error: RESOURCE_LEAK resource acquired by call to exec(...) at line 89 is not released after line 89
Fix was as easy as changing return Runtime.getRuntime().exec(cmd).waitFor(); to
Process p = Runtime.getRuntime().exec(cmd);
int res = p.waitFor();
p.destroy();
return res;
As the beginning was encouraging, I went on and ran it on one of our core libraries. One similar problem found, one potential null dereference in an error path and another resource leak warning. The first two were trivial to fix, the last one was a weird one as in some cases it didn't detect a stream to be closed. After refactoring this piece of code which was quite old, the warning was gone, but what appeared to me as being a false positive frightened me when it came to running it on our big API.
The relevant annoying warning: error: RESOURCE_LEAK resource acquired by call to FileReaderHelper(...) at line 28 is not released after line 30
FileReaderHelper makes use of a BufferedReader; changing the scope of the inside reader fixed the issue.
Most of our other projects are really small and no defects were detected at all; then came our API. Our API is the oldest piece of software we're running. As time goes by, we externalise the codebase into separate projects but it's still quite a huge one with its 415 files.
Good surprise: only 20 defects, 18 of them in the same error path which would mean that our platform is unavailable anyways. Amongst the two others, one was clearly a false positive. I added an assertion to silence the warning with a comment explaining why it was there. But the warning wouldn't go. I've since then opened an issue for this which hopefully will be fixed soon.
An example of NULL pointer dereference for which we added checks because "better safe than sorry" when it really should never happen: error: NULL_DEREFERENCE object returned by token.getUserId() could be null and is dereferenced at line 86
Conclusions
Only two false positives in all of our codebase, and a couple of really pertinent warnings makes this project really promising. It doesn't throw a lot of warnings yet (or we didn't hit much) but it reduces the noise by giving only relevant ones and avoiding false positives, which is a really good thing for a static analyser.
With an integration into our CI, it'll now provide yet more guaranties about code safety.
Go and try it for yourself!