Skip to content

Commit

Permalink
Fix Symbol$CompletionFailure crash when building CFG (typetools#6397)
Browse files Browse the repository at this point in the history
Co-authored-by: Michael Ernst <mernst@cs.washington.edu>
  • Loading branch information
2 people authored and wmdietl committed Jan 30, 2025
1 parent 20f496f commit 283bebf
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 4 deletions.
4 changes: 4 additions & 0 deletions checker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ dependencies {
testImplementation 'org.apiguardian:apiguardian-api:1.1.2'
// For tests that use JSpecify annotations
testImplementation 'org.jspecify:jspecify:1.0.0'
// To test for an obscure crash in CFG construction for try-with-resources;
// see https://github.com/typetools/checker-framework/issues/6396
testImplementation 'org.apache.spark:spark-sql_2.12:3.3.2'

// Required for checker/tests/index-initializedfields/RequireJavadoc.java
if (JavaVersion.current() == JavaVersion.VERSION_1_8) {
Expand Down Expand Up @@ -1201,3 +1204,4 @@ publishing {
signing {
sign publishing.publications.checker
}

10 changes: 10 additions & 0 deletions checker/tests/resourceleak/SparkSessionCFGCrash.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// To test crash in CFG construction for try-with-resources
// See https://github.com/typetools/checker-framework/issues/6396
import org.apache.spark.sql.SparkSession;

public class SparkSessionCFGCrash {

private void run() {
try (SparkSession session = SparkSession.builder().getOrCreate()) {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.sun.tools.javac.tree.TreeInfo;
import com.sun.tools.javac.tree.TreeMaker;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.Name;
import com.sun.tools.javac.util.Names;
import java.util.List;
import javax.annotation.processing.ProcessingEnvironment;
Expand All @@ -46,14 +47,40 @@
* TreeMaker.
*/
public class TreeBuilder {

/** The javac {@link Elements} object. */
protected final Elements elements;

/** The javac {@link javax.lang.model.util.Types} object. */
protected final Types modelTypes;

/** The internal javac {@link com.sun.tools.javac.code.Types} object. */
protected final com.sun.tools.javac.code.Types javacTypes;

/** For constructing trees */
protected final TreeMaker maker;

/** The javac {@link Names} object. */
protected final Names names;

/** The javac {@link Symtab} object. */
protected final Symtab symtab;

/** The javac {@link ProcessingEnvironment} */
protected final ProcessingEnvironment env;

/**
* {@link Name} object for "close", used when building a tree for a call to {@code close()}.
*
* @see #buildCloseMethodAccess(ExpressionTree)
*/
private final Name closeName;

/**
* Creates a new TreeBuilder.
*
* @param env the javac {@link ProcessingEnvironment}
*/
public TreeBuilder(ProcessingEnvironment env) {
this.env = env;
Context context = ((JavacProcessingEnvironment) env).getContext();
Expand All @@ -63,6 +90,7 @@ public TreeBuilder(ProcessingEnvironment env) {
maker = TreeMaker.instance(context);
names = Names.instance(context);
symtab = Symtab.instance(context);
closeName = names.fromString("close");
}

/**
Expand Down Expand Up @@ -145,10 +173,20 @@ public MemberSelectTree buildCloseMethodAccess(ExpressionTree autoCloseableExpr)
// Find the close() method
Symbol.MethodSymbol closeMethod = null;

for (ExecutableElement method : ElementFilter.methodsIn(elements.getAllMembers(exprElement))) {
if (method.getParameters().isEmpty() && method.getSimpleName().contentEquals("close")) {
closeMethod = (Symbol.MethodSymbol) method;
break;
// We could use elements.getAllMembers(exprElement) to find the close method, but in rare cases
// calling that method crashes with a Symbol$CompletionFailure exception. See
// https://github.com/typetools/checker-framework/issues/6396. The code below directly searches
// all supertypes for the method and avoids the crash.
for (Type s : javacTypes.closure(((Symbol) exprElement).type)) {
for (Symbol m : s.tsym.members().getSymbolsByName(closeName)) {
if (!(m instanceof Symbol.MethodSymbol)) {
continue;
}
Symbol.MethodSymbol msym = (Symbol.MethodSymbol) m;
if (!msym.isStatic() && msym.getParameters().isEmpty()) {
closeMethod = msym;
break;
}
}
}

Expand Down

0 comments on commit 283bebf

Please sign in to comment.