Skip to content

Commit

Permalink
cleanup warnings around the old bcel Constants interface use
Browse files Browse the repository at this point in the history
  • Loading branch information
@mebigfatguy
mebigfatguy committed Oct 11, 2017
1 parent dc80266 commit b114125
Showing 1 changed file with 176 additions and 183 deletions.
359 changes: 176 additions & 183 deletions src/main/java/com/mebigfatguy/fbcontrib/detect/SuspiciousGetterSetterUse.java
View file
Expand Up @@ -18,9 +18,11 @@
*/ */
package com.mebigfatguy.fbcontrib.detect; package com.mebigfatguy.fbcontrib.detect;


import org.apache.bcel.Const;
import org.apache.bcel.classfile.Code; import org.apache.bcel.classfile.Code;


import com.mebigfatguy.fbcontrib.utils.BugType; import com.mebigfatguy.fbcontrib.utils.BugType;
import com.mebigfatguy.fbcontrib.utils.OpcodeUtils;
import com.mebigfatguy.fbcontrib.utils.SignatureBuilder; import com.mebigfatguy.fbcontrib.utils.SignatureBuilder;
import com.mebigfatguy.fbcontrib.utils.Values; import com.mebigfatguy.fbcontrib.utils.Values;


Expand All @@ -34,188 +36,179 @@
*/ */
public class SuspiciousGetterSetterUse extends BytecodeScanningDetector { public class SuspiciousGetterSetterUse extends BytecodeScanningDetector {


private static enum State { private static enum State {
SEEN_NOTHING, SEEN_ALOAD, SEEN_GETFIELD, SEEN_DUAL_LOADS, SEEN_INVOKEVIRTUAL SEEN_NOTHING, SEEN_ALOAD, SEEN_GETFIELD, SEEN_DUAL_LOADS, SEEN_INVOKEVIRTUAL
}; };


private final BugReporter bugReporter; private final BugReporter bugReporter;
private State state; private State state;
private String beanReference1; private String beanReference1;
private String beanReference2; private String beanReference2;
private String propName; private String propName;
private String propType; private String propType;
private boolean sawField; private boolean sawField;


/** /**
* constructs a SGSU detector given the reporter to report bugs on * constructs a SGSU detector given the reporter to report bugs on
* *
* @param bugReporter * @param bugReporter
* the sync of bug reports * the sync of bug reports
*/ */
public SuspiciousGetterSetterUse(BugReporter bugReporter) { public SuspiciousGetterSetterUse(BugReporter bugReporter) {
this.bugReporter = bugReporter; this.bugReporter = bugReporter;
} }


/** /**
* overrides the visitor to reset the state to SEEN_NOTHING, and clear the * overrides the visitor to reset the state to SEEN_NOTHING, and clear the
* beanReference, propName and propType * beanReference, propName and propType
* *
* @param obj * @param obj
* the context object of the currently parsed code block * the context object of the currently parsed code block
*/ */
@Override @Override
public void visitCode(Code obj) { public void visitCode(Code obj) {
state = State.SEEN_NOTHING; state = State.SEEN_NOTHING;
beanReference1 = null; beanReference1 = null;
beanReference2 = null; beanReference2 = null;
propName = null; propName = null;
propType = null; propType = null;
sawField = false; sawField = false;
super.visitCode(obj); super.visitCode(obj);
} }


/** /**
* overrides the visitor to look for a setXXX with the value returned from a * overrides the visitor to look for a setXXX with the value returned from a
* getXXX using the same base object. * getXXX using the same base object.
* *
* @param seen * @param seen
* the currently parsed opcode * the currently parsed opcode
*/ */
@Override @Override
public void sawOpcode(int seen) { public void sawOpcode(int seen) {
boolean reset = true; boolean reset = true;
switch (state) { // TODO: Refactor this to use state pattern, not nested switch (state) { // TODO: Refactor this to use state pattern, not nested
// switches // switches
case SEEN_NOTHING: case SEEN_NOTHING:
reset = sawOpcodeAfterNothing(seen); reset = sawOpcodeAfterNothing(seen);
break; break;


case SEEN_ALOAD: case SEEN_ALOAD:
reset = sawOpcodeAfterLoad(seen); reset = sawOpcodeAfterLoad(seen);
break; break;


case SEEN_GETFIELD: case SEEN_GETFIELD:
reset = sawOpcodeAfterGetField(seen); reset = sawOpcodeAfterGetField(seen);
break; break;


case SEEN_DUAL_LOADS: case SEEN_DUAL_LOADS:
reset = sawOpcodeAfterDualLoads(seen); reset = sawOpcodeAfterDualLoads(seen);
break; break;


case SEEN_INVOKEVIRTUAL: case SEEN_INVOKEVIRTUAL:
if (seen == INVOKEVIRTUAL) { if (seen == Const.INVOKEVIRTUAL) {
checkForSGSU(); checkForSGSU();
} }
break; break;
} }


if (reset) { if (reset) {
beanReference1 = null; beanReference1 = null;
beanReference2 = null; beanReference2 = null;
propType = null; propType = null;
propName = null; propName = null;
sawField = false; sawField = false;
state = State.SEEN_NOTHING; state = State.SEEN_NOTHING;
} }
} }


private boolean sawOpcodeAfterNothing(int seen) { private boolean sawOpcodeAfterNothing(int seen) {
switch (seen) { if (!OpcodeUtils.isALoad(seen)) {
case ALOAD: return true;
case ALOAD_0: }
case ALOAD_1:
case ALOAD_2: beanReference1 = String.valueOf(getRegisterOperand());
case ALOAD_3: state = State.SEEN_ALOAD;
beanReference1 = String.valueOf(getRegisterOperand()); return false;
state = State.SEEN_ALOAD; }
return false;
default: private boolean sawOpcodeAfterLoad(int seen) {
return true; switch (seen) {
} case Const.ALOAD:
} case Const.ALOAD_0:

case Const.ALOAD_1:
private boolean sawOpcodeAfterLoad(int seen) { case Const.ALOAD_2:
switch (seen) { case Const.ALOAD_3:
case ALOAD: if (!sawField && beanReference1.equals(String.valueOf(getRegisterOperand()))) {
case ALOAD_0: state = State.SEEN_DUAL_LOADS;
case ALOAD_1: return false;
case ALOAD_2: }
case ALOAD_3: break;
if (!sawField && beanReference1.equals(String.valueOf(getRegisterOperand()))) {
state = State.SEEN_DUAL_LOADS; case Const.GETFIELD: {
return false; if (sawField) {
} beanReference2 += ':' + getNameConstantOperand();
break; if (beanReference1.equals(beanReference2)) {

state = State.SEEN_DUAL_LOADS;
case GETFIELD: { return false;
if (sawField) { }
beanReference2 += ':' + getNameConstantOperand(); } else {
if (beanReference1.equals(beanReference2)) { state = State.SEEN_GETFIELD;
state = State.SEEN_DUAL_LOADS; beanReference1 += ':' + getNameConstantOperand();
return false; sawField = true;
} return false;
} else { }
state = State.SEEN_GETFIELD; }
beanReference1 += ':' + getNameConstantOperand(); break;
sawField = true;
return false; default:
} break;
} }
break; return true;

}
default:
break; private boolean sawOpcodeAfterGetField(int seen) {
}
return true; if (!OpcodeUtils.isALoad(seen)) {
} return true;

}
private boolean sawOpcodeAfterGetField(int seen) {
switch (seen) { beanReference2 = String.valueOf(getRegisterOperand());
case ALOAD: state = State.SEEN_ALOAD;
case ALOAD_0: return false;
case ALOAD_1: }
case ALOAD_2:
case ALOAD_3: private boolean sawOpcodeAfterDualLoads(int seen) {
beanReference2 = String.valueOf(getRegisterOperand()); if (seen != Const.INVOKEVIRTUAL) {
state = State.SEEN_ALOAD; return true;
return false; }

String sig = getSigConstantOperand();
default: String noParams = SignatureBuilder.PARAM_NONE;
return true; if (!sig.startsWith(noParams)) {
} return true;
} }

propType = sig.substring(noParams.length());
private boolean sawOpcodeAfterDualLoads(int seen) { if (Values.SIG_VOID.equals(propType)) {
if (seen != INVOKEVIRTUAL) { return true;
return true; }
} String methodName = getNameConstantOperand();
String sig = getSigConstantOperand(); if (!methodName.startsWith("get")) {
String noParams = SignatureBuilder.PARAM_NONE; return true;
if (!sig.startsWith(noParams)) { }
return true; propName = methodName.substring("get".length());
} state = State.SEEN_INVOKEVIRTUAL;
propType = sig.substring(noParams.length()); return false;
if (Values.SIG_VOID.equals(propType)) { }
return true;
} private void checkForSGSU() {
String methodName = getNameConstantOperand(); if (!getSigConstantOperand().equals('(' + propType + ")V")) {
if (!methodName.startsWith("get")) { return;
return true; }
} String name = getNameConstantOperand();
propName = methodName.substring("get".length()); if (name.startsWith("set") && propName.equals(name.substring("set".length()))) {
state = State.SEEN_INVOKEVIRTUAL; bugReporter
return false; .reportBug(new BugInstance(this, BugType.SGSU_SUSPICIOUS_GETTER_SETTER_USE.name(), NORMAL_PRIORITY)
} .addClass(this).addMethod(this).addSourceLine(this));

}
private void checkForSGSU() { }
if (!getSigConstantOperand().equals('(' + propType + ")V")) {
return;
}
String name = getNameConstantOperand();
if (name.startsWith("set") && propName.equals(name.substring("set".length()))) {
bugReporter.reportBug(new BugInstance(this, BugType.SGSU_SUSPICIOUS_GETTER_SETTER_USE.name(), NORMAL_PRIORITY).addClass(this)
.addMethod(this).addSourceLine(this));
}
}


} }

0 comments on commit b114125

Please sign in to comment.