How To Find Bugs, Part 1: A Minimal Bug Detector

LMAX Exchange

Findbugs is an incredibly powerful tool, and it supports running of custom detectors. However, the API for writing custom detectors is not well documented, at least as far as I’ve been able to find. So, as I started writing detectors, I’ve been working primarily off a process of trial and error. It’s likely there are better ways of doing things: what follows, however, at least works.

Let’s start off with something easy: a detector which labels invocations of a method as bugs. That’s about as simple as a detector can get, so it’s a good case to demonstrate the wiring required to get the thing working.

Step 1 is, of course, a failing test case. Here it is:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class Cat
{
    public static void main(String... args) throws IOException
    {
        Files.lines(Paths.get(args[0])).forEach(System.out::println);
    }
}

This implements a minimal version of the Unix cat utility, piping the contents of its argument to stdout, line by line. It’s a failing case because when we run Findbugs on it, it doesn’t identify our bug (an invocation of Files.lines()).

In order to watch it fail, we’ll need Findbugs: http://findbugs.sourceforge.net/downloads.html
Unzip the latest version: we’ll be using the command-line analyzer here for simplicity. Compile Cat.java, and then run Findbugs on it:

:>findbugs-3.0.1/bin/fb analyze -low ~/sampleCode/Cat.class

That gives me no errors. Let’s just verify that this is capable of finding bugs: modify Cat.java to the following:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class Cat
{
    public static void main(String... args) throws IOException
    {
        boolean ignore = ("Hello" == args[0]);
        Files.lines(Paths.get(args[0])).forEach(System.out::println);
    }
}

and run Findbugs again:

:> findbugs-3.0.1/bin/fb analyze -low ~/sampleCode/Cat.class 
L B ES: Comparison of String objects using == or != in Cat.main(String[])   At Cat.java:[line 9]
Warnings generated: 1

So far so good. We have Findbugs capable of finding bugs, but not finding the bug we want it to find. This is hardly surprising, as we haven’t written the code to find that bug yet.

The detector is fairly straightforward:

import java.nio.file.Files;

import edu.umd.cs.findbugs.BugInstance;
import edu.umd.cs.findbugs.BugReporter;
import edu.umd.cs.findbugs.BytecodeScanningDetector;
import edu.umd.cs.findbugs.classfile.ClassDescriptor;
import edu.umd.cs.findbugs.classfile.DescriptorFactory;
import edu.umd.cs.findbugs.classfile.MethodDescriptor;

public class FilesLinesDetector extends BytecodeScanningDetector
{
    private static final ClassDescriptor JAVA_NIO_FILES =
        DescriptorFactory.createClassDescriptor(Files.class);
    final BugReporter bugReporter;

    public FilesLinesDetector(final BugReporter bugReporter)
    {
        this.bugReporter = bugReporter;
    }
    
    @Override
    public void sawMethod()
    {
        final MethodDescriptor invokedMethod 
            = getMethodDescriptorOperand();
        final ClassDescriptor invokedObject 
            = getClassDescriptorOperand();
        if(invokedMethod != null && 
           "lines".equals(invokedMethod.getName()) && 
           JAVA_NIO_FILES.equals(invokedObject))
        {
            bugReporter.reportBug(
                 new BugInstance(this, 
                                 "FILES_LINES_CALLED", 
                                 HIGH_PRIORITY)
                     .addClassAndMethod(this)
                     .addSourceLine(this)
            );
        }
    }
}

Of course, you’ll need findbugs/libs/findbugs.jar on your classpath to compile this.

But we need more than just the detector: we need to build a Findbugs plugin. That consists of a number of detectors, and (of course) some XML configuration files. One called findbugs.xml, which defines what detectors to run:

<?xml version="1.0" encoding="UTF-8"?>
<FindbugsPlugin pluginid="com.lmax.testing.findbugs.detectors"
 defaultenabled="true"
 provider="LMAX">

 <Detector class="FilesLinesDetector" reports="FILES_LINES_CALLED"/>
 <BugPattern abbrev="LMAX" type="FILES_LINES_CALLED" category="CORRECTNESS"/>
</FindbugsPlugin>
    

And one called messages.xml, which handles feedback to your users:

<?xml version="1.0" encoding="UTF-8"?>
<MessageCollection>

  <BugCode abbrev="LMAX">LMAX</BugCode>

  <Plugin>
    <ShortDescription>LMAX Custom Findbugs Detectors</ShortDescription>
    <Details>Provides custom detectors for the LMAX codebase.</Details>
  </Plugin>

  <Detector class="FilesLinesDetector">
    <Details>
      Finds invocations of Files.lines()
    </Details>
  </Detector>

  <BugPattern type="FILES_LINES_CALLED">
    <ShortDescription>Files.lines() called</ShortDescription>
    <LongDescription>Don't call Files.lines(): it leaks files. Use MyFiles.lines() instead</LongDescription>
    <Details>
      <![CDATA[ <p>Don't call Files.lines(): it leaks files. Use MyFiles.lines() instead</p> ]]>
    </Details>
  </BugPattern>
</MessageCollection>

Jar up the xml files, along with the class file for the detector, and that’s a Findbugs plugin. Add it to your Findbugs command line arguments, and presto:

:> findbugs-3.0.1/bin/fb analyze -pluginList ~/detector/plugin.jar -low ~/sampleCode/Cat.class 
H C LMAX: Don't call Files.lines(): it leaks files. Use MyFiles.lines() instead  At Cat.java:[line 9]
Warnings generated: 1