diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b77352561d..b239362cd8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. - Bump up gson to 2.10 ([#2235](https://github.com/spotbugs/spotbugs/pull/2235)) - Allowed for large command line through writing arguments to file (UnionResults/UnionBugs2) - Use com.github.stephenc.jcip for jcip-annotations fixing #887 +- Rewrite some member in `ResourceValueFrame.java` to Enum([#2061](https://github.com/spotbugs/spotbugs/issues/2061)) ### Fixed - Fixed missing classes not in report if using IErrorLogger.reportMissingClass(ClassDescriptor) ([#219](https://github.com/spotbugs/spotbugs/issues/219)) @@ -80,6 +81,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. - Fixed CFGBuilderException thrown when `dup_x2` is used to swap the reference and wide-value (double, long) in the stack ([#2146](https://github.com/spotbugs/spotbugs/pull/2146)) ## 4.7.1 - 2022-06-26 + ### Fixed - Fixed False positives for `RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE` on try-with-resources with interface references ([#1931](https://github.com/spotbugs/spotbugs/issues/1931)) - Fixed NullPointerException thrown by detector `FindPotentialSecurityCheckBasedOnUntrustedSource` on Kotlin files. ([#2041](https://github.com/spotbugs/spotbugs/issues/2041)) diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueAnalysis.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueAnalysis.java index 8fb4b4c957e..22349859f9c 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueAnalysis.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueAnalysis.java @@ -102,10 +102,10 @@ public void meetInto(ResourceValueFrame fact, Edge edge, ResourceValueFrame resu return; } - if (fact.getStatus() == ResourceValueFrame.OPEN) { + if (fact.getStatus() == ResourceValueFrame.State.OPEN) { // If status is OPEN, downgrade to OPEN_ON_EXCEPTION_PATH tmpFact = modifyFrame(fact, null); - tmpFact.setStatus(ResourceValueFrame.OPEN_ON_EXCEPTION_PATH); + tmpFact.setStatus(ResourceValueFrame.State.OPEN_ON_EXCEPTION_PATH); } if (fact.isValid()) { @@ -122,7 +122,7 @@ public void meetInto(ResourceValueFrame fact, Edge edge, ResourceValueFrame resu && resourceTracker.isResourceClose(fallThroughSuccessor, exceptionThrower, methodGen.getConstantPool(), resource, fact)) { tmpFact = modifyFrame(fact, tmpFact); - tmpFact.setStatus(ResourceValueFrame.CLOSED); + tmpFact.setStatus(ResourceValueFrame.State.CLOSED); if (DEBUG) { System.out.print("(failed attempt to close)"); } @@ -192,7 +192,7 @@ else if (lastInSource instanceof IFNULL || lastInSource instanceof IFNONNULL) { if ((isNullCheck && edgeType == IFCMP_EDGE) || (isNonNullCheck && edgeType == FALL_THROUGH_EDGE)) { // System.out.println("**** making resource nonexistent on edge "+edge.getId()); tmpFact = modifyFrame(fact, tmpFact); - tmpFact.setStatus(ResourceValueFrame.NONEXISTENT); + tmpFact.setStatus(ResourceValueFrame.State.NONEXISTENT); } } } @@ -213,7 +213,7 @@ protected void mergeInto(ResourceValueFrame frame, ResourceValueFrame result) th super.mergeInto(frame, result); // Merge status - result.setStatus(Math.min(result.getStatus(), frame.getStatus())); + result.setStatus(result.getStatus().getType() > frame.getStatus().getType() ? frame.getStatus() : result.getStatus()); } @Override diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrame.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrame.java index fe8faf315ce..7471468cac3 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrame.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrame.java @@ -20,50 +20,63 @@ package edu.umd.cs.findbugs.ba; public class ResourceValueFrame extends Frame { - /** - * The resource escapes the method. - */ - public static final int ESCAPED = 0; - - /** - * The resource is open (or locked, etc) on paths that include only normal - * control flow. - */ - public static final int OPEN = 1; - - /** - * The resource is open (or locked, etc) on paths that include exception - * control flow. - */ - public static final int OPEN_ON_EXCEPTION_PATH = 2; - - /** - * The resource is closed (or unlocked, etc). - */ - public static final int CLOSED = 3; - - /** - * The resource has been created, but is not open. - */ - public static final int CREATED = 4; - - /** - * The resource doesn't exist. - */ - public static final int NONEXISTENT = 5; - - private int status; + + public enum State { + /** + * The resource escapes the method. + */ + ESCAPED(0), + + /** + * The resource is open (or locked, etc) on paths that include only normal + * control flow. + */ + OPEN(1), + + /** + * The resource is open (or locked, etc) on paths that include exception + * control flow. + */ + OPEN_ON_EXCEPTION_PATH(2), + + /** + * The resource is closed (or unlocked, etc). + */ + CLOSED(3), + + /** + * The resource has been created, but is not open. + */ + CREATED(4), + + /** + * The resource doesn't exist. + */ + NONEXISTENT(5); + + State(int type) { + this.type = type; + } + + private final int type; + + public int getType() { + return type; + } + } + + private State status; public ResourceValueFrame(int numSlots) { super(numSlots); - this.status = NONEXISTENT; + this.status = State.NONEXISTENT; } - public int getStatus() { + public State getStatus() { return status; } - public void setStatus(int status) { + public void setStatus(State status) { this.status = status; } @@ -89,7 +102,7 @@ public void copyFrom(Frame other_) { @Override public String toString() { - return super.toString() + statusList[status]; + return super.toString() + statusList[status.getType()]; } } diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrameModelingVisitor.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrameModelingVisitor.java index 8330ccdd038..aa5e2ce6548 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrameModelingVisitor.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/ResourceValueFrameModelingVisitor.java @@ -63,7 +63,7 @@ private void handleFieldStore(FieldInstruction ins) { ResourceValueFrame frame = getFrame(); ResourceValue topValue = frame.getTopValue(); if (topValue.equals(ResourceValue.instance())) { - frame.setStatus(ResourceValueFrame.ESCAPED); + frame.setStatus(ResourceValueFrame.State.ESCAPED); } } catch (DataflowAnalysisException e) { throw new InvalidBytecodeException("Stack underflow", e); @@ -86,7 +86,7 @@ private void handleArrayStore(ArrayInstruction ins) { ResourceValueFrame frame = getFrame(); ResourceValue topValue = frame.getTopValue(); if (topValue.equals(ResourceValue.instance())) { - frame.setStatus(ResourceValueFrame.ESCAPED); + frame.setStatus(ResourceValueFrame.State.ESCAPED); } } catch (DataflowAnalysisException e) { throw new InvalidBytecodeException("Stack underflow", e); @@ -135,7 +135,7 @@ private void handleInvoke(InvokeInstruction inv) { } if (instanceArgNum >= 0 && instanceEscapes(inv, instanceArgNum)) { - frame.setStatus(ResourceValueFrame.ESCAPED); + frame.setStatus(ResourceValueFrame.State.ESCAPED); } handleNormalInstruction(inv); @@ -150,7 +150,7 @@ public void visitCHECKCAST(CHECKCAST obj) { topValue = frame.getTopValue(); if (topValue.equals(ResourceValue.instance())) { - frame.setStatus(ResourceValueFrame.ESCAPED); + frame.setStatus(ResourceValueFrame.State.ESCAPED); } } catch (DataflowAnalysisException e) { AnalysisContext.logError("Analysis error", e); @@ -183,7 +183,7 @@ public void visitARETURN(ARETURN ins) { ResourceValueFrame frame = getFrame(); ResourceValue topValue = frame.getTopValue(); if (topValue.equals(ResourceValue.instance())) { - frame.setStatus(ResourceValueFrame.ESCAPED); + frame.setStatus(ResourceValueFrame.State.ESCAPED); } } catch (DataflowAnalysisException e) { throw new InvalidBytecodeException("Stack underflow", e); diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindOpenStream.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindOpenStream.java index 08f6124f5e7..91319cf0e9c 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindOpenStream.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindOpenStream.java @@ -496,21 +496,22 @@ public void inspectResult(ClassContext classContext, MethodGen methodGen, CFG cf } ResourceValueFrame exitFrame = dataflow.getResultFact(cfg.getExit()); - int exitStatus = exitFrame.getStatus(); - if (exitStatus == ResourceValueFrame.OPEN || exitStatus == ResourceValueFrame.OPEN_ON_EXCEPTION_PATH) { + ResourceValueFrame.State exitStatus = exitFrame.getStatus(); + if (exitStatus == ResourceValueFrame.State.OPEN || + exitStatus == ResourceValueFrame.State.OPEN_ON_EXCEPTION_PATH) { // FIXME: Stream object should be queried for the // priority. String bugType = stream.getBugType(); int priority = NORMAL_PRIORITY; - if (exitStatus == ResourceValueFrame.OPEN_ON_EXCEPTION_PATH) { + if (exitStatus == ResourceValueFrame.State.OPEN_ON_EXCEPTION_PATH) { bugType += "_EXCEPTION_PATH"; priority = LOW_PRIORITY; } potentialOpenStreamList.add(new PotentialOpenStream(bugType, priority, stream)); - } else if (exitStatus == ResourceValueFrame.CLOSED) { + } else if (exitStatus == ResourceValueFrame.State.CLOSED) { // Remember that this stream was closed on all paths. // Later, we will mark all of the streams in its equivalence class // as having been closed. diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindUnreleasedLock.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindUnreleasedLock.java index 3a1dac5c7c8..830e7be276f 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindUnreleasedLock.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindUnreleasedLock.java @@ -108,7 +108,8 @@ public void transferInstruction(InstructionHandle handle, BasicBlock basicBlock) final ConstantPoolGen cpg = getCPG(); final ResourceValueFrame frame = getFrame(); - int status = -1; + ResourceValueFrame.State status = ResourceValueFrame.State.NONEXISTENT; + boolean updated = false; if (DEBUG) { System.out.println("PC : " + handle.getPosition() + " " + ins); @@ -123,12 +124,14 @@ public void transferInstruction(InstructionHandle handle, BasicBlock basicBlock) // Is a lock acquired or released by this instruction? Location creationPoint = lock.getLocation(); if (handle == creationPoint.getHandle() && basicBlock == creationPoint.getBasicBlock()) { - status = ResourceValueFrame.OPEN; + status = ResourceValueFrame.State.OPEN; + updated = true; if (DEBUG) { System.out.println("OPEN"); } } else if (resourceTracker.isResourceClose(basicBlock, handle, cpg, lock, frame)) { - status = ResourceValueFrame.CLOSED; + status = ResourceValueFrame.State.CLOSED; + updated = true; if (DEBUG) { System.out.println("CLOSE"); } @@ -166,7 +169,7 @@ public void transferInstruction(InstructionHandle handle, BasicBlock basicBlock) } // If needed, update frame status - if (status != -1) { + if (updated) { frame.setStatus(status); } if (DEBUG) { @@ -431,12 +434,13 @@ public void inspectResult(ClassContext classContext, MethodGen methodGen, CFG cf if (DEBUG) { System.out.println("Resource value at exit: " + exitFrame); } - int exitStatus = exitFrame.getStatus(); + ResourceValueFrame.State exitStatus = exitFrame.getStatus(); - if (exitStatus == ResourceValueFrame.OPEN || exitStatus == ResourceValueFrame.OPEN_ON_EXCEPTION_PATH) { + if (exitStatus == ResourceValueFrame.State.OPEN || + exitStatus == ResourceValueFrame.State.OPEN_ON_EXCEPTION_PATH) { String bugType; int priority; - if (exitStatus == ResourceValueFrame.OPEN) { + if (exitStatus == ResourceValueFrame.State.OPEN) { bugType = "UL_UNRELEASED_LOCK"; priority = HIGH_PRIORITY; } else { diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/StreamFrameModelingVisitor.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/StreamFrameModelingVisitor.java index f52c5ec1c3e..14c59b2e899 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/StreamFrameModelingVisitor.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/StreamFrameModelingVisitor.java @@ -57,7 +57,8 @@ public void transferInstruction(InstructionHandle handle, BasicBlock basicBlock) final Instruction ins = handle.getInstruction(); final ResourceValueFrame frame = getFrame(); - int status = -1; + ResourceValueFrame.State status = ResourceValueFrame.State.NONEXISTENT; + boolean updated = false; boolean created = false; // Is a resource created, opened, or closed by this instruction? @@ -65,28 +66,31 @@ public void transferInstruction(InstructionHandle handle, BasicBlock basicBlock) if (handle == creationPoint.getHandle() && basicBlock == creationPoint.getBasicBlock()) { // Resource creation if (stream.isOpenOnCreation()) { - status = ResourceValueFrame.OPEN; + status = ResourceValueFrame.State.OPEN; stream.setOpenLocation(location); resourceTracker.addStreamOpenLocation(location, stream); } else { - status = ResourceValueFrame.CREATED; + status = ResourceValueFrame.State.CREATED; } + updated = true; created = true; } else if (resourceTracker.isResourceOpen(basicBlock, handle, cpg, stream, frame)) { // Resource opened - status = ResourceValueFrame.OPEN; + status = ResourceValueFrame.State.OPEN; + updated = true; stream.setOpenLocation(location); resourceTracker.addStreamOpenLocation(location, stream); } else if (resourceTracker.isResourceClose(basicBlock, handle, cpg, stream, frame)) { // Resource closed - status = ResourceValueFrame.CLOSED; + status = ResourceValueFrame.State.CLOSED; + updated = true; } // Model use of instance values in frame slots analyzeInstruction(ins); // If needed, update frame status - if (status != -1) { + if (updated) { frame.setStatus(status); if (created) { frame.setValue(frame.getNumSlots() - 1, ResourceValue.instance());