Thread: [Fb-contrib-commit] fb-contrib/src/com/mebigfatguy/fbcontrib/detect LocalSynchronizedCollection.java
Brought to you by:
dbrosius
Menu
▾
▴
fb-contrib-commit
|
From: Dave B. <dbr...@us...> - 2006-03-13 05:44:49
|
Update of /cvsroot/fb-contrib/fb-contrib/src/com/mebigfatguy/fbcontrib/detect In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv4186/src/com/mebigfatguy/fbcontrib/detect Modified Files: LocalSynchronizedCollection.java Log Message: honor variable ranges, and don't report collections that are assigned to from other unknown sources. Index: LocalSynchronizedCollection.java =================================================================== RCS file: /cvsroot/fb-contrib/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/LocalSynchronizedCollection.java,v retrieving revision 1.6 retrieving revision 1.7 diff -u -d -r1.6 -r1.7 --- LocalSynchronizedCollection.java 12 Mar 2006 15:19:07 -0000 1.6 +++ LocalSynchronizedCollection.java 13 Mar 2006 05:44:45 -0000 1.7 @@ -20,10 +20,14 @@ import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.Map; import java.util.Set; import org.apache.bcel.classfile.Code; +import org.apache.bcel.classfile.LocalVariable; +import org.apache.bcel.classfile.LocalVariableTable; +import org.apache.bcel.classfile.Method; import org.apache.bcel.generic.Type; import edu.umd.cs.findbugs.BugInstance; @@ -39,191 +43,275 @@ * thread safe, using synchronized collections in this context makes no sense. */ public class LocalSynchronizedCollection extends BytecodeScanningDetector implements StatelessDetector { - private static Set<String> syncCtors = new HashSet<String>(); - static { - syncCtors.add("java/util/Vector"); - syncCtors.add("java/util/Hashtable"); - } - private static Set<String> syncMethods = new HashSet<String>(); - static { - syncMethods.add("synchronizedCollection"); - syncMethods.add("synchronizedList"); - syncMethods.add("synchronizedMap"); - syncMethods.add("synchronizedSet"); - syncMethods.add("synchronizedSortedMap"); - syncMethods.add("synchronizedSortedSet"); - } - - private BugReporter bugReporter; - private OpcodeStack stack = new OpcodeStack(); - private Map<Integer, CollectionRegInfo> syncRegs = new HashMap<Integer, CollectionRegInfo>(); - - /** + private static Set<String> syncCtors = new HashSet<String>(); + static { + syncCtors.add("java/util/Vector"); + syncCtors.add("java/util/Hashtable"); + } + private static Set<String> syncMethods = new HashSet<String>(); + static { + syncMethods.add("synchronizedCollection"); + syncMethods.add("synchronizedList"); + syncMethods.add("synchronizedMap"); + syncMethods.add("synchronizedSet"); + syncMethods.add("synchronizedSortedMap"); + syncMethods.add("synchronizedSortedSet"); + } + + private BugReporter bugReporter; + private OpcodeStack stack = new OpcodeStack(); + private Map<Integer, CollectionRegInfo> syncRegs = new HashMap<Integer, CollectionRegInfo>(); + /** * constructs a LSYC detector given the reporter to report bugs on * @param bugReporter the sync of bug reports - */ - public LocalSynchronizedCollection(BugReporter bugReporter) { - this.bugReporter = bugReporter; - } - - /** - * clone this detector so that it can be a StatelessDetector - * - * @return a clone of this object - */ - @Override - public Object clone() throws CloneNotSupportedException { - return super.clone(); - } - - /** - * implements the visitor to reset the stack - * - * @param obj the context object of the currently parsed code block - */ - @Override - public void visitCode(Code obj) { - stack.resetForMethodEntry(this); - syncRegs.clear(); - super.visitCode(obj); - - for (Map.Entry<Integer, CollectionRegInfo> entry : syncRegs.entrySet()) { - CollectionRegInfo cri = entry.getValue(); - bugReporter.reportBug(new BugInstance(this, "LSYC_LOCAL_SYNCHRONIZED_COLLECTION", cri.getPriority()) - .addClass(this) - .addMethod(this) - .addSourceLine(cri.getSourceLineAnnotation())); - - } - } - - /** - * implements the visitor to find stores to locals of synchronized collections - * - * @param seen the opcode of the currently parsed instruction - */ - @Override - public void sawOpcode(int seen) { - //TODO: Keep track of local variable scope - //TODO: If a store into a variable, other than above, remove from consideration - Integer tosIsSyncColReg = null; - try { - stack.mergeJumps(this); - - if (seen == INVOKESPECIAL) { - if ("<init>".equals(getNameConstantOperand())) { - if (syncCtors.contains(getClassConstantOperand())) { - tosIsSyncColReg = new Integer(-1); - } - } - } else if (seen == INVOKESTATIC) { - if ("java/util/Collections".equals(getClassConstantOperand())) { - if (syncMethods.contains(getNameConstantOperand())) { - tosIsSyncColReg = new Integer(-1); - } - } - } else if ((seen == ASTORE) || ((seen >= ASTORE_0) && (seen <= ASTORE_3))) { - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - if (item.getUserValue() != null) { - int reg = getAStoreReg(seen); - if (!syncRegs.containsKey(new Integer(reg))) { - CollectionRegInfo cri = new CollectionRegInfo(SourceLineAnnotation.fromVisitedInstruction(this)); - syncRegs.put(new Integer(reg), cri); - - } - } - } - } else if ((seen == ALOAD) || ((seen >= ALOAD_0) && (seen <= ALOAD_3))) { - int reg = getALoadReg(seen); - if (syncRegs.containsKey(new Integer(reg))) - tosIsSyncColReg = new Integer(reg); - } else if ((seen == PUTFIELD) || (seen == ARETURN)) { - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - syncRegs.remove(item.getUserValue()); - } - } - - if (syncRegs.size() > 0) { - if ((seen == INVOKESPECIAL) - || (seen == INVOKEINTERFACE) - || (seen == INVOKEVIRTUAL) - || (seen == INVOKESTATIC)) { - String sig = getSigConstantOperand(); - int argCount = Type.getArgumentTypes(sig).length; - if (stack.getStackDepth() >= argCount) { - for (int i = 0; i < argCount; i++) { - OpcodeStack.Item item = stack.getStackItem(i); - CollectionRegInfo cri = syncRegs.get(item.getUserValue()); - if (cri != null) - cri.setPriority(LOW_PRIORITY); - } - } - } else if (seen == MONITORENTER) { - //Assume if synchronized blocks are used then something tricky is going on. - //There is really no valid reason for this, other than folks who use - //synchronized blocks tend to know what's going on. - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - syncRegs.remove(item.getUserValue()); - } - } - } - } finally { - stack.sawOpcode(this, seen); - if (tosIsSyncColReg != null) { - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - item.setUserValue(tosIsSyncColReg); - } - } - } - } + */ + public LocalSynchronizedCollection(BugReporter bugReporter) { + this.bugReporter = bugReporter; + } + + /** + * clone this detector so that it can be a StatelessDetector + * + * @return a clone of this object + */ + @Override + public Object clone() throws CloneNotSupportedException { + return super.clone(); + } + + /** + * implements the visitor to collect parameter registers + * + * @param obj the context object of the currently parsed method + */ + @Override + public void visitMethod(Method obj) { + syncRegs.clear(); + String sig = obj.getSignature(); + Type[] argTypes = Type.getArgumentTypes(sig); + int curReg = obj.isStatic() ? 0 : 1; + for (Type t : argTypes) { + sig = t.getSignature(); + syncRegs.put(new Integer(curReg), new CollectionRegInfo(getLocalVariableEndRange(curReg, 0))); + if ("J".equals(sig) || "D".equals(sig)) { + curReg++; + } + curReg++; + } + } + + /** + * implements the visitor to reset the stack + * + * @param obj the context object of the currently parsed code block + */ + @Override + public void visitCode(Code obj) { + stack.resetForMethodEntry(this); + super.visitCode(obj); + + for (Map.Entry<Integer, CollectionRegInfo> entry : syncRegs.entrySet()) { + CollectionRegInfo cri = entry.getValue(); + if (!cri.getIgnore()) { + bugReporter.reportBug(new BugInstance(this, "LSYC_LOCAL_SYNCHRONIZED_COLLECTION", cri.getPriority()) + .addClass(this) + .addMethod(this) + .addSourceLine(cri.getSourceLineAnnotation())); + } + + } + } + + /** + * implements the visitor to find stores to locals of synchronized collections + * + * @param seen the opcode of the currently parsed instruction + */ + @Override + public void sawOpcode(int seen) { + Integer tosIsSyncColReg = null; + try { + stack.mergeJumps(this); + + int curPC = getPC(); + Iterator<CollectionRegInfo> it = syncRegs.values().iterator(); + while (it.hasNext()) { + CollectionRegInfo cri = it.next(); + if (cri.getEndPCRange() < curPC) { + if (!cri.getIgnore()) { + bugReporter.reportBug(new BugInstance(this, "LSYC_LOCAL_SYNCHRONIZED_COLLECTION", cri.getPriority()) + .addClass(this) + .addMethod(this) + .addSourceLine(cri.getSourceLineAnnotation())); + } + it.remove(); + } + } + + if (seen == INVOKESPECIAL) { + if ("<init>".equals(getNameConstantOperand())) { + if (syncCtors.contains(getClassConstantOperand())) { + tosIsSyncColReg = new Integer(-1); + } + } + } else if (seen == INVOKESTATIC) { + if ("java/util/Collections".equals(getClassConstantOperand())) { + if (syncMethods.contains(getNameConstantOperand())) { + tosIsSyncColReg = new Integer(-1); + } + } + } else if ((seen == ASTORE) || ((seen >= ASTORE_0) && (seen <= ASTORE_3))) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + int reg = getAStoreReg(seen); + if (item.getUserValue() != null) { + if (!syncRegs.containsKey(new Integer(reg))) { + CollectionRegInfo cri = new CollectionRegInfo(SourceLineAnnotation.fromVisitedInstruction(this), getLocalVariableEndRange(reg, getNextPC())); + syncRegs.put(new Integer(reg), cri); + + } + } else { + CollectionRegInfo cri = syncRegs.get(new Integer(reg)); + if (cri == null) { + cri = new CollectionRegInfo(getLocalVariableEndRange(reg, getNextPC())); + syncRegs.put(new Integer(reg), cri); + } + cri.setIgnore(); + } + } + } else if ((seen == ALOAD) || ((seen >= ALOAD_0) && (seen <= ALOAD_3))) { + int reg = getALoadReg(seen); + CollectionRegInfo cri = syncRegs.get(new Integer(reg)); + if ((cri != null) && !cri.getIgnore()) + tosIsSyncColReg = new Integer(reg); + } else if ((seen == PUTFIELD) || (seen == ARETURN)) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + syncRegs.remove(item.getUserValue()); + } + } + + if (syncRegs.size() > 0) { + if ((seen == INVOKESPECIAL) + || (seen == INVOKEINTERFACE) + || (seen == INVOKEVIRTUAL) + || (seen == INVOKESTATIC)) { + String sig = getSigConstantOperand(); + int argCount = Type.getArgumentTypes(sig).length; + if (stack.getStackDepth() >= argCount) { + for (int i = 0; i < argCount; i++) { + OpcodeStack.Item item = stack.getStackItem(i); + CollectionRegInfo cri = syncRegs.get(item.getUserValue()); + if (cri != null) + cri.setPriority(LOW_PRIORITY); + } + } + } else if (seen == MONITORENTER) { + //Assume if synchronized blocks are used then something tricky is going on. + //There is really no valid reason for this, other than folks who use + //synchronized blocks tend to know what's going on. + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + syncRegs.remove(item.getUserValue()); + } + } + } + } finally { + stack.sawOpcode(this, seen); + if (tosIsSyncColReg != null) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + item.setUserValue(tosIsSyncColReg); + } + } + } + } - /** - * returns the register used to store a reference - * - * @param seen the opcode of the store - * @return the register stored into - */ - private int getAStoreReg(int seen) { - if (seen == ASTORE) - return getRegisterOperand(); - return seen - ASTORE_0; - } - - /** - * returns the register used to load a reference - * - * @param seen the opcode of the load - * @return the register loaded from - */ - private int getALoadReg(int seen) { - if (seen == ALOAD) - return getRegisterOperand(); - return seen - ALOAD_0; - } - - static class CollectionRegInfo - { - private SourceLineAnnotation slAnnotation; - private int priority = HIGH_PRIORITY; - - public CollectionRegInfo(SourceLineAnnotation sla) { - slAnnotation = sla; - } - public SourceLineAnnotation getSourceLineAnnotation() { - return slAnnotation; - } - - public void setPriority(int newPriority) { - if (newPriority > priority) - priority = newPriority; - } - - public int getPriority() { - return priority; - } - } + /** + * returns the register used to store a reference + * + * @param seen the opcode of the store + * @return the register stored into + */ + private int getAStoreReg(int seen) { + if (seen == ASTORE) + return getRegisterOperand(); + return seen - ASTORE_0; + } + + /** + * returns the register used to load a reference + * + * @param seen the opcode of the load + * @return the register loaded from + */ + private int getALoadReg(int seen) { + if (seen == ALOAD) + return getRegisterOperand(); + return seen - ALOAD_0; + } + + /** + * returns the end pc of the visible range of this register at this pc + * + * @param reg the register to examine + * @param curPC the pc of the current instruction + * @return the endpc + */ + private int getLocalVariableEndRange(int reg, int curPC) { + int endRange = Integer.MAX_VALUE; + LocalVariableTable lvt = this.getMethod().getLocalVariableTable(); + if (lvt != null) { + LocalVariable lv = lvt.getLocalVariable(reg, curPC); + if (lv != null) + endRange = lv.getStartPC() + lv.getLength(); + } + return endRange; + } + + static class CollectionRegInfo + { + private SourceLineAnnotation slAnnotation; + private int priority = HIGH_PRIORITY; + private int endPCRange = Integer.MAX_VALUE; + + public CollectionRegInfo(SourceLineAnnotation sla, int endPC) { + slAnnotation = sla; + endPCRange = endPC; + } + + public CollectionRegInfo(int endPC) { + slAnnotation = null; + endPCRange = endPC; + } + + public SourceLineAnnotation getSourceLineAnnotation() { + return slAnnotation; + } + + public void setEndPCRange(int pc) { + endPCRange = pc; + } + + public int getEndPCRange() { + return endPCRange; + } + + public void setIgnore() { + slAnnotation = null; + } + + public boolean getIgnore() { + return slAnnotation == null; + } + + public void setPriority(int newPriority) { + if (newPriority > priority) + priority = newPriority; + } + + public int getPriority() { + return priority; + } + } } |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X