fix(deps): update dependency org.apache.bcel:bcel to v6.8.0 (#2756)

* fix(deps): update dependency org.apache.bcel:bcel to v6.8.0 * Fixes for the bcel update (#2757) * Partial fix org.apache.bcel:bcel dependency update (#2446) * Refactor deprecated tests to the new type * Follow the changes in ConstantPool --------- Co-authored-by: Judit Knoll <[email protected]> * migrate the tests to junit jupiter --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Judit Knoll <[email protected]>
spotbugs · Dec 9, 2023 · a7aada2 · a7aada2
1 parent c176966
commit a7aada2
Show file tree

Hide file tree

Showing 13 changed files with 230 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 
 ### Fixed
 - Fix FP in CT_CONSTRUCTOR_THROW when the finalizer does not run, since the exception is thrown before java.lang.Object's constructor exits for checked exceptions ([#2710](https://github.com/spotbugs/spotbugs/issues/2710))
+- Applied changes for bcel 6.8.0 with adjustments to constant pool ([#2756](https://github.com/spotbugs/spotbugs/pull/2756))
+    - More information bcel changes can be found on ([#2757(https://github.com/spotbugs/spotbugs/pull/2757))
 
 ### Changed
 - Improved Matcher checks for empty strings ([#2755](https://github.com/spotbugs/spotbugs/pull/2755))

diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Bug3107916Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Bug3107916Test.java
@@ -0,0 +1,56 @@
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+import org.junit.jupiter.api.Test;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItem;
+
+class Bug3107916Test extends AbstractIntegrationTest {
+    @Test
+    void test() {
+        performAnalysis("sfBugs/Bug3107916.class");
+
+        assertNpBugInMethod("doCompare", 20);
+        assertNoNpBugInMethod("doCompare2");
+    }
+
+    private void assertNoNpBugInMethod(String method) {
+        final String[] bugtypes = new String[] {
+            "NP_SYNC_AND_NULL_CHECK_FIELD", "NP_BOOLEAN_RETURN_NULL", "NP_OPTIONAL_RETURN_NULL",
+            "NP_NONNULL_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", "NP_ARGUMENT_MIGHT_BE_NULL",
+            "NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT", "NP_DEREFERENCE_OF_READLINE_VALUE",
+            "NP_IMMEDIATE_DEREFERENCE_OF_READLINE", "NP_UNWRITTEN_FIELD", "NP_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD",
+            "NP_ALWAYS_NULL", "NP_CLOSING_NULL", "NP_STORE_INTO_NONNULL_FIELD", "NP_ALWAYS_NULL_EXCEPTION",
+            "NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE", "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE",
+            "NP_NULL_ON_SOME_PATH_MIGHT_BE_INFEASIBLE", "NP_NULL_ON_SOME_PATH", "NP_NULL_ON_SOME_PATH_EXCEPTION",
+            "NP_NULL_PARAM_DEREF", "NP_NULL_PARAM_DEREF_NONVIRTUAL", "NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS",
+            "NP_NONNULL_PARAM_VIOLATION", "NP_NONNULL_RETURN_VIOLATION", "NP_TOSTRING_COULD_RETURN_NULL",
+            "NP_CLONE_COULD_RETURN_NULL", "NP_LOAD_OF_KNOWN_NULL_VALUE", "NP_GUARANTEED_DEREF",
+            "NP_GUARANTEED_DEREF_ON_EXCEPTION_PATH", "NP_NULL_INSTANCEOF", "BC_NULL_INSTANCEOF",
+            "NP_METHOD_RETURN_RELAXING_ANNOTATION", "NP_METHOD_PARAMETER_TIGHTENS_ANNOTATION",
+            "NP_METHOD_PARAMETER_RELAXING_ANNOTATION",
+        };
+        for (String bugtype : bugtypes) {
+            final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                    .bugType(bugtype)
+                    .inClass("Bug3107916")
+                    .inMethod(method)
+                    .build();
+            assertThat(getBugCollection(), containsExactly(0, bugInstanceMatcher));
+        }
+    }
+
+    private void assertNpBugInMethod(String method, int line) {
+        final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                .bugType("NP_NULL_ON_SOME_PATH_MIGHT_BE_INFEASIBLE")
+                .inClass("Bug3107916")
+                .inMethod(method)
+                .atLine(line)
+                .build();
+        assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
+    }
+}
diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Bug3277814Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Bug3277814Test.java
@@ -0,0 +1,56 @@
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+import org.junit.jupiter.api.Test;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItem;
+
+class Bug3277814Test extends AbstractIntegrationTest {
+    @Test
+    void test() {
+        performAnalysis("sfBugs/Bug3277814.class");
+
+        assertNoNpBugInMethod("test");
+        assertNpBugInMethod("test2", 44);
+    }
+
+    private void assertNoNpBugInMethod(String method) {
+        final String[] bugtypes = new String[] {
+            "NP_SYNC_AND_NULL_CHECK_FIELD", "NP_BOOLEAN_RETURN_NULL", "NP_OPTIONAL_RETURN_NULL",
+            "NP_NONNULL_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", "NP_ARGUMENT_MIGHT_BE_NULL",
+            "NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT", "NP_DEREFERENCE_OF_READLINE_VALUE",
+            "NP_IMMEDIATE_DEREFERENCE_OF_READLINE", "NP_UNWRITTEN_FIELD", "NP_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD",
+            "NP_ALWAYS_NULL", "NP_CLOSING_NULL", "NP_STORE_INTO_NONNULL_FIELD", "NP_ALWAYS_NULL_EXCEPTION",
+            "NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE", "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE",
+            "NP_NULL_ON_SOME_PATH_MIGHT_BE_INFEASIBLE", "NP_NULL_ON_SOME_PATH", "NP_NULL_ON_SOME_PATH_EXCEPTION",
+            "NP_NULL_PARAM_DEREF", "NP_NULL_PARAM_DEREF_NONVIRTUAL", "NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS",
+            "NP_NONNULL_PARAM_VIOLATION", "NP_NONNULL_RETURN_VIOLATION", "NP_TOSTRING_COULD_RETURN_NULL",
+            "NP_CLONE_COULD_RETURN_NULL", "NP_LOAD_OF_KNOWN_NULL_VALUE", "NP_GUARANTEED_DEREF",
+            "NP_GUARANTEED_DEREF_ON_EXCEPTION_PATH", "NP_NULL_INSTANCEOF", "BC_NULL_INSTANCEOF",
+            "NP_METHOD_RETURN_RELAXING_ANNOTATION", "NP_METHOD_PARAMETER_TIGHTENS_ANNOTATION",
+            "NP_METHOD_PARAMETER_RELAXING_ANNOTATION",
+        };
+        for (String bugtype : bugtypes) {
+            final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                    .bugType(bugtype)
+                    .inClass("Bug3277814")
+                    .inMethod(method)
+                    .build();
+            assertThat(getBugCollection(), containsExactly(0, bugInstanceMatcher));
+        }
+    }
+
+    private void assertNpBugInMethod(String method, int line) {
+        final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                .bugType("NP_NULL_ON_SOME_PATH")
+                .inClass("Bug3277814")
+                .inMethod(method)
+                .atLine(line)
+                .build();
+        assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
+    }
+}
diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/RegressionIdeas2008091415Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/RegressionIdeas2008091415Test.java
@@ -0,0 +1,30 @@
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+import org.junit.jupiter.api.Test;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+class RegressionIdeas2008091415Test extends AbstractIntegrationTest {
+    @Test
+    void test() {
+        performAnalysis("bugIdeas/Ideas_2008_09_14.class", "bugIdeas/Ideas_2008_09_15.class");
+
+        assertBugInMethod("Ideas_2008_09_14", "foo", 6);
+        assertBugInMethod("Ideas_2008_09_15", "alternativesToInstanceof", 6);
+        assertBugInMethod("Ideas_2008_09_15", "alternativesToInstanceofAndCheckedCast", 12);
+    }
+
+    private void assertBugInMethod(String className, String method, int line) {
+        final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                .bugType("BC_IMPOSSIBLE_CAST")
+                .inClass(className)
+                .inMethod(method)
+                .atLine(line)
+                .build();
+        assertThat(getBugCollection(), containsExactly(1, bugInstanceMatcher));
+    }
+}
diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/RegressionIdeas20111219Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/RegressionIdeas20111219Test.java
@@ -0,0 +1,65 @@
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+import org.junit.jupiter.api.Test;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItem;
+
+class RegressionIdeas20111219Test extends AbstractIntegrationTest {
+    @Test
+    void test() {
+        performAnalysis("bugIdeas/Ideas_2011_12_19.class");
+
+        assertNoNpBugInMethod("f");
+        assertBugInMethod("NP_ALWAYS_NULL", "f2", 21);
+        assertBugInMethod("NP_LOAD_OF_KNOWN_NULL_VALUE", "f2", 21);
+        assertNoNpBugInMethod("f3");
+        assertNoNpBugInMethod("f4");
+        assertBugInMethod("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", "f5a", 48);
+        assertNoNpBugInMethod("f5b");
+        assertNoNpBugInMethod("f5c");
+        assertNoNpBugInMethod("f5e");
+        assertNoNpBugInMethod("f5f");
+        assertBugInMethod("NP_NULL_ON_SOME_PATH", "f5g", 81);
+    }
+
+    private void assertNoNpBugInMethod(String method) {
+        final String[] bugtypes = new String[] {
+            "NP_SYNC_AND_NULL_CHECK_FIELD", "NP_BOOLEAN_RETURN_NULL", "NP_OPTIONAL_RETURN_NULL",
+            "NP_NONNULL_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", "NP_ARGUMENT_MIGHT_BE_NULL",
+            "NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT", "NP_DEREFERENCE_OF_READLINE_VALUE",
+            "NP_IMMEDIATE_DEREFERENCE_OF_READLINE", "NP_UNWRITTEN_FIELD", "NP_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD",
+            "NP_ALWAYS_NULL", "NP_CLOSING_NULL", "NP_STORE_INTO_NONNULL_FIELD", "NP_ALWAYS_NULL_EXCEPTION",
+            "NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE", "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE",
+            "NP_NULL_ON_SOME_PATH_MIGHT_BE_INFEASIBLE", "NP_NULL_ON_SOME_PATH", "NP_NULL_ON_SOME_PATH_EXCEPTION",
+            "NP_NULL_PARAM_DEREF", "NP_NULL_PARAM_DEREF_NONVIRTUAL", "NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS",
+            "NP_NONNULL_PARAM_VIOLATION", "NP_NONNULL_RETURN_VIOLATION", "NP_TOSTRING_COULD_RETURN_NULL",
+            "NP_CLONE_COULD_RETURN_NULL", "NP_LOAD_OF_KNOWN_NULL_VALUE", "NP_GUARANTEED_DEREF",
+            "NP_GUARANTEED_DEREF_ON_EXCEPTION_PATH", "NP_NULL_INSTANCEOF", "BC_NULL_INSTANCEOF",
+            "NP_METHOD_RETURN_RELAXING_ANNOTATION", "NP_METHOD_PARAMETER_TIGHTENS_ANNOTATION",
+            "NP_METHOD_PARAMETER_RELAXING_ANNOTATION",
+        };
+        for (String bugtype : bugtypes) {
+            final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                    .bugType(bugtype)
+                    .inClass("Bug3277814")
+                    .inMethod(method)
+                    .build();
+            assertThat(getBugCollection(), containsExactly(0, bugInstanceMatcher));
+        }
+    }
+
+    private void assertBugInMethod(String bugtype, String method, int line) {
+        final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                .bugType(bugtype)
+                .inClass("Ideas_2011_12_19")
+                .inMethod(method)
+                .atLine(line)
+                .build();
+        assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
+    }
+}
diff --git a/spotbugs/build.gradle b/spotbugs/build.gradle
@@ -67,7 +67,7 @@ dependencies {
   api libs.asm.commons
   api libs.asm.tree
   api libs.asm.util
-  api 'org.apache.bcel:bcel:6.6.1'
+  api 'org.apache.bcel:bcel:6.8.0'
   api 'com.github.stephenc.jcip:jcip-annotations:1.0-1'
   api('org.dom4j:dom4j:2.1.4') {
     // exclude transitive dependencies to keep compatible with dom4j 2.1.1

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
@@ -27,6 +27,7 @@
 
 import org.apache.bcel.Const;
 import org.apache.bcel.classfile.Constant;
+import org.apache.bcel.classfile.ConstantPool;
 import org.apache.bcel.classfile.ConstantInterfaceMethodref;
 import org.apache.bcel.classfile.ConstantMethodref;
 import org.apache.bcel.classfile.JavaClass;
@@ -301,26 +302,32 @@ public Object clone() {
      */
     @Override
     public void visitClassContext(ClassContext classContext) {
-        JavaClass jclass = classContext.getJavaClass();
+        ConstantPool cp = classContext.getJavaClass().getConstantPool();
 
         // Check to see if the class references any other classes
         // which could be resources we want to track.
         // If we don't find any such classes, we skip analyzing
         // the class. (Note: could do this by method.)
         boolean sawResourceClass = false;
-        for (int i = 0; i < jclass.getConstantPool().getLength(); ++i) {
-            Constant constant = jclass.getConstantPool().getConstant(i);
+        for (int i = 1; i < cp.getLength(); ++i) {
+            Constant constant = cp.getConstant(i);
+            // Quote from the JVM specification: "All eight byte constants take up two spots in the constant pool.
+            // If this is the n'th byte in the constant pool, then the next item will be numbered n+2"
+            // So the indices after CONSTANT_Double and CONSTANT_Long are null, not used and throw ClassFormatException
+            if (constant.getTag() == Const.CONSTANT_Double || constant.getTag() == Const.CONSTANT_Long) {
+                i++;
+            }
             String className = null;
             if (constant instanceof ConstantMethodref) {
                 ConstantMethodref cmr = (ConstantMethodref) constant;
 
                 int classIndex = cmr.getClassIndex();
-                className = jclass.getConstantPool().getConstantString(classIndex, Const.CONSTANT_Class);
+                className = cp.getConstantString(classIndex, Const.CONSTANT_Class);
             } else if (constant instanceof ConstantInterfaceMethodref) {
                 ConstantInterfaceMethodref cmr = (ConstantInterfaceMethodref) constant;
 
                 int classIndex = cmr.getClassIndex();
-                className = jclass.getConstantPool().getConstantString(classIndex, Const.CONSTANT_Class);
+                className = cp.getConstantString(classIndex, Const.CONSTANT_Class);
             }
 
             if (className != null) {

diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/visitclass/Util.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/visitclass/Util.java
@@ -122,10 +122,14 @@ public static int getSizeOfSurroundingTryBlock(ConstantPool constantPool, Code c
         }
         for (CodeException catchBlock : code.getExceptionTable()) {
             if (vmNameOfExceptionClass != null) {
-                Constant catchType = constantPool.getConstant(catchBlock.getCatchType());
-                if (catchType == null && !vmNameOfExceptionClass.isEmpty() || catchType instanceof ConstantClass
-                        && !((ConstantClass) catchType).getBytes(constantPool).equals(vmNameOfExceptionClass)) {
+                if (catchBlock.getCatchType() == 0) {
                     continue;
+                } else {
+                    Constant catchType = constantPool.getConstant(catchBlock.getCatchType());
+                    if (catchType == null && !vmNameOfExceptionClass.isEmpty() || catchType instanceof ConstantClass
+                            && !((ConstantClass) catchType).getBytes(constantPool).equals(vmNameOfExceptionClass)) {
+                        continue;
+                    }
                 }
             }
             int startPC = catchBlock.getStartPC();

diff --git a/spotbugsTestCases/src/java/bugIdeas/Ideas_2008_09_14.java b/spotbugsTestCases/src/java/bugIdeas/Ideas_2008_09_14.java
@@ -1,10 +1,6 @@
 package bugIdeas;
 
-import edu.umd.cs.findbugs.annotations.ExpectWarning;
-
 public class Ideas_2008_09_14 {
-
-    @ExpectWarning(value="BC_IMPOSSIBLE_CAST", num = 1)
     public String foo(Object o) {
         if (Integer.class.isInstance(o))
             return (String) o;

diff --git a/spotbugsTestCases/src/java/bugIdeas/Ideas_2008_09_15.java b/spotbugsTestCases/src/java/bugIdeas/Ideas_2008_09_15.java
@@ -1,17 +1,12 @@
 package bugIdeas;
 
-import edu.umd.cs.findbugs.annotations.ExpectWarning;
-
 public class Ideas_2008_09_15 {
-
-    @ExpectWarning(value="BC_IMPOSSIBLE_CAST", num = 1)
     public String alternativesToInstanceof(Object x) {
         if (Integer.class.isInstance(x))
             return (String) x;
         return "";
     }
 
-    @ExpectWarning(value="BC_IMPOSSIBLE_CAST", num = 1)
     public String alternativesToInstanceofAndCheckedCast(Object x) {
         if (Integer.class.isInstance(x))
             return String.class.cast(x);