Fix for redundant assertion in ValueRangeAnalysisFactory

CHANGELOG.md

@@ -25,7 +25,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 - Do not report DLS_DEAD_LOCAL_STORE for Hibernate bytecode enhancements ([#2865](https://github.com/spotbugs/spotbugs/pull/2865))
 - Fixed NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE false positives due to source code formatting ([#2874](https://github.com/spotbugs/spotbugs/pull/2874))
 - Added more nullability annotations in TypeQualifierResolver ([#2558](https://github.com/spotbugs/spotbugs/issues/2558) [#2694](https://github.com/spotbugs/spotbugs/pull/2694))
-
+- Fixed crash in ValueRangeAnalysisFactory when looking for redundant conditions used in assertions [#2887](https://github.com/spotbugs/spotbugs/pull/2887))
 ### Added
 - New detector `MultipleInstantiationsOfSingletons` and introduced new bug types:
   - `SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR` is reported in case of a non-private constructor,

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue608Test.java

@@ -0,0 +1,15 @@
+package edu.umd.cs.findbugs.detect;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+
+import org.junit.jupiter.api.Test;
+
+class Issue608Test extends AbstractIntegrationTest {
+
+    @Test
+    void testIssue() {
+        assertDoesNotThrow(() -> performAnalysis("ghIssues/Issue608.class"));
+    }
+}

spotbugs/src/main/java/edu/umd/cs/findbugs/classfile/engine/bcel/ValueRangeAnalysisFactory.java

@@ -38,6 +38,7 @@
 import org.apache.bcel.generic.ARRAYLENGTH;
 import org.apache.bcel.generic.CPInstruction;
 import org.apache.bcel.generic.ConstantPushInstruction;
+import org.apache.bcel.generic.ATHROW;
 import org.apache.bcel.generic.GETFIELD;
 import org.apache.bcel.generic.GETSTATIC;
 import org.apache.bcel.generic.IFNE;
@@ -772,7 +773,7 @@ public ValueRangeAnalysis analyze(IAnalysisCache analysisCache, MethodDescriptor
                         GETSTATIC getStatic = (GETSTATIC) ih.getInstruction();
                         if ("$assertionsDisabled".equals(getStatic.getFieldName(methodGen.getConstantPool()))
                                 && "Z".equals(getStatic.getSignature(methodGen.getConstantPool()))) {
-                            int end = ((IFNE) next).getTarget().getPosition();
+                            int end = findEndOfAssertBlock(ih);
                             assertionBlocks.set(ih.getNext().getPosition(), end);
                         }
                     }
@@ -794,6 +795,40 @@ public ValueRangeAnalysis analyze(IAnalysisCache analysisCache, MethodDescriptor
         return null;
     }
 
+    /**
+     * Expecting an assert block to look like this:
+     *<code>
+     * 6  getstatic ghIssues.Issue608.$assertionsDisabled : boolean [7]<br>
+     * 9  ifne 0<br>
+     * 12  iload_1 [i]  // Loading some variable named "i"<br>
+     * 13  bipush 12    // Loading a constant 12<br>
+     * 15  if_icmplt 0  // Comparing i and 12<br>
+     * 18  new java.lang.AssertionError [13]<br>
+     * 21  dup<br>
+     * 22  ldc <String "assertion failure message"> [15]<br>
+     * 24  invokespecial java.lang.AssertionError(java.lang.Object) [17]<br>
+     * 27  athrow<br>
+     *</code>
+     * @param ih The InstructionHandle corresponding to the <code>ifne 0</code> in the above sample
+     * @return The position for the final <code>athrow</code>
+     */
+    private int findEndOfAssertBlock(InstructionHandle ih) {
+        InstructionHandle next = ih.getNext();
+        int end = ih.getPosition();
+
+        while (next != null && !(next.getInstruction() instanceof ATHROW)) {
+            end = next.getPosition();
+
+            next = next.getNext();
+        }
+
+        if (next != null) {
+            return next.getPosition();
+        }
+
+        return end;
+    }
+
     private static Location getLocation(BasicBlock block) {
         InstructionHandle handle = block.getFirstInstruction();
         if (handle == null) {

spotbugsTestCases/src/java/ghIssues/Issue608.java

@@ -0,0 +1,10 @@
+package ghIssues;
+
+public class Issue608 {
+
+	void methodThatFails(int i) {
+		while (i < 12) {
+			assert (i < 12) : "assertion failure message";
+		}
+	}
+}