[Fb-contrib-commit] SF.net SVN: fb-contrib: [738] trunk/fb-contrib/src/com/mebigfatguy/ fbcontrib/detect/DeletingWhileIterating.java

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

Revision: 738
          http://svn.sourceforge.net/fb-contrib/?rev=738&view=rev
Author:   dbrosius
Date:     2006-12-18 23:41:37 -0800 (Mon, 18 Dec 2006)

Log Message:
-----------
more fleshing out

Modified Paths:
--------------
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/DeletingWhileIterating.java

Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/DeletingWhileIterating.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/DeletingWhileIterating.java	2006-12-19 06:57:27 UTC (rev 737)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/DeletingWhileIterating.java	2006-12-19 07:41:37 UTC (rev 738)
@@ -32,6 +32,7 @@
 import com.mebigfatguy.fbcontrib.utils.Integer14;
 import com.mebigfatguy.fbcontrib.utils.RegisterUtils;
 
+import edu.umd.cs.findbugs.BugInstance;
 import edu.umd.cs.findbugs.BugReporter;
 import edu.umd.cs.findbugs.BytecodeScanningDetector;
 import edu.umd.cs.findbugs.OpcodeStack;
@@ -61,7 +62,8 @@
 	private BugReporter bugReporter;
 	private OpcodeStack stack;
 	private List<Set<Comparable>> collectionGroups;
-	private Map<Integer, Integer> iteratorToGroup;
+	private Map<Integer, Integer> groupToIterator;
+	private Map<Integer, Loop> loops;
 	
 	/**
      * constructs a DWI detector given the reporter to report bugs on
@@ -72,7 +74,7 @@
 	}
 	
 	/**
-	 * implements the visitor to setup the opcode stack, and collectionGroups
+	 * implements the visitor to setup the opcode stack, collectionGroups, groupToIterator and loops
 	 * 
 	 * @param classContext the context object of the currently parsed class
 	 */
@@ -83,24 +85,27 @@
 		try {	
 			stack = new OpcodeStack();
 			collectionGroups = new ArrayList<Set<Comparable>>();
-			iteratorToGroup = new HashMap<Integer, Integer>();
+			groupToIterator = new HashMap<Integer, Integer>();
+			loops = new HashMap<Integer, Loop>();
 			super.visitClassContext(classContext);
 		} finally {
 			stack = null;
 			collectionGroups = null;
-			iteratorToGroup = null;
+			groupToIterator = null;
+			loops = null;
 		}
 	}
 	
 	/**
-	 * implements the visitor to reset the stack and collectionGroups
+	 * implements the visitor to reset the stack, collectionGroups, groupToIterator and loops
 	 * 
 	 * @param obj the context object of the currently parsed code block
 	 */
 	public void visitCode(Code obj) {
 		stack.resetForMethodEntry(this);
 		collectionGroups.clear();
-		iteratorToGroup.clear();
+		groupToIterator.clear();
+		loops.clear();
 		super.visitCode(obj);
 	}
 	
@@ -116,25 +121,48 @@
 			if (seen == INVOKEINTERFACE)
 			{
 				String className = getClassConstantOperand();
+				String methodName = getNameConstantOperand();
+				String signature = getSigConstantOperand();
+				String methodInfo = methodName + signature;
+				
 				if (isCollection(className)) {
-					String methodName = getNameConstantOperand();
-					String signature = getSigConstantOperand();
-					String methodInfo = methodName + signature;
 					if (collectionMethods.contains(methodInfo)) {
 						if (stack.getStackDepth() > 0) {
 							OpcodeStack.Item itm = stack.getStackItem(0);
-							groupId = findCollectionGroup(itm);
+							groupId = findCollectionGroup(itm, true);
 							
 						}
 					} else if ("iterator()Ljava/util/Iterator;".equals(methodInfo)) {
 						if (stack.getStackDepth() > 0) {
 							OpcodeStack.Item itm = stack.getStackItem(0);
-							groupId = findCollectionGroup(itm);
-							
+							groupId = findCollectionGroup(itm, true);
 						}
 					} else if ("remove(Ljava/lang/Object;)Z".equals(methodInfo)) {
-						
+						if (stack.getStackDepth() > 0) {
+							OpcodeStack.Item itm = stack.getStackItem(0);
+							int id = findCollectionGroup(itm, true);
+							if (id >= 0) {
+								Integer it = groupToIterator.get(id);
+								Loop loop = loops.get(it);
+								if (loop != null) {
+									int pc = getPC();
+									if (loop.hasPC(pc)) {
+										bugReporter.reportBug(new BugInstance(this, "DWI_DELETING_WHILE_ITERATING", NORMAL_PRIORITY)
+													.addClass(this)
+													.addMethod(this)
+													.addSourceLine(this));
+									}
+								}
+							}
+						}
 					}
+				} else if ("java/util/Iterator".equals(className) && "hasNext()Z".equals(methodInfo)) {
+					if (stack.getStackDepth() > 0) {
+						OpcodeStack.Item itm = stack.getStackItem(0);
+						Integer id = (Integer)itm.getUserValue();
+						if (id != null)
+							groupId = id.intValue();
+					}
 				}
 			} else if ((seen == ASTORE) || ((seen >= ASTORE_0) && (seen <= ASTORE_3))) {
 				if (stack.getStackDepth() > 0) {
@@ -145,19 +173,34 @@
 						
 						try {
 							if (itm.getJavaClass().implementationOf(iteratorClass)) {
-								iteratorToGroup.put(Integer14.valueOf(reg), id);
-							} else {
-								Set<Comparable> group = collectionGroups.get(id);
-								if (group != null) {
-									group.add(Integer14.valueOf(reg));
-								}
+								groupToIterator.put(id, Integer14.valueOf(reg));
 							}
+							
+							Set<Comparable> group = collectionGroups.get(id);
+							if (group != null) {
+								group.add(Integer14.valueOf(reg));
+							}
 						} catch (ClassNotFoundException cnfe) {
 							bugReporter.reportMissingClass(cnfe);
 						}
 					}
 				}
-				
+			} else if ((seen == ALOAD) || ((seen >= ALOAD_0) && (seen <= ALOAD_3))) {
+				int reg = RegisterUtils.getALoadReg(this, seen);
+				OpcodeStack.Item itm = new OpcodeStack.Item(new OpcodeStack.Item(), reg);
+				groupId = findCollectionGroup(itm, false);
+			} else if (seen == IFEQ) {
+				if (stack.getStackDepth() > 0) {
+					OpcodeStack.Item itm = stack.getStackItem(0);
+					Integer id = (Integer)itm.getUserValue();
+					if (id != null) {
+						int target = getBranchTarget();
+						int gotoAddr = target - 3;
+						if (getCode().getCode()[gotoAddr] == GOTO) {
+							loops.put(id, new Loop(getPC(), gotoAddr));
+						}
+					}
+				}
 			}
 		} finally {
 			stack.sawOpcode(this, seen);
@@ -179,7 +222,7 @@
 		}
 	}
 	
-	private int findCollectionGroup(OpcodeStack.Item itm) {
+	private int findCollectionGroup(OpcodeStack.Item itm, boolean addIfNotFound) {
 		Comparable groupElement = null;
 		
 		Integer id = (Integer)itm.getUserValue();
@@ -205,12 +248,37 @@
 				}
 			}
 			
-			Set<Comparable> group = new HashSet<Comparable>();
-			group.add(groupElement);
-			collectionGroups.add(group);
-			return collectionGroups.size() - 1;
+			if (addIfNotFound) {
+				Set<Comparable> group = new HashSet<Comparable>();
+				group.add(groupElement);
+				collectionGroups.add(group);
+				return collectionGroups.size() - 1;
+			}
 		}
 		
 		return -1;
 	}
+	
+	class Loop
+	{
+		public int loopStart;
+		public int loopFinish;
+		
+		public Loop(int start, int finish) {
+			loopStart = start;
+			loopFinish = finish;
+		}
+
+		public int getLoopFinish() {
+			return loopFinish;
+		}
+
+		public int getLoopStart() {
+			return loopStart;
+		}
+		
+		public boolean hasPC(int pc) {
+			return (loopStart <= pc) && (pc <= loopFinish);
+		}
+	}
 }


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.