We’ve posted previously on some of the differences between Findbugs’ open source Java analysis and commercial Java static analysis tools, specifically on the JSR-305 specification and source code versus byte code analysis topics. Due to these differences, many Java shops will use a commercial Java static analysis tool in conjunction with Findbugs to make sure they’re getting as complete issue detection as possible.
One area that’s been discussed previously is the ability to identify situations of possible null pointer dereference. This peaked my interest and led me to do some benchmarking against a few open source projects to assess the Findbugs analysis on intra- and inter-procedural possible null pointer dereference issues.
The normal assumption is that Findbugs is strong with intra-procedural analysis but unable to provide inter-procedural Java analysis. Here is an inter-procedural example from an open source project called hsqldb:
protected void directRefreshTree() {
int[] rowCounts;
...
try {
rowCounts = getRowCounts(tables, schemas);
} catch (Exception e) {
// Added: (weconsultants)@users
CommonSwing.errorMessage(e);
}
ResultSet col;
// For each table, build a tree node with interesting info
for (int i = 0; i < tables.size(); i++) {
col = null;
String name;
try {
name = (String) tables.elementAt(i);
if (isOracle && name.startsWith("BIN$")) {
continue;
// Oracle Recyle Bin tables.
// Contains metacharacters which screw up metadata
// queries below.
}
schema = (String) schemas.elementAt(i);
String schemaname = "";
if (schema != null && showSchemas) {
schemaname = schema + '.';
}
String rowcount = displayRowCounts
? (" " + DECFMT.format(rowCounts[i]))
: "";
...
}In the first try block getRowCounts() can have a null value. How you ask?
protected int[] getRowCounts(Vector inTable,
Vector inSchema) throws Exception {
if (!displayRowCounts) {
return (null);
}
...
}Okay, so automatically finding these issues is not going to happen with Findbugs. I understand that, and that is why there are commercial tools to help with that.
But what really surprised me are the intra-procedural examples that are missed. Again using examples from hsqldb and a specific class called TransferDb, Findbugs finds one issue. But there are clearly more intra-procedural issues that you would think Findbugs would have found. Let’s take a look:
TransferResultSet getData(String statement)
throws DataAccessPointException {
ResultSet rsData = null;
try {
if (srcStatement != null) {
srcStatement.close();
}
srcStatement = conn.createStatement();
rsData = srcStatement.executeQuery(statement);
} catch (SQLException e) {
try {
srcStatement.close();
} catch (Exception e1) {}
srcStatement = null;
rsData = null;
throw new DataAccessPointException(e.getMessage());
}
return new TransferResultSet(rsData);
}This is a conditional situation where “srcStatement” could have a null value (the conditional is false), then you throw an exception with “createStatement()” and dereference later with “srcStatement.close()”. Why does Findbugs miss this issue? There are several issues of this type.
Or another situation where you throw an exception with “createStatement()” again. This example spans multiple lines so you will find it in an attachment. Here “select_rs” has a value of null and is clearly dereferenced much later on.
…
ResultSet col = null;
int colnum = 1;
Statement stmt = null;
Source: null here -> ResultSet select_rs = null;
ResultSetMetaData select_rsmdata = null;
try {
Exception throw here -> stmt = conn.createStatement();
select_rs = stmt.executeQuery(TTable.Stmts.sSourceSelect);
select_rsmdata = select_rs.getMetaData();
col = meta.getColumns(TTable.Stmts.sDatabaseToConvert,
TTable.Stmts.sSchema,
TTable.Stmts.sSourceTable, null);
} catch (SQLException eSchema) {
// fredt - second try with null schema
if (TTable.Stmts.sSchema.equals("")) {
try {
col = meta.getColumns(TTable.Stmts.sDatabaseToConvert,
null, TTable.Stmts.sSourceTable,
null);
} catch (SQLException eSchema1) {}
}
}
try {
while (col.next()) {
String name = Dest.helper.formatIdentifier(col.getString(4));
int type = col.getShort(5);
String source = col.getString(6);
int column_size = col.getInt(7);
String DefaultVal = col.getString(13);
boolean rsmdata_NoNulls =
(select_rsmdata.isNullable(colnum)
== java.sql.DatabaseMetaData.columnNoNulls);
boolean rsmdata_isAutoIncrement = false;
try {
rsmdata_isAutoIncrement =
select_rsmdata.isAutoIncrement(colnum);
} catch (SQLException e) {
rsmdata_isAutoIncrement = false;
}
int rsmdata_precision = select_rsmdata.getPrecision(colnum);
int rsmdata_scale = select_rsmdata.getScale(colnum);
type = helper.convertFromType(type);
type = Dest.helper.convertToType(type);
Integer inttype = new Integer(type);
String datatype = (String) TTable.hTypes.get(inttype);
if (datatype == null) {
datatype = source;
tracer.trace("No mapping for type: " + name + " type: "
+ type + " source: " + source);
}
if (type == Types.NUMERIC) {
datatype += "(" + Integer.toString(rsmdata_precision);
if (rsmdata_scale > 0) {
datatype += "," + Integer.toString(rsmdata_scale);
}
datatype += ")";
} else if (type == Types.CHAR) {
datatype += "(" + Integer.toString(column_size) + ")";
} else if (rsmdata_isAutoIncrement) {
datatype = "SERIAL";
}
if (DefaultVal != null) {
if (type == Types.CHAR || type == Types.VARCHAR
|| type == Types.LONGVARCHAR
|| type == Types.BINARY || type == Types.DATE
|| type == Types.TIME
|| type == Types.TIMESTAMP) {
DefaultVal = "\'" + DefaultVal + "\'";
}
datatype += " DEFAULT " + DefaultVal;
}
if (rsmdata_NoNulls) {
datatype += " NOT NULL ";
}
v.addElement(inttype);
datatype = helper.fixupColumnDefRead(TTable, select_rsmdata,
datatype, col, colnum);
datatype = Dest.helper.fixupColumnDefWrite(TTable,
select_rsmdata, datatype, col, colnum);
create += name + " " + datatype + ",";
insert += "?,";
colnum++;
}
Sink: dereference here-> select_rs.close();
stmt.close();
col.close();
} catch (SQLException e) {
throw new DataAccessPointException(e.getMessage());
}So I’m a little confused as to why these issues are not highlighted. It seems that this is the exact type of analysis Findbugs does. Maybe it has to do with not recognizing the possible exceptions? Does anyone have any ideas?
