Designing Bug Detection Rules for Fewer False Alarms

Short Paper [PDF]

FeeFin Patterns and Detection Rules

CompareSameValue

CompareSameValue compares the same values from a getter and a field.

The example always returns true as the getter, getVersion, actually returns the field, VERISON.
  
  public static final byte VERSION = 1;
  public void readFields(DataInput in) throws IOException {
   byte version = in.readByte();
   ...
- } else if (getVersion() == VERSION) {
+ } else if (getVersion() == version) {
   ...
  public byte getVersion() {
    return VERSION;
  }

Detection Question and Rule

Detection Rule = (Q1: Yes, Q2: No, Q3: No, Q4: No, Q5: No)

EqualToSameExpression

EqualToSameExpression compares the same expression with ’==’.

The example always returns true and a redundant part is removed in a fix.
  
- if(replicaInfo.getStamp() == replicaInfo.getStamp()) {
+ if(block.getStamp() == replicaInfo.getStamp()) {
  

Detection Question and Rule

Detection Rule = (Q1: Yes, Q2: No, Q3: No)

IllogicalCondition

IllogicalCondition loads a known null object in conditional expressions that can cause a null pointer exception (NPE) because of an illogical condition.

In the example, when the left operand of ‘||’ in the deleted line is false, prefix is null and will cause an NPE in the right operand. The ‘||’ is replaced into ‘&&’ in the added line of the patch for avoiding NPE when calling prefix.value().
  
- if(prefix != null || prefix.value()!=null)
+ if(prefix != null && prefix.value()!=null)
  

Detection Question and Rule

Detection Rule = (Q1: Yes, Q2: Yes)

IncorrectDirectoySlash

IncorrectDirectorySlash causes an inconsistent path by an optional slash.

In the example, depending on whether args[1] is ended with a slash or not, mDir could have an inconsistent path. This is fixed by calling getAbsolutePath() that always returns a path not ending with a slash.
  
- File mDir = new File(args[1] + "-tmp");
+ File mDir = new File(args[1]);
+ mDir = new File(mDir.getAbsolutePath() + "-tmp");
  

Detection Question and Rule

Detection Rule = (Q1: Yes)

IncorrectMapIterator

IncorrectMapIterator is a wrong iteration of a map by values instead of an entry set.

In the example, developers incorrectly used values of a map to iterate the map.
  
  Map m = this.getMap();
- for(Iterator i=m.values().iterator();i.hasNext();){
+ for (Iterator i=m.entrySet().iterator();i.hasNext();){ Map.Entry e = (Map.Entry) i.next();
  String uri = (String) e.getKey();
  

Detection Question and Rule

Detection Rule = (Q1: Yes, Q2: Yes)

MissingLForLong

MissingLForLong causes an integer overflow while defining a long integer with the multiplication of actual values.

The example shows its fix by adding a suffix, L, for a long type on the largest value.
  
- final long DEFAULT_MAX_FILE_SIZE = 10*1024*1024*1024;
+ final long DEFAULT_MAX_FILE_SIZE = 10*1024*1024*1024L;
  

Detection Question and Rule

Detection Rule = (Q1: Yes)

RedundantException

RedundantException handles the same exception in both a try-catch-finally block and a method declaration with ‘throws’. This may miss intended catch or finally blocks.

In the example, the finally block is not executed when there is an IOException from initializing os or is before the try block as the method copy() throws the same exception. This bug leads to memory leak and is fixed by moving the statements causing IOException into the try block.
    
     public void copy() throws IOException {
-    IndexOutput os = createOutput(dest);
-    IndexInput is = openInput(src);
+    IndexOutput os = null;
+    IndexInput is = null;
     try {
+      os = createOutput(dest);
+      is = openInput(src);
       ...
     } catch (IOException ioe) {
       ...
     } finally {
       IOUtils.closeSafely(pExp, os, is);
  

Detection Question and Rule

Detection Rule = (Q1: Yes, Q2: Yes, Q3: Yes, Q4: Yes, Q5: No, Q6: No )

RedundantInstantiation

RedundantInstantiation initializes an object redundantly that can cause a performance issue.

The example shows redundant object instantiations of conn, one of which is removed in a patch.
    
- Connection conn = getConnection();
- conn = new Connection(url,user);
+ Connection conn = new Connection(url,user);
  

Detection Question and Rule

Detection Rule = (Q1: Yes)

SameObjEquals

SameObjEquals compares the same object, method, variable, etc., with equals().

The comparison in the example always returns true and a redundant part is removed in a fix.
  
- return other.getUUID().equals(other.getUUID());
+ return getUUID().equals(other.getUUID());
  

Detection Question and Rule

Detection Rule = (Q1: Yes, Q2: Yes, Q3: No, Q4: No)

WrongIncrementer

WrongIncrementer is the misuse of an outer incrementer in an inner loop.

In the example, developers incorrectly used the incrementer, i. If the sizes of a and b are not the same, there will be an index-out-of-bounds exception.
    
  for(int i=0; i < a.length; i++){
    for(int j=0; j < b.length; j++){
-     a[i] = b[i];
+     a[i] = b[j];
  

Detection Question and Rule

Detection Rule = (Q1: Yes, Q2: No, Q3: No, Q4: No)