Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -AresourceLeakIgnoredExceptions option #6242

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 =
msridhar marked this conversation as resolved.
Show resolved Hide resolved
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}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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));
}
Expand Down Expand Up @@ -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}.
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -148,7 +148,7 @@ public void postAnalyze(ControlFlowGraph cfg) {

@Override
protected ResourceLeakAnalysis createFlowAnalysis() {
return new ResourceLeakAnalysis(checker, this);
return new ResourceLeakAnalysis((ResourceLeakChecker) checker, this);
}

/**
Expand Down