Add -AresourceLeakIgnoredExceptions option (#6242)

Author: Calvin-L

checker/src/main/java/org/checkerframework/checker/calledmethods/CalledMethodsAnalysis.java

@@ -2,9 +2,9 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.sun.tools.javac.code.Type;
-import java.util.HashSet;
 import java.util.Set;
 import javax.lang.model.type.TypeMirror;
+import org.checkerframework.checker.signature.qual.CanonicalName;
 import org.checkerframework.common.accumulation.AccumulationAnalysis;
 import org.checkerframework.common.basetype.BaseTypeChecker;
 
@@ -18,17 +18,16 @@ public class CalledMethodsAnalysis extends AccumulationAnalysis {
    * The fully-qualified names of the exception types that are ignored by this checker when
    * computing dataflow stores.
    */
-  protected static final Set<String> ignoredExceptionTypes =
-      new HashSet<>(
-          ImmutableSet.of(
-              // Use the Nullness Checker instead.
-              NullPointerException.class.getCanonicalName(),
-              // Ignore run-time errors, which cannot be predicted statically. Doing
-              // so is unsound in the sense that they could still occur - e.g., the
-              // program could run out of memory - but if they did, the checker's
-              // results would be useless anyway.
-              Error.class.getCanonicalName(),
-              RuntimeException.class.getCanonicalName()));
+  protected static final Set<@CanonicalName String> ignoredExceptionTypes =
+      ImmutableSet.of(
+          // Use the Nullness Checker instead.
+          NullPointerException.class.getCanonicalName(),
+          // Ignore run-time errors, which cannot be predicted statically. Doing
+          // so is unsound in the sense that they could still occur - e.g., the
+          // program could run out of memory - but if they did, the checker's
+          // results would be useless anyway.
+          Error.class.getCanonicalName(),
+          RuntimeException.class.getCanonicalName());
 
   /**
    * Creates a new {@code CalledMethodsAnalysis}.

checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java

@@ -10,8 +10,6 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
-import com.sun.tools.javac.code.Type;
-import java.io.UnsupportedEncodingException;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -34,7 +32,6 @@
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
-import javax.lang.model.element.Name;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.TypeKind;
@@ -48,8 +45,6 @@
 import org.checkerframework.checker.mustcall.qual.NotOwning;
 import org.checkerframework.checker.mustcall.qual.Owning;
 import org.checkerframework.checker.nullness.qual.Nullable;
-import org.checkerframework.checker.signature.qual.FullyQualifiedName;
-import org.checkerframework.common.accumulation.AccumulationAnalysis;
 import org.checkerframework.common.accumulation.AccumulationStore;
 import org.checkerframework.common.accumulation.AccumulationValue;
 import org.checkerframework.dataflow.cfg.ControlFlowGraph;
@@ -182,7 +177,7 @@ class MustCallConsistencyAnalyzer {
   private final ResourceLeakChecker checker;
 
   /** The analysis from the Resource Leak Checker, used to get input stores based on CFG blocks. */
-  private final AccumulationAnalysis analysis;
+  private final ResourceLeakAnalysis analysis;
 
   /** True if -AnoLightweightOwnership was passed on the command line. */
   private final boolean noLightweightOwnership;
@@ -561,7 +556,7 @@ public String stringForErrorMessage() {
    *     so this constructor cannot get it directly.
    */
   /*package-private*/ MustCallConsistencyAnalyzer(
-      ResourceLeakAnnotatedTypeFactory typeFactory, AccumulationAnalysis analysis) {
+      ResourceLeakAnnotatedTypeFactory typeFactory, ResourceLeakAnalysis analysis) {
     this.typeFactory = typeFactory;
     this.checker = (ResourceLeakChecker) typeFactory.getChecker();
     this.analysis = analysis;
@@ -1906,8 +1901,8 @@ private boolean shouldTrackReturnType(MethodInvocationNode node) {
 
   /**
    * Get all successor blocks for some block, except for those corresponding to ignored exception
-   * types. See {@link #ignoredExceptionTypes}. Each exceptional successor is paired with the type
-   * of exception that leads to it, for use in error messages.
+   * types. See {@link ResourceLeakAnalysis#isIgnoredExceptionType(TypeMirror)}. Each exceptional
+   * successor is paired with the type of exception that leads to it, for use in error messages.
    *
    * @param block input block
    * @return set of pairs (b, t), where b is a successor block, and t is the type of exception for
@@ -1927,7 +1922,7 @@ private boolean shouldTrackReturnType(MethodInvocationNode node) {
       Map<TypeMirror, Set<Block>> exceptionalSuccessors = excBlock.getExceptionalSuccessors();
       for (Map.Entry<TypeMirror, Set<Block>> entry : exceptionalSuccessors.entrySet()) {
         TypeMirror exceptionType = entry.getKey();
-        if (!isIgnoredExceptionType(((Type) exceptionType).tsym.getQualifiedName())) {
+        if (!analysis.isIgnoredExceptionType(exceptionType)) {
           for (Block exSucc : entry.getValue()) {
             result.add(IPair.of(exSucc, exceptionType));
           }
@@ -2477,58 +2472,6 @@ private void incrementMustCallImpl(TypeMirror type) {
         .isSubtypeQualifiersOnly(cmAnno, cmAnnoForMustCallMethods);
   }
 
-  /**
-   * The exception types in this set are ignored in the CFG when determining if a resource leaks
-   * along an exceptional path. These kinds of errors fall into a few categories: runtime errors,
-   * errors that the JVM can issue on any statement, and errors that can be prevented by running
-   * some other CF checker.
-   *
-   * <p>Package-private to permit access from {@link ResourceLeakAnalysis}.
-   */
-  /*package-private*/ static final Set<String> ignoredExceptionTypes =
-      new HashSet<>(
-          ImmutableSet.of(
-              // Any method call has a CFG edge for Throwable/RuntimeException/Error
-              // to represent run-time misbehavior. Ignore it.
-              Throwable.class.getCanonicalName(),
-              Error.class.getCanonicalName(),
-              RuntimeException.class.getCanonicalName(),
-              // Use the Nullness Checker to prove this won't happen.
-              NullPointerException.class.getCanonicalName(),
-              // These errors can't be predicted statically, so ignore them and assume
-              // they won't happen.
-              ClassCircularityError.class.getCanonicalName(),
-              ClassFormatError.class.getCanonicalName(),
-              NoClassDefFoundError.class.getCanonicalName(),
-              OutOfMemoryError.class.getCanonicalName(),
-              // It's not our problem if the Java type system is wrong.
-              ClassCastException.class.getCanonicalName(),
-              // It's not our problem if the code is going to divide by zero.
-              ArithmeticException.class.getCanonicalName(),
-              // Use the Index Checker to prevent these errors.
-              ArrayIndexOutOfBoundsException.class.getCanonicalName(),
-              NegativeArraySizeException.class.getCanonicalName(),
-              // Most of the time, this exception is infeasible, as the charset used
-              // is guaranteed to be present by the Java spec (e.g., "UTF-8").
-              // Eventually, this exclusion could be refined by looking at the charset
-              // being requested.
-              UnsupportedEncodingException.class.getCanonicalName()));
-
-  /**
-   * Is {@code exceptionClassName} an exception type the checker ignores, to avoid excessive false
-   * positives? For now the checker ignores most runtime exceptions (especially the runtime
-   * exceptions that can occur at any point during the program due to something going wrong in the
-   * JVM, like OutOfMemoryError and ClassCircularityError) and exceptions that can be proved to
-   * never occur by another Checker Framework built-in checker, such as null-pointer dereferences
-   * (the Nullness Checker) and out-of-bounds array indexing (the Index Checker).
-   *
-   * @param exceptionClassName the fully-qualified name of the exception
-   * @return true if the given exception class should be ignored
-   */
-  private static boolean isIgnoredExceptionType(@FullyQualifiedName Name exceptionClassName) {
-    return ignoredExceptionTypes.contains(exceptionClassName.toString());
-  }
-
   /**
    * If the input {@code state} has not been visited yet, add it to {@code visited} and {@code
    * worklist}.

checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakAnalysis.java

@@ -1,24 +1,36 @@
 package org.checkerframework.checker.resourceleak;
 
+import javax.lang.model.type.TypeMirror;
 import org.checkerframework.checker.calledmethods.CalledMethodsAnalysis;
 import org.checkerframework.checker.calledmethods.CalledMethodsAnnotatedTypeFactory;
-import org.checkerframework.common.basetype.BaseTypeChecker;
 
 /**
- * This variant of CFAnalysis extends the set of ignored exception types to include all those
- * ignored by the {@link MustCallConsistencyAnalyzer}. See {@link
- * MustCallConsistencyAnalyzer#ignoredExceptionTypes}.
+ * This variant of CFAnalysis extends the set of ignored exception types.
+ *
+ * @see ResourceLeakChecker#getIgnoredExceptions()
  */
 public class ResourceLeakAnalysis extends CalledMethodsAnalysis {
+
+  /**
+   * The set of exceptions to ignore, cached from {@link
+   * ResourceLeakChecker#getIgnoredExceptions()}.
+   */
+  private final SetOfTypes ignoredExceptions;
+
   /**
    * Creates a new {@code CalledMethodsAnalysis}.
    *
    * @param checker the checker
    * @param factory the factory
    */
   protected ResourceLeakAnalysis(
-      BaseTypeChecker checker, CalledMethodsAnnotatedTypeFactory factory) {
+      ResourceLeakChecker checker, CalledMethodsAnnotatedTypeFactory factory) {
     super(checker, factory);
-    ignoredExceptionTypes.addAll(MustCallConsistencyAnalyzer.ignoredExceptionTypes);
+    this.ignoredExceptions = checker.getIgnoredExceptions();
+  }
+
+  @Override
+  public boolean isIgnoredExceptionType(TypeMirror exceptionType) {
+    return ignoredExceptions.contains(getTypes(), exceptionType);
   }
 }

checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakAnnotatedTypeFactory.java

@@ -131,7 +131,7 @@ public AnnotationMirror createCalledMethods(String... val) {
   @Override
   public void postAnalyze(ControlFlowGraph cfg) {
     MustCallConsistencyAnalyzer mustCallConsistencyAnalyzer =
-        new MustCallConsistencyAnalyzer(this, this.analysis);
+        new MustCallConsistencyAnalyzer(this, (ResourceLeakAnalysis) this.analysis);
     mustCallConsistencyAnalyzer.analyze(cfg);
 
     // Inferring owning annotations for @Owning fields/parameters, @EnsuresCalledMethods for
@@ -148,7 +148,7 @@ public void postAnalyze(ControlFlowGraph cfg) {
 
   @Override
   protected ResourceLeakAnalysis createFlowAnalysis() {
-    return new ResourceLeakAnalysis(checker, this);
+    return new ResourceLeakAnalysis((ResourceLeakChecker) checker, this);
   }
 
   /**