[Fb-contrib-commit] SF.net SVN: fb-contrib: [699] trunk/fb-contrib
Brought to you by:
dbrosius
Menu
▾
▴
[Fb-contrib-commit] SF.net SVN: fb-contrib: [699] trunk/fb-contrib
|
From: <dbr...@us...> - 2006-12-03 09:52:28
|
Revision: 699
http://svn.sourceforge.net/fb-contrib/?rev=699&view=rev
Author: dbrosius
Date: 2006-12-03 01:52:26 -0800 (Sun, 03 Dec 2006)
Log Message:
-----------
BAS starts to hobble along
Modified Paths:
--------------
trunk/fb-contrib/etc/findbugs.xml
trunk/fb-contrib/etc/messages.xml
trunk/fb-contrib/samples/BAS_Sample.java
trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/BloatedAssignmentScope.java
Modified: trunk/fb-contrib/etc/findbugs.xml
===================================================================
--- trunk/fb-contrib/etc/findbugs.xml 2006-11-30 06:05:35 UTC (rev 698)
+++ trunk/fb-contrib/etc/findbugs.xml 2006-12-03 09:52:26 UTC (rev 699)
@@ -257,7 +257,10 @@
<Detector class="com.mebigfatguy.fbcontrib.detect.SillynessPotPourri"
speed="fast"
reports="SPP_NEGATIVE_BITSET_ITEM,SPP_INTERN_ON_CONSTANT,SPP_NO_CHAR_SB_CTOR,SPP_USE_MATH_CONSTANT,SPP_STUTTERED_ASSIGNMENT,SPP_USE_ISNAN,SPP_USE_BIGDECIMAL_STRING_CTOR"/>
-
+
+ <Detector class="com.mebigfatguy.fbcontrib.detect.BloatedAssignmentScope"
+ speed="fast"
+ reports="BAS_BLOATED_ASSIGNMENT_SCOPE" />
<!-- BugPattern -->
<BugPattern abbrev="ISB" type="ISB_INEFFICIENT_STRING_BUFFERING" category="PERFORMANCE" />
@@ -330,4 +333,5 @@
<BugPattern abbrev="SPP" type="SPP_STUTTERED_ASSIGNMENT" category="CORRECTNESS" experimental="true" />
<BugPattern abbrev="SPP" type="SPP_USE_ISNAN" category="CORRECTNESS" experimental="true" />
<BugPattern abbrev="SPP" type="SPP_USE_BIGDECIMAL_STRING_CTOR" category="CORRECTNESS" experimental="true" />
+ <BugPattern abbrev="BAS" type="BAS_BLOATED_ASSIGNMENT_SCOPE" category="PERFORMANCE" experimental="true" />
</FindbugsPlugin>
\ No newline at end of file
Modified: trunk/fb-contrib/etc/messages.xml
===================================================================
--- trunk/fb-contrib/etc/messages.xml 2006-11-30 06:05:35 UTC (rev 698)
+++ trunk/fb-contrib/etc/messages.xml 2006-12-03 09:52:26 UTC (rev 699)
@@ -693,6 +693,15 @@
]]>
</Details>
</Detector>
+
+ <Detector class="com.mebigfatguy.fbcontrib.detect.BloatedAssignmentScope">
+ <Details>
+ <![CDATA[
+ <p>looks for assignments to variables in a scope larger than it's use. As long as the evaluation of the assignment
+ does not have side effects, the assignment can be moved into the inner scope where it is used.</p>
+ ]]>
+ </Details>
+ </Detector>
<!-- BugPattern -->
@@ -1596,6 +1605,19 @@
]]>
</Details>
</BugPattern>
+
+ <BugPattern type="BAS_BLOATED_ASSIGNMENT_SCOPE">
+ <ShortDescription>Method assigns a variable in a larger scope then is needed</ShortDescription>
+ <LongDescription>Method {1} assigns a variable in a larger scope then is needed</LongDescription>
+ <Details>
+ <![CDATA[
+ <p>This method assigns a value to a variable in an outer scope compared to where the variable is actually used.
+ Assuming this evaluation does not have side effects, the assignment can be moved into the inner scope (if block)
+ so that its execution time isn't taken up if the if guard is false.</p>
+ ]]>
+ </Details>
+ </BugPattern>
+
<!-- BugCode -->
<BugCode abbrev="ISB">Inefficient String Buffering</BugCode>
@@ -1655,5 +1677,6 @@
<BugCode abbrev="PIS">Possible Incomplete Serialization</BugCode>
<BugCode abbrev="SCRV">Suspicious Comparator Return Values</BugCode>
<BugCode abbrev="SPP">Sillyness Pot Pourri</BugCode>
+ <BugCode abbrev="BAS">Bloated Assignment Scope</BugCode>
</MessageCollection>
\ No newline at end of file
Modified: trunk/fb-contrib/samples/BAS_Sample.java
===================================================================
--- trunk/fb-contrib/samples/BAS_Sample.java 2006-11-30 06:05:35 UTC (rev 698)
+++ trunk/fb-contrib/samples/BAS_Sample.java 2006-12-03 09:52:26 UTC (rev 699)
@@ -20,5 +20,30 @@
}
return s;
}
+
+ public String testFP2Scopes(String s)
+ {
+ Object o = new Object();
+ if (s.equals("Boo"))
+ s = o.toString();
+ else if (s.equals("Hoo"))
+ s = o.toString();
+
+ return s;
+ }
+
+ public String test2InnerScopes(String s)
+ {
+ Object o = new Object();
+ if (s != null)
+ {
+ if (s.equals("Boo"))
+ s = o.toString();
+ else if (s.equals("Hoo"))
+ s = o.toString();
+ }
+
+ return s;
+ }
}
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/BloatedAssignmentScope.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/BloatedAssignmentScope.java 2006-11-30 06:05:35 UTC (rev 698)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/BloatedAssignmentScope.java 2006-12-03 09:52:26 UTC (rev 699)
@@ -1,12 +1,18 @@
package com.mebigfatguy.fbcontrib.detect;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.bcel.classfile.Code;
+import org.apache.bcel.classfile.Method;
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.ba.ClassContext;
@@ -18,19 +24,20 @@
*/
public class BloatedAssignmentScope extends BytecodeScanningDetector
{
- private BugReporter bugReporter;
+ BugReporter bugReporter;
private Map<Integer, Integer> regToLocationMap;
- private Map<Integer, Integer> scopeBlockMap;
+ private Set<Integer> ignoreRegs;
+ private ScopeBlock rootScopeBlock;
/**
- * constructs a BLT detector given the reporter to report bugs on
+ * constructs a BAS detector given the reporter to report bugs on
* @param bugReporter the sync of bug reports
*/
public BloatedAssignmentScope(BugReporter bugReporter) {
this.bugReporter = bugReporter;
}
-
+
/**
* implements the visitor to create and the clear the register to location map
*
@@ -40,11 +47,11 @@
public void visitClassContext(ClassContext classContext) {
try {
regToLocationMap = new HashMap<Integer, Integer>();
- scopeBlockMap = new HashMap<Integer, Integer>();
+ ignoreRegs = new HashSet<Integer>();
super.visitClassContext(classContext);
} finally {
regToLocationMap = null;
- scopeBlockMap = null;
+ ignoreRegs = null;
}
}
@@ -55,23 +62,282 @@
*/
@Override
public void visitCode(Code obj) {
- regToLocationMap.clear();
- scopeBlockMap.clear();
- super.visitCode(obj);
+ try {
+
+ regToLocationMap.clear();
+ ignoreRegs.clear();
+ Method method = getMethod();
+ if (!method.isStatic())
+ ignoreRegs.add(Integer.valueOf(0));
+
+ int[] parmRegs = RegisterUtils.getParameterRegisters(method);
+ for (int parm : parmRegs) {
+ ignoreRegs.add(Integer.valueOf(parm));
+ }
+
+ rootScopeBlock = new ScopeBlock(0, obj.getLength());
+ super.visitCode(obj);
+
+ rootScopeBlock.findBugs();
+
+ } finally {
+ rootScopeBlock = null;
+ }
}
-
+
/**
- * implements the visitor to look for variables assigned out side of the scope
+ * implements the visitor to look for variables assigned below the scope
* in which they are used.
*
* @param seen the opcode of the currently parsed instruction
*/
@Override
public void sawOpcode(int seen) {
- if ((seen == ASTORE) || ((seen >= ASTORE_0) && (seen <= ASTORE_3))) {
- int reg = RegisterUtils.getAStoreReg(this, seen);
- } else if ((seen == ALOAD) || ((seen >= ALOAD_0) && (seen <= ALOAD_3))) {
- int reg = RegisterUtils.getALoadReg(this, seen);
+ if ((seen == ASTORE)
+ || (seen == ISTORE)
+ || (seen == LSTORE)
+ || (seen == FSTORE)
+ || (seen == DSTORE)
+ || ((seen >= ASTORE_0) && (seen <= ASTORE_3))
+ || ((seen >= ISTORE_0) && (seen <= ISTORE_3))
+ || ((seen >= LSTORE_0) && (seen <= LSTORE_1))
+ || ((seen >= FSTORE_0) && (seen <= FSTORE_1))
+ || ((seen >= DSTORE_0) && (seen <= DSTORE_1))) {
+ int reg = RegisterUtils.getStoreReg(this, seen);
+ if (!ignoreRegs.contains(Integer.valueOf(reg))) {
+ ScopeBlock sb = findScopeBlock(rootScopeBlock, getPC());
+ if (sb != null)
+ sb.addStore(reg, getPC());
+ else
+ ignoreRegs.add(Integer.valueOf(reg));
+ }
+
+ } else if ((seen == ALOAD)
+ || (seen == ILOAD)
+ || (seen == LLOAD)
+ || (seen == FLOAD)
+ || (seen == DLOAD)
+ || ((seen >= ALOAD_0) && (seen <= ALOAD_3))
+ || ((seen >= ILOAD_0) && (seen <= ILOAD_3))
+ || ((seen >= LLOAD_0) && (seen <= LLOAD_1))
+ || ((seen >= FLOAD_0) && (seen <= FLOAD_1))
+ || ((seen >= DLOAD_0) && (seen <= DLOAD_1))) {
+ int reg = RegisterUtils.getLoadReg(this, seen);
+ if (!ignoreRegs.contains(Integer.valueOf(reg))) {
+ ScopeBlock sb = findScopeBlock(rootScopeBlock, getPC());
+ if (sb != null)
+ sb.addLoad(reg, getPC());
+ else
+ ignoreRegs.add(Integer.valueOf(reg));
+ }
+ } else if (((seen >= IFEQ) && (seen <= GOTO)) || (seen == GOTO_W)) {
+ int target = getBranchTarget();
+ if (target > getPC()) {
+ ScopeBlock sb = new ScopeBlock(getPC(), target);
+ rootScopeBlock.addChild(sb);
+ } else {
+ ScopeBlock sb = findScopeBlock(rootScopeBlock, getPC());
+ if (sb != null)
+ sb.setLoop();
+ }
}
}
+
+ /**
+ * returns the scope block in which this register was assigned, by traversing the scope block tree
+ *
+ * @param sb the scope block to start searching in
+ * @param pc the current program counter
+ * @return the scope block or null if not found
+ */
+ private ScopeBlock findScopeBlock(ScopeBlock sb, int pc) {
+
+ if ((pc > sb.getStart()) && (pc < sb.getFinish())) {
+ if (sb.children != null) {
+ for (ScopeBlock child : sb.children) {
+ sb = findScopeBlock(child, pc);
+ if (sb != null)
+ return sb;
+ }
+ }
+ return sb;
+ }
+ return null;
+ }
+
+ /** holds the description of a scope { } block, be it a for, if, while block
+ */
+ private class ScopeBlock
+ {
+ private int startLocation;
+ private int finishLocation;
+ private boolean isLoop;
+ private Map<Integer, Integer> loads;
+ private Map<Integer, Integer> stores;
+ private List<ScopeBlock> children;
+
+ /** construts a new scope block
+ *
+ * @param start the beginning of the block
+ * @param finish the end of the block
+ */
+ public ScopeBlock(int start, int finish) {
+ startLocation = start;
+ finishLocation = finish;
+ isLoop = false;
+ loads = null;
+ stores = null;
+ children = null;
+ }
+
+ /**
+ * returns a string representation of the scope block
+ *
+ * @returns a string representation
+ */
+ public String toString() {
+ return "Start=" + startLocation + " Finish=" + finishLocation + " Loop=" + isLoop + " Loads=" + loads + " Stores=" + stores;
+ }
+
+ /** returns the start of the block
+ *
+ * @return the start of the block
+ */
+ public int getStart() {
+ return startLocation;
+ }
+
+ /** returns the end of the block
+ *
+ * @return the end of the block
+ */
+ public int getFinish() {
+ return finishLocation;
+ }
+
+ /**
+ * sets that this block is a loop
+ */
+ public void setLoop() {
+ isLoop = true;
+ }
+
+ /**
+ * adds the register as a store in this scope block
+ *
+ * @param reg the register that was stored
+ * @param pc the instruction that did the store
+ */
+ public void addStore(int reg, int pc) {
+ if (stores == null)
+ stores = new HashMap<Integer, Integer>();
+
+ stores.put(Integer.valueOf(reg), Integer.valueOf(pc));
+ }
+
+ /**
+ * adds the register as a load in this scope block
+ *
+ * @param reg the register that was loaded
+ * @param pc the instruction that did the load
+ */
+ public void addLoad(int reg, int pc) {
+ if (loads == null)
+ loads = new HashMap<Integer, Integer>();
+
+ loads.put(Integer.valueOf(reg), Integer.valueOf(pc));
+ }
+
+ /**
+ * adds a scope block to this subtree by finding the correct place in the hierarchy to store it
+ *
+ * @param child the scope block to add to the tree
+ */
+ public void addChild(ScopeBlock newChild) {
+ if (children != null) {
+ for (ScopeBlock child : children) {
+ if ((newChild.startLocation > child.startLocation) && (newChild.finishLocation < child.finishLocation)) {
+ child.addChild(newChild);
+ return;
+ }
+ }
+ int pos = 0;
+ for (ScopeBlock child : children) {
+ if (newChild.startLocation < child.startLocation) {
+ children.add(pos, newChild);
+ return;
+ }
+ pos++;
+ }
+ children.add(newChild);
+ return;
+ }
+ children = new ArrayList<ScopeBlock>();
+ children.add(newChild);
+ }
+
+ /**
+ * report stores that occur at scopes higher than associated loads that are not involved with loops
+ */
+ public void findBugs() {
+ if (isLoop)
+ return;
+
+ if (stores != null) {
+ if (loads != null)
+ stores.keySet().removeAll(loads.keySet());
+
+ if (children != null) {
+ for (Map.Entry<Integer, Integer> entry : stores.entrySet()) {
+ int childUseCount = 0;
+ boolean inLoop = false;
+ for (ScopeBlock child : children) {
+ if (child.usesReg(entry.getKey())) {
+ if (child.isLoop) {
+ inLoop = true;
+ break;
+ }
+ childUseCount++;
+ }
+ }
+ if ((!inLoop) && (childUseCount == 1)) {
+ bugReporter.reportBug(new BugInstance(BloatedAssignmentScope.this, "BAS_BLOATED_ASSIGNMENT_SCOPE", NORMAL_PRIORITY)
+ .addClass(BloatedAssignmentScope.this)
+ .addMethod(BloatedAssignmentScope.this)
+ .addSourceLine(BloatedAssignmentScope.this, entry.getValue()));
+ }
+ }
+ }
+ }
+
+ if (children != null) {
+ for (ScopeBlock child : children) {
+ child.findBugs();
+ }
+ }
+ }
+
+ /**
+ * returns whether this block either loads or stores into the register in question
+ *
+ * @param reg the register to look for loads or stores
+ *
+ * @return whether the block uses the register
+ */
+ public boolean usesReg(Integer reg) {
+ if ((loads != null) && (loads.containsKey(reg)))
+ return true;
+ if ((stores != null) && (stores.containsKey(reg)))
+ return true;
+
+ if (children != null) {
+ for (ScopeBlock child : children) {
+ if (child.usesReg(reg))
+ return true;
+ }
+ }
+
+ return false;
+ }
+ }
}
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
|
