Add failing test case for class generated by Groovy compiler

CHANGELOG.md

@@ -8,6 +8,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 ### Fixed
 - Bumped gson from 2.9.0 to 2.9.1 ([#2136](https://github.com/spotbugs/spotbugs/pull/2136))
 - Fixed InvalidInputException in Eclipse while bug reporting ([#2134](https://github.com/spotbugs/spotbugs/issues/2134))
+- 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

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

@@ -21,10 +21,14 @@
 
 import static org.junit.Assert.assertTrue;
 
-import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Arrays;
-import java.util.stream.Collectors;
+import java.util.List;
 
 import edu.umd.cs.findbugs.internalAnnotations.SlashedClassName;
 import edu.umd.cs.findbugs.test.AnalysisRunner;
@@ -64,40 +68,36 @@
 public abstract class AbstractIntegrationTest {
 
     /**
-     * Build path if running command line build
+     * Build prefix directories to search for classes
      */
-    private static final String BUILD_CLASSES_CLI = "/build/classes/java/main/";
-
-    /**
-     * Build path if running in Eclipse
-     */
-    private static final String BUILD_CLASSES_ECLIPSE = "/classesEclipse/";
+    private static final List<Path> BUILD_CLASS_SEARCH_PREFIXES = List.of(
+            // Build path if running command line build
+            Paths.get("build/classes/java/main"),
+            Paths.get("build/classes/groovy/main"),
+            // Build path if running in Eclipse
+            Paths.get("classesEclipse"));
 
     private BugCollectionBugReporter bugReporter;
 
-    private static File getFindbugsTestCases() {
-        final File f = new File(SystemProperties.getProperty("spotbugsTestCases.home", "../spotbugsTestCases"));
-        assertTrue("'spotbugsTestCases' directory not found", f.exists());
-        assertTrue(f.isDirectory());
-        assertTrue(f.canRead());
+    private static Path getFindbugsTestCases() {
+        final Path p = Paths.get(SystemProperties.getProperty("spotbugsTestCases.home", "../spotbugsTestCases"));
+        assertTrue("'spotbugsTestCases' directory not found", Files.exists(p));
+        assertTrue(Files.isDirectory(p));
+        assertTrue(Files.isReadable(p));
 
-        return f;
+        return p;
     }
 
-    private static File getFindbugsTestCasesFile(final String path) {
-        File f = new File(getFindbugsTestCases(), path);
-        if (!f.exists() && path.startsWith(BUILD_CLASSES_CLI)) {
-            String replaced = path.replace(BUILD_CLASSES_CLI, BUILD_CLASSES_ECLIPSE);
-            replaced = replaced.replace("../java9/", "");
-            File f2 = new File(getFindbugsTestCases(), replaced);
-            if (f2.exists()) {
-                f = f2;
-            }
-        }
-        assertTrue(f.getAbsolutePath() + " not found", f.exists());
-        assertTrue(f.getAbsolutePath() + " is not readable", f.canRead());
+    private static Path getFindbugsTestCasesFile(final String path) {
+        final Path root = getFindbugsTestCases();
+        final Path p = BUILD_CLASS_SEARCH_PREFIXES.stream()
+                .map(prefix -> root.resolve(prefix).resolve(path))
+                .filter(Files::exists)
+                .findFirst()
+                .orElseThrow(() -> new AssertionError(path + " not found"));
 
-        return f;
+        assertTrue(p + " is not readable", Files.isReadable(p));
+        return p;
     }
 
     protected BugCollection getBugCollection() {
@@ -112,19 +112,21 @@ protected BugCollection getBugCollection() {
     protected void performAnalysis(@SlashedClassName final String... analyzeMe) {
         AnalysisRunner runner = new AnalysisRunner();
 
-        final File lib = getFindbugsTestCasesFile("lib");
-        for (final File f : lib.listFiles()) {
-            final String path = f.getPath();
-            if (f.canRead() && path.endsWith(".jar")) {
-                runner.addAuxClasspathEntry(f.toPath());
+        final Path lib = getFindbugsTestCases().resolve("lib");
+        try (DirectoryStream<Path> ds = Files.newDirectoryStream(lib, "*.jar")) {
+            for (Path jar : ds) {
+                if (Files.isReadable(jar)) {
+                    runner.addAuxClasspathEntry(jar);
+                }
             }
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
         }
 
         // TODO : Unwire this once we move bug samples to a proper sourceset
         Path[] paths = Arrays.stream(analyzeMe)
-                .map(s -> getFindbugsTestCasesFile(BUILD_CLASSES_CLI + s).toPath())
-                .collect(Collectors.toList())
-                .toArray(new Path[analyzeMe.length]);
+                .map(AbstractIntegrationTest::getFindbugsTestCasesFile)
+                .toArray(Path[]::new);
         bugReporter = runner.run(paths);
     }
 }

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue2145Test.java

@@ -0,0 +1,19 @@
+package edu.umd.cs.findbugs.ba;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.SortedBugCollection;
+import org.junit.Test;
+
+public class Issue2145Test extends AbstractIntegrationTest {
+
+    @Test
+    public void test() {
+        performAnalysis("ghIssues/Issue2145.class");
+        SortedBugCollection bugCollection = (SortedBugCollection) getBugCollection();
+        assertThat(bugCollection.getErrors(), hasSize(0));
+    }
+
+}

spotbugs/src/main/java/edu/umd/cs/findbugs/ba/BetterCFGBuilder2.java

@@ -35,40 +35,7 @@
 import org.apache.bcel.classfile.ClassParser;
 import org.apache.bcel.classfile.JavaClass;
 import org.apache.bcel.classfile.Method;
-import org.apache.bcel.generic.ACONST_NULL;
-import org.apache.bcel.generic.ALOAD;
-import org.apache.bcel.generic.BranchInstruction;
-import org.apache.bcel.generic.ClassGen;
-import org.apache.bcel.generic.CodeExceptionGen;
-import org.apache.bcel.generic.ConstantPoolGen;
-import org.apache.bcel.generic.ExceptionThrower;
-import org.apache.bcel.generic.GETFIELD;
-import org.apache.bcel.generic.GETSTATIC;
-import org.apache.bcel.generic.GOTO;
-import org.apache.bcel.generic.GotoInstruction;
-import org.apache.bcel.generic.ICONST;
-import org.apache.bcel.generic.IFNONNULL;
-import org.apache.bcel.generic.IFNULL;
-import org.apache.bcel.generic.IF_ACMPEQ;
-import org.apache.bcel.generic.IF_ACMPNE;
-import org.apache.bcel.generic.INSTANCEOF;
-import org.apache.bcel.generic.INVOKESTATIC;
-import org.apache.bcel.generic.IfInstruction;
-import org.apache.bcel.generic.Instruction;
-import org.apache.bcel.generic.InstructionHandle;
-import org.apache.bcel.generic.InstructionList;
-import org.apache.bcel.generic.InstructionTargeter;
-import org.apache.bcel.generic.JsrInstruction;
-import org.apache.bcel.generic.LDC;
-import org.apache.bcel.generic.MONITOREXIT;
-import org.apache.bcel.generic.MethodGen;
-import org.apache.bcel.generic.NEW;
-import org.apache.bcel.generic.NOP;
-import org.apache.bcel.generic.POP;
-import org.apache.bcel.generic.POP2;
-import org.apache.bcel.generic.PUTFIELD;
-import org.apache.bcel.generic.PUTSTATIC;
-import org.apache.bcel.generic.ReturnInstruction;
+import org.apache.bcel.generic.*;
 
 import edu.umd.cs.findbugs.SystemProperties;
 import edu.umd.cs.findbugs.ba.type.ExceptionSetFactory;
@@ -1010,6 +977,7 @@ private boolean isPEI(InstructionHandle handle) throws CFGBuilderException {
         if (ins instanceof PUTFIELD && !methodGen.isStatic()) {
             // Assume that PUTFIELD on this object is not PEI
             int depth = ins.consumeStack(cpg);
+
             for (InstructionHandle prev = handle.getPrev(); prev != null; prev = prev.getPrev()) {
                 Instruction prevInst = prev.getInstruction();
                 if (prevInst instanceof BranchInstruction) {
@@ -1027,11 +995,19 @@ private boolean isPEI(InstructionHandle handle) throws CFGBuilderException {
                         return true;
                     }
                 }
-                depth = depth - prevInst.produceStack(cpg) + prevInst.consumeStack(cpg);
+                if (prevInst instanceof DUP_X2 && depth == 4) {
+                    depth = 1;
+                } else if (prevInst instanceof DUP_X1 && depth == 3) {
+                    depth = 1;
+                } else {
+                    depth = depth - prevInst.produceStack(cpg) + prevInst.consumeStack(cpg);
+                }
                 if (depth < 1) {
                     throw new CFGBuilderException("Invalid stack at " + prev + " when checking " + handle);
                 }
                 if (depth == 1) {
+                    // now we assume that the previous instruction (prevPrev) is the opcode
+                    // that pushes the reference to the target of PUTFIELD
                     InstructionHandle prevPrev = prev.getPrev();
                     if (prevPrev != null && prevPrev.getInstruction() instanceof BranchInstruction) {
                         continue;

spotbugsTestCases/build.gradle

@@ -1,8 +1,15 @@
+plugins {
+  id 'groovy'
+}
+
 sourceSets {
   main {
     java {
       srcDirs = ['src/java', 'src/fakeAnnotations', 'src/fakeLibraries']
     }
+    groovy {
+      srcDirs = ['src/groovy']
+    }
   }
 }
 
@@ -31,6 +38,7 @@ dependencies {
 
   implementation project(':spotbugs')
   api project(':spotbugs-annotations')
+  implementation 'org.apache.groovy:groovy-all:4.0.4'
 }
 
 tasks.withType(JavaCompile).configureEach {

spotbugsTestCases/src/groovy/ghIssues/Issue2145.groovy

@@ -0,0 +1,8 @@
+package ghIssues
+
+class Issue2145 {
+    final double field1
+    Issue2145(double field1) {
+        this.field1 = field1
+    }
+}