[Fb-contrib-commit] fb-contrib/src/com/mebigfatguy/fbcontrib/detect BloatedSynchronizedBlock.java,1.4,1.5

[Dave B. <dbr...@us...>]

Update of /cvsroot/fb-contrib/fb-contrib/src/com/mebigfatguy/fbcontrib/detect
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv1245/src/com/mebigfatguy/fbcontrib/detect

Modified Files:
	BloatedSynchronizedBlock.java 
Log Message:
keep track of local variables that are aliases to the results of methods calls on this. Consider all such loads on these variables as unsafe.

Index: BloatedSynchronizedBlock.java
===================================================================
RCS file: /cvsroot/fb-contrib/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/BloatedSynchronizedBlock.java,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -d -r1.4 -r1.5
--- BloatedSynchronizedBlock.java	2 Jan 2006 14:57:11 -0000	1.4
+++ BloatedSynchronizedBlock.java	2 Jan 2006 16:44:01 -0000	1.5
@@ -18,7 +18,11 @@
  */
 package com.mebigfatguy.fbcontrib.detect;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.apache.bcel.classfile.Method;
+import org.apache.bcel.generic.Type;
 
 import edu.umd.cs.findbugs.BugInstance;
 import edu.umd.cs.findbugs.BugReporter;
@@ -31,11 +35,12 @@
 	private BugReporter bugReporter;
 	private static final String BSB_MIN_SAFE_CODE_SIZE = "fb-contrib.bsb.minsize";
 	private OpcodeStack stack = new OpcodeStack();
+	private Set<Integer> unsafeAliases = new HashSet<Integer>();
 	private int syncPC;
 	private int lastPC;
-	private int monitorReg;
 	private boolean isStatic;
 	private int minSafeCodeLength;	
+	private boolean thisCallOccurred;
 	
     /**
      * constructs a BSB detector given the reporter to report bugs on
@@ -68,8 +73,9 @@
 		else
 			syncPC = -1;
 		lastPC = -1;
-		monitorReg = -1;
 		isStatic = obj.isStatic();
+		unsafeAliases.clear();
+		thisCallOccurred = false;
 		stack.resetForMethodEntry(this);
 	}
 	
@@ -81,12 +87,39 @@
 	 */
 	public void sawOpcode(int seen) {
 		try {
+			if (thisCallOccurred && ((seen == ASTORE) || ((seen >= ASTORE_0) && (seen <= ASTORE_3)))) {
+				int storeReg = astoreReg(seen);
+				if (storeReg >= 0)
+					unsafeAliases.add(new Integer(storeReg));
+			}
+			
+			if ((seen == INVOKEVIRTUAL)
+			||  (seen == INVOKESPECIAL)
+			||  (seen == INVOKEINTERFACE)) {
+				String methodSig = getSigConstantOperand();
+				Type returnType = Type.getReturnType(methodSig);
+				if (!(returnType.equals(Type.VOID)) {
+					int parmCount = Type.getArgumentTypes(methodSig).length;
+					if (stack.getStackDepth() > parmCount) {
+						OpcodeStack.Item itm = stack.getStackItem(parmCount);
+						thisCallOccurred = (itm.getRegisterNumber() == 0);
+					} else
+						thisCallOccurred = false;
+				} else
+					thisCallOccurred = false;
+			}
+			else
+				thisCallOccurred = false;
+			
 			if (seen == MONITORENTER) {
 				if (syncPC < 0) {
 					syncPC = getPC();
 					if (stack.getStackDepth() > 0) {
 						OpcodeStack.Item itm = stack.getStackItem(0);
-						monitorReg = itm.getRegisterNumber();
+						int monitorReg = itm.getRegisterNumber();
+						if (monitorReg >= 0) {
+							unsafeAliases.add(new Integer(monitorReg));
+						}
 					}
 				}
 			}
@@ -95,7 +128,8 @@
 			else if (syncPC >= 0) {
 				boolean unsafe = ((seen == PUTFIELD) || (seen == GETFIELD));
 				unsafe |= (!isStatic) && ((seen == ALOAD_0) || (seen == ASTORE_0));
-				unsafe |= (monitorReg >= 0) && (aloadReg(seen) == monitorReg);
+				int aloadReg = aloadReg(seen);
+				unsafe |= (aloadReg >= 0) && unsafeAliases.contains(new Integer(aloadReg));
 				if (unsafe) {
 					if ((getPC() - syncPC) > minSafeCodeLength) {
 						bugReporter.reportBug(new BugInstance(this, "BSB_BLOATED_SYNCHRONIZED_BLOCK", NORMAL_PRIORITY)
@@ -126,4 +160,19 @@
 			return seen - ALOAD_0;
 		return -1;
 	}
+	
+	/** 
+	 * return the register number used in this ASTORE method, if it is an astore instruction
+	 * 
+	 * @param seen the currently visited opcode
+	 * 
+	 * @return the register used if it's an astore instruction, or -1 otherwise
+	 */
+	public int astoreReg(int seen) {
+		if (seen == ASTORE)
+			return getRegisterOperand();
+		if ((seen >= ASTORE_0) && (seen <= ASTORE_3))
+			return seen - ASTORE_0;
+		return -1;
+	}
 }