[Fb-contrib-commit] fb-contrib/src/com/mebigfatguy/fbcontrib/detect SyncCollectionIterators.java,1.6
Brought to you by:
dbrosius
Menu
▾
▴
[Fb-contrib-commit] fb-contrib/src/com/mebigfatguy/fbcontrib/detect SyncCollectionIterators.java,1.6,1.7
|
From: Dave B. <dbr...@us...> - 2005-11-11 04:29:36
|
Update of /cvsroot/fb-contrib/fb-contrib/src/com/mebigfatguy/fbcontrib/detect In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv21236/src/com/mebigfatguy/fbcontrib/detect Modified Files: SyncCollectionIterators.java Log Message: report synchronized collections that are iterated over, when synced on an object that is NOT the collection. Index: SyncCollectionIterators.java =================================================================== RCS file: /cvsroot/fb-contrib/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SyncCollectionIterators.java,v retrieving revision 1.6 retrieving revision 1.7 diff -u -d -r1.6 -r1.7 --- SyncCollectionIterators.java 5 Nov 2005 02:23:26 -0000 1.6 +++ SyncCollectionIterators.java 11 Nov 2005 04:29:25 -0000 1.7 @@ -18,7 +18,9 @@ */ package com.mebigfatguy.fbcontrib.detect; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.apache.bcel.classfile.Code; @@ -28,6 +30,7 @@ import edu.umd.cs.findbugs.BugInstance; import edu.umd.cs.findbugs.BugReporter; import edu.umd.cs.findbugs.BytecodeScanningDetector; +import edu.umd.cs.findbugs.FieldAnnotation; import edu.umd.cs.findbugs.OpcodeStack; import edu.umd.cs.findbugs.StatelessDetector; import edu.umd.cs.findbugs.ba.ClassContext; @@ -41,7 +44,7 @@ add("synchronizedMap"); add("synchronizedList"); add("synchronizedSortedSet"); - add("synchronzedSortedMap"); + add("synchronizedSortedMap"); }}; private static Set<String> mapToSetMethods = new HashSet<String>() {{ @@ -57,7 +60,9 @@ private int state; private Set<String> memberCollections = new HashSet<String>(); private Set<Integer> localCollections = new HashSet<Integer>(); - private int monitorCount; + private List<Object> monitorObjects = new ArrayList<Object>(); + private OpcodeStack stack = new OpcodeStack(); + private Object collectionInfo = null; public SyncCollectionIterators(final BugReporter bugReporter) { this.bugReporter = bugReporter; @@ -79,89 +84,125 @@ if (obj.getCode() != null) { state = SEEN_NOTHING; localCollections.clear(); - monitorCount = 0; + monitorObjects.clear(); + stack.resetForMethodEntry(this); super.visitCode(obj); } } @Override public void sawOpcode(final int seen) { - switch (state) { - case SEEN_NOTHING: - if ((seen == INVOKESTATIC) && "java/util/Collections".equals(getClassConstantOperand())) { - if (synchCollectionNames.contains(getNameConstantOperand())) { - state = SEEN_SYNC; - } - } else if (seen == ALOAD) { - int reg = getRegisterOperand(); - if (localCollections.contains(new Integer(reg))) { - state = SEEN_LOAD; + try { + switch (state) { + case SEEN_NOTHING: + if ((seen == INVOKESTATIC) && "java/util/Collections".equals(getClassConstantOperand())) { + if (synchCollectionNames.contains(getNameConstantOperand())) { + state = SEEN_SYNC; + } + } else if (seen == ALOAD) { + int reg = getRegisterOperand(); + if (localCollections.contains(new Integer(reg))) { + collectionInfo = new Integer(reg); + state = SEEN_LOAD; + } + } else if ((seen >= ALOAD_0) && (seen <= ALOAD_3)) { + int reg = seen - ALOAD_0; + if (localCollections.contains(new Integer(reg))) { + collectionInfo = new Integer(reg); + state = SEEN_LOAD; + } + } else if (seen == GETFIELD) { + ConstantFieldref ref = (ConstantFieldref)getConstantRefOperand(); + ConstantNameAndType nandt = (ConstantNameAndType)getConstantPool().getConstant(ref.getNameAndTypeIndex()); + + String fieldName = nandt.getName(getConstantPool()); + if (memberCollections.contains(fieldName)) { + collectionInfo = fieldName; + state = SEEN_LOAD; + } } - } else if ((seen >= ALOAD_0) && (seen <= ALOAD_3)) { - int reg = seen - ALOAD_0; - if (localCollections.contains(new Integer(reg))) { - state = SEEN_LOAD; + break; + + case SEEN_SYNC: + if (seen == ASTORE) { + int reg = getRegisterOperand(); + localCollections.add(new Integer(reg)); + } else if ((seen >= ASTORE_0) && (seen <= ASTORE_3)) { + int reg = seen - ASTORE_0; + localCollections.add(new Integer(reg)); } - } else if (seen == GETFIELD) { - ConstantFieldref ref = (ConstantFieldref)getConstantRefOperand(); - ConstantNameAndType nandt = (ConstantNameAndType)getConstantPool().getConstant(ref.getNameAndTypeIndex()); - - if (memberCollections.contains(nandt.getName(getConstantPool()))) { - state = SEEN_LOAD; + else if (seen == PUTFIELD) { + ConstantFieldref ref = (ConstantFieldref)getConstantRefOperand(); + ConstantNameAndType nandt = (ConstantNameAndType)getConstantPool().getConstant(ref.getNameAndTypeIndex()); + memberCollections.add(nandt.getName(getConstantPool())); } - } - break; - - case SEEN_SYNC: - if (seen == ASTORE) { - int reg = getRegisterOperand(); - localCollections.add(new Integer(reg)); - } else if ((seen >= ASTORE_0) && (seen <= ASTORE_3)) { - int reg = seen - ASTORE_0; - localCollections.add(new Integer(reg)); - } - else if (seen == PUTFIELD) { - ConstantFieldref ref = (ConstantFieldref)getConstantRefOperand(); - ConstantNameAndType nandt = (ConstantNameAndType)getConstantPool().getConstant(ref.getNameAndTypeIndex()); - memberCollections.add(nandt.getName(getConstantPool())); - } - state = SEEN_NOTHING; - break; - - case SEEN_LOAD: - if (seen == INVOKEINTERFACE) { - String calledClass = getClassConstantOperand(); - if ("java/lang/Map".equals(calledClass)) { - if (mapToSetMethods.contains(getNameConstantOperand())) { - state = SEEN_LOAD; - } else { - state = SEEN_NOTHING; - } - } else if (calledClass.startsWith("java/util/")) { - if ("iterator".equals(getNameConstantOperand())) { - if (monitorCount == 0) { - bugReporter.reportBug( new BugInstance( this, "SCI_SYNCHRONIZED_COLLECTION_ITERATORS", NORMAL_PRIORITY) - .addClass(this) - .addMethod(this) - .addSourceLine(this)); - state = SEEN_NOTHING; + state = SEEN_NOTHING; + break; + + case SEEN_LOAD: + if (seen == INVOKEINTERFACE) { + String calledClass = getClassConstantOperand(); + if ("java/lang/Map".equals(calledClass)) { + if (mapToSetMethods.contains(getNameConstantOperand())) { + state = SEEN_LOAD; } else { state = SEEN_NOTHING; } + } else if (calledClass.startsWith("java/util/")) { + if ("iterator".equals(getNameConstantOperand())) { + if (monitorObjects.size() == 0) { + bugReporter.reportBug( new BugInstance( this, "SCI_SYNCHRONIZED_COLLECTION_ITERATORS", NORMAL_PRIORITY) + .addClass(this) + .addMethod(this) + .addSourceLine(this)); + state = SEEN_NOTHING; + } else { + Object syncObj = monitorObjects.get(monitorObjects.size() - 1); + + if (!syncIsMap(syncObj, collectionInfo)) { + bugReporter.reportBug( new BugInstance( this, "SCI_SYNCHRONIZED_COLLECTION_ITERATORS", NORMAL_PRIORITY) + .addClass(this) + .addMethod(this) + .addSourceLine(this)); + } + state = SEEN_NOTHING; + } + } + } else { + state = SEEN_NOTHING; } } else { state = SEEN_NOTHING; } - } else { - state = SEEN_NOTHING; + break; + } + + if (seen == MONITORENTER) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + int reg = item.getRegisterNumber(); + if (reg >= 0) + monitorObjects.add(new Integer(reg)); + else { + FieldAnnotation fa = item.getField(); + if (fa != null) + monitorObjects.add(fa.getFieldName()); + } } - break; + } else if (seen == MONITOREXIT) { + if (monitorObjects.size() > 0) + monitorObjects.remove(monitorObjects.size()-1); + } + } finally { + stack.sawOpcode(this, seen); } + } + + private boolean syncIsMap(Object syncObject, Object collectionInfo) { + if ((syncObject != null) && (collectionInfo != null) && syncObject.getClass().equals(collectionInfo.getClass())) + return syncObject.equals(collectionInfo); - if (seen == MONITORENTER) { - monitorCount++; - } else if (seen == MONITOREXIT) { - monitorCount--; - } + //Something went wrong... don't report + return true; } } |
