[Fb-contrib-commit] SF.net SVN: fb-contrib: [400] trunk/fb-contrib/lib
Brought to you by:
dbrosius
Menu
▾
▴
[Fb-contrib-commit] SF.net SVN: fb-contrib: [400] trunk/fb-contrib/lib
|
From: <dbr...@us...> - 2006-04-06 14:19:55
|
Revision: 400 Author: dbrosius Date: 2006-04-06 07:19:34 -0700 (Thu, 06 Apr 2006) ViewCVS: http://svn.sourceforge.net/fb-contrib/?rev=400&view=rev Log Message: ----------- Initial check: new NOS detector - doesn't work at all at this point :) Modified Paths: -------------- trunk/fb-contrib/etc/findbugs.xml trunk/fb-contrib/etc/messages.xml trunk/fb-contrib/lib/findbugs.jar Added Paths: ----------- trunk/fb-contrib/samples/NOS_Sample.java trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NonOwnedSynchronization.java Modified: trunk/fb-contrib/etc/findbugs.xml =================================================================== --- trunk/fb-contrib/etc/findbugs.xml 2006-04-04 21:36:11 UTC (rev 399) +++ trunk/fb-contrib/etc/findbugs.xml 2006-04-06 14:19:34 UTC (rev 400) @@ -175,6 +175,10 @@ speed="fast" reports="FCBL_FIELD_COULD_BE_LOCAL" /> + <Detector class="com.mebigfatguy.fbcontrib.detect.NonOwnedSynchronization" + speed="fast" + reports="NOS_NON_OWNED_SYNCHRONIZATION" /> + <!-- BugPattern --> <BugPattern abbrev="ISB" type="ISB_INEFFICIENT_STRING_BUFFERING" category="PERFORMANCE" /> @@ -217,4 +221,5 @@ <BugPattern abbrev="PMB" type="PMB_POSSIBLE_MEMORY_BLOAT" category="CORRECTNESS" experimental="true" /> <BugPattern abbrev="LSYC" type="LSYC_LOCAL_SYNCHRONIZED_COLLECTION" category="CORRECTNESS" experimental="true" /> <BugPattern abbrev="FCBL" type="FCBL_FIELD_COULD_BE_LOCAL" category="CORRECTNESS" experimental="true" /> + <BugPattern abbrev="NOS" type="NOS_NON_OWNED_SYNCHRONIZATION" category="STYLE" experimental="true" /> </FindbugsPlugin> \ No newline at end of file Modified: trunk/fb-contrib/etc/messages.xml =================================================================== --- trunk/fb-contrib/etc/messages.xml 2006-04-04 21:36:11 UTC (rev 399) +++ trunk/fb-contrib/etc/messages.xml 2006-04-06 14:19:34 UTC (rev 400) @@ -504,6 +504,19 @@ ]]> </Details> </Detector> + + <Detector class="com.mebigfatguy.fbcontrib.detect.NonOwnedSynchronization"> + <Details> + <![CDATA[ + <p>looks for methods that synchronize on variables that are not owned by the + current class. Doing this causes confusion when two classes use the same variable + for their own synchronization purposes. For cleanest separation of interests, only + synchronize on private fields of the class. Note that 'this' is not owned by + the current class and synchronization on 'this' should be avoided as well.</p> + <p>It is a fast detector</p> + ]]> + </Details> + </Detector> <!-- BugPattern --> @@ -1030,6 +1043,20 @@ </Details> </BugPattern> + <BugPattern type="NOS_NON_OWNED_SYNCHRONIZATION"> + <ShortDescription>class uses non owned variables to synchronize on</ShortDescription> + <LongDescription>class {0} uses non owned variables to synchronize on</LongDescription> + <Details> + <![CDATA[ + <p>This method uses a synchronize block where the object that is being synchronized on, + is not owned by this current instance. This means that other instances may use this same + object for synchronization for its own purposes causing synchronization confusion. It is + always cleaner and safer to only synchronize on private fields of this class. Note that 'this' + is not owned by the current instance, but is owned by whomever assigns it to a field of its + class. Synchronizing on 'this' is also not a good idea.</p> + ]]> + </Details> + </BugPattern> <!-- BugCode --> <BugCode abbrev="ISB">Inefficient String Buffering</BugCode> @@ -1072,4 +1099,5 @@ <BugCode abbrev="PMB">Possible Memory Bloat</BugCode> <BugCode abbrev="LSYC">Local Synchronized Collection</BugCode> <BugCode abbrev="FCBL">Field Could Be Local</BugCode> + <BugCode abbrev="NOS">Non Owned Synchronization</BugCode> </MessageCollection> \ No newline at end of file Modified: trunk/fb-contrib/lib/findbugs.jar =================================================================== (Binary files differ) Added: trunk/fb-contrib/samples/NOS_Sample.java =================================================================== --- trunk/fb-contrib/samples/NOS_Sample.java (rev 0) +++ trunk/fb-contrib/samples/NOS_Sample.java 2006-04-06 14:19:34 UTC (rev 400) @@ -0,0 +1,8 @@ + +public class NOS_Sample { + public String test(Object o) { + synchronized(o) { + return o.toString(); + } + } +} Added: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NonOwnedSynchronization.java =================================================================== --- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NonOwnedSynchronization.java (rev 0) +++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/NonOwnedSynchronization.java 2006-04-06 14:19:34 UTC (rev 400) @@ -0,0 +1,165 @@ +package com.mebigfatguy.fbcontrib.detect; + +import java.util.BitSet; +import java.util.HashMap; +import java.util.Map; + +import org.apache.bcel.Constants; +import org.apache.bcel.classfile.Code; +import org.apache.bcel.classfile.Method; +import org.apache.bcel.generic.Type; + +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; +import edu.umd.cs.findbugs.StatelessDetector; +import edu.umd.cs.findbugs.ba.ClassContext; + +/** + * looks for methods that synchronize on variables that are not owned by the + * for their own synchronization purposes. For cleanest separation of interests, only + * synchronize on private fields of the class. Note that 'this' is not owned by + * the current class and synchronization on 'this' should be avoided as well. + */ +public class NonOwnedSynchronization extends BytecodeScanningDetector implements StatelessDetector { + private static final Integer OWNED = Integer.MAX_VALUE; + private static final Integer LOW = new Integer(LOW_PRIORITY); + private static final Integer NORMAL = new Integer(NORMAL_PRIORITY); + private BugReporter bugReporter; + private OpcodeStack stack = new OpcodeStack(); + private Map<Integer, Integer> regPriorities = new HashMap<Integer, Integer>(); + + /** + * constructs a NOS detector given the reporter to report bugs on + * @param bugReporter the sync of bug reports + */ public NonOwnedSynchronization(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(); + } + + /** + * looks for methods that contain a MONITORENTER opcode + * + * @param method the context object of the current method + * @return if the class uses synchronization + */ + public boolean prescreen(Method method) { + BitSet bytecodeSet = ClassContext.getBytecodeSet(method); + return bytecodeSet != null && (bytecodeSet.get(Constants.MONITORENTER)); + } + + /** + * implements the visitor to reset the opcode stack + * + * @param obj the context object of the currently parsed code block + */ + @Override + public void visitCode(Code obj) { + if (prescreen(getMethod())) { + stack.resetForMethodEntry(this); + regPriorities.clear(); + int[] parmRegs = RegisterUtils.getParameterRegisters(getMethod()); + for (int reg : parmRegs) + regPriorities.put(new Integer(reg), NORMAL); + super.visitCode(obj); + } + } + + /** + * implements the visitor to look for synchronization on non-owned objects + * + * @param seen the opcode of the currently parsed instruction + */ + public void sawOpcode(int seen) { + Integer tosIsPriority = null; + try { + stack.mergeJumps(this); + + switch (seen) { + case GETFIELD: + tosIsPriority = OWNED; + break; + + case ALOAD: { + int reg = RegisterUtils.getALoadReg(this, seen); + if (getMethod().isStatic() && (reg == 0)) + tosIsPriority = LOW_PRIORITY; + else { + tosIsPriority = regPriorities.get(new Integer(reg)); + if (tosIsPriority == null) + tosIsPriority = NORMAL; + } + } + break; + + case ASTORE: { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + Integer priority = (Integer)item.getUserValue(); + regPriorities.put(new Integer(RegisterUtils.getAStoreReg(this, seen)), priority); + } + } + break; + + case INVOKEVIRTUAL: + case INVOKEINTERFACE: + case INVOKESPECIAL: { + String sig = getSigConstantOperand(); + Type t = Type.getReturnType(sig); + if (t.getSignature().startsWith("L")) { + int parmCnt = Type.getArgumentTypes(sig).length; + if (stack.getStackDepth() >= parmCnt) { + OpcodeStack.Item itm = stack.getStackItem(parmCnt); + if (itm.getRegisterNumber() != 0) + tosIsPriority = NORMAL; + } + } + } + break; + + case INVOKESTATIC: { + String sig = getSigConstantOperand(); + Type t = Type.getReturnType(sig); + if (t.getSignature().startsWith("L")) { + tosIsPriority = NORMAL; + } + } + break; + + case MONITORENTER: { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item itm = stack.getStackItem(0); + Integer priority = (Integer)itm.getUserValue(); + if (priority != null) { + bugReporter.reportBug(new BugInstance(this, "NOS_NON_OWNED_SYNCHRONIZATION", priority.intValue()) + .addClass(this) + .addMethod(this) + .addSourceLine(this)); + } + } + } + break; + } + } finally { + stack.sawOpcode(this, seen); + if (tosIsPriority != null) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item itm = stack.getStackItem(0); + itm.setUserValue(tosIsPriority); + } + } + } + } +} This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. |
