[Fb-contrib-commit] SF.net SVN: fb-contrib:[1597] trunk/fb-contrib

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

Revision: 1597
          http://fb-contrib.svn.sourceforge.net/fb-contrib/?rev=1597&view=rev
Author:   dbrosius
Date:     2010-08-30 02:43:22 +0000 (Mon, 30 Aug 2010)

Log Message:
-----------
later jdk's can be sloppy about tidying up the stack, so look for returns with unassigned allocations still on the stack

Modified Paths:
--------------
    trunk/fb-contrib/samples/SEC_Sample.java
    trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SideEffectConstructor.java

Modified: trunk/fb-contrib/samples/SEC_Sample.java
===================================================================
--- trunk/fb-contrib/samples/SEC_Sample.java	2010-08-29 06:22:58 UTC (rev 1596)
+++ trunk/fb-contrib/samples/SEC_Sample.java	2010-08-30 02:43:22 UTC (rev 1597)
@@ -14,4 +14,11 @@
 		List<SEC_Sample> l = new ArrayList<SEC_Sample>();
 		new SEC_Sample(l);
 	}
+	
+	public void test() 
+	{
+		List<SEC_Sample> l = new ArrayList<SEC_Sample>();
+		new SEC_Sample(l);
+		main(new String[0]);
+	}
 }

Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SideEffectConstructor.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SideEffectConstructor.java	2010-08-29 06:22:58 UTC (rev 1596)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/SideEffectConstructor.java	2010-08-30 02:43:22 UTC (rev 1597)
@@ -19,10 +19,13 @@
 package com.mebigfatguy.fbcontrib.detect;
 
 import org.apache.bcel.classfile.Code;
+import org.apache.bcel.generic.Type;
 
 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.ba.ClassContext;
 
 /**
  * looks for constructors that operate through side effects, specifically
@@ -32,6 +35,7 @@
 
 	private enum State {SAW_NOTHING, SAW_CTOR};
 	private final BugReporter bugReporter;
+	private OpcodeStack stack;
 	private State state;
 	
 	/**
@@ -44,42 +48,92 @@
 	}
 	
 	/**
-	 * overrides the visitor to reset the state
+	 * overrides the visitor to set up and tear down the opcode stack
 	 * 
+	 * @param classContext the context object of the currently parsed class
+	 */
+	@Override
+	public void visitClassContext(ClassContext classContext) {
+		try {
+			stack = new OpcodeStack();
+			super.visitClassContext(classContext);
+		} finally {
+			stack = null;
+		}
+	}
+	/**
+	 * overrides the visitor to reset the state and reset the opcode stack
+	 * 
 	 * @param obj the context object of the currently parsed code
 	 */
 	@Override
 	public void visitCode(Code obj) {
 		state = State.SAW_NOTHING;
+		stack.resetForMethodEntry(this);
 		super.visitCode(obj);
 	}
+
 	/**
 	 * overrides the visitor to look for constructors who's value is 
-	 * popped off the stack, and not assigned.
+	 * popped off the stack, and not assigned before the pop of the value, or if a
+	 * return is issued with that object still on the stack.
 	 * 
 	 * @param seen the opcode of the currently parse opcode
 	 */
 	@Override
 	public void sawOpcode(int seen) {
-		switch (state) {
-			case SAW_NOTHING:
-				if (seen == INVOKESPECIAL) {
-					String name = getNameConstantOperand();
-					if ("<init>".equals(name)) {
-						state = State.SAW_CTOR;
+		int pc = 0;
+		try {
+			switch (state) {
+				case SAW_NOTHING:
+					if (seen == INVOKESPECIAL) {
+						String name = getNameConstantOperand();
+						if ("<init>".equals(name)) {
+							String sig = getSigConstantOperand();
+							int numArgs = Type.getArgumentTypes(sig).length;
+							if (stack.getStackDepth() > numArgs) {
+								OpcodeStack.Item caller = stack.getStackItem(numArgs);
+								if (caller.getRegisterNumber() != 0) {
+									state = State.SAW_CTOR;
+									pc = getPC();
+								}
+							}
+						}
+					} else if (seen == RETURN) {
+						int depth = stack.getStackDepth();
+						for (int i = 0; i < depth; i++) {
+							OpcodeStack.Item item = stack.getStackItem(i);
+							Integer secPC = (Integer)item.getUserValue();
+							if (secPC != null) {
+								bugReporter.reportBug(new BugInstance(this, "SEC_SIDE_EFFECT_CONSTRUCTOR", NORMAL_PRIORITY)
+								.addClass(this)
+								.addMethod(this)
+								.addSourceLine(this, secPC.intValue()));
+								break;
+							}
+							
+						}
 					}
+				break;
+				
+				case SAW_CTOR:
+					if (seen == POP || seen == RETURN) {
+						bugReporter.reportBug(new BugInstance(this, "SEC_SIDE_EFFECT_CONSTRUCTOR", NORMAL_PRIORITY)
+															.addClass(this)
+															.addMethod(this)
+															.addSourceLine(this));
+					}
+					state = State.SAW_NOTHING;
+				break;
+			}
+		} finally {
+			stack.sawOpcode(this, seen);
+			if (pc != 0) {
+				if (stack.getStackDepth() > 0) {
+					OpcodeStack.Item item = stack.getStackItem(0);
+					item.setUserValue(Integer.valueOf(pc));
 				}
-			break;
-			
-			case SAW_CTOR:
-				if (seen == POP) {
-					bugReporter.reportBug(new BugInstance(this, "SEC_SIDE_EFFECT_CONSTRUCTOR", NORMAL_PRIORITY)
-														.addClass(this)
-														.addMethod(this)
-														.addSourceLine(this));
-				}
-				state = State.SAW_NOTHING;
-			break;
+			}
 		}
 	}
 }


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