typetools / checker-framework

Pluggable type-checking for Java
http://checkerframework.org/
Other
1.02k stars 353 forks source link

Compiler exception due to mismatch between JavaParser and javac #6630

Open iamsanjaymalakar opened 4 months ago

iamsanjaymalakar commented 4 months ago

Encountered it while running CF on the NJR benchmark.

Stacktrace

An exception has occurred in the compiler (11.0.22). Please file a bug against the Java compiler via the Java bug reporting page (https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com) for duplicates. Include your program and the following diagnostic in your report. Thank you.
com.sun.tools.javac.util.ClientCodeException: java.lang.Error: Problem while parsing ../dataset/june2020_dataset_copy/url400631cdd0_ludaiqian_appliedxml_tgz-pJ8-test_simples_TestJ8/wpi-out/test/fromlist/Test-org.checkerframework.checker.mustcall.MustCallChecker.ajava that corresponds to ../dataset/june2020_dataset_copy/url400631cdd0_ludaiqian_appliedxml_tgz-pJ8-test_simples_TestJ8/src/test/fromlist/Test.java
    at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:832)
    at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1414)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1371)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:973)
    at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:311)
    at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:170)
    at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:57)
    at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:43)
Caused by: java.lang.Error: Problem while parsing ../dataset/june2020_dataset_copy/url400631cdd0_ludaiqian_appliedxml_tgz-pJ8-test_simples_TestJ8/wpi-out/test/fromlist/Test-org.checkerframework.checker.mustcall.MustCallChecker.ajava that corresponds to ../dataset/june2020_dataset_copy/url400631cdd0_ludaiqian_appliedxml_tgz-pJ8-test_simples_TestJ8/src/test/fromlist/Test.java
    at org.checkerframework.framework.type.AnnotatedTypeFactory.setRoot(AnnotatedTypeFactory.java:1015)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.setRoot(GenericAnnotatedTypeFactory.java:443)
    at org.checkerframework.checker.mustcall.MustCallAnnotatedTypeFactory.setRoot(MustCallAnnotatedTypeFactory.java:153)
    at org.checkerframework.common.basetype.BaseTypeVisitor.setRoot(BaseTypeVisitor.java:387)
    at org.checkerframework.framework.source.SourceChecker.setRoot(SourceChecker.java:685)
    at org.checkerframework.common.basetype.BaseTypeChecker.setRoot(BaseTypeChecker.java:167)
    at org.checkerframework.framework.source.SourceChecker.typeProcess(SourceChecker.java:1074)
    at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:559)
    at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:552)
    at org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:188)
    at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:828)
    ... 8 more
Caused by: java.lang.AssertionError
    at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitClassMembers(JointJavacJavaParserVisitor.java:562)
    at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitClass(JointJavacJavaParserVisitor.java:479)
    at org.checkerframework.framework.stub.AnnotationFileParser$AjavaAnnotationCollectorVisitor.visitClass(AnnotationFileParser.java:3123)
    at org.checkerframework.framework.stub.AnnotationFileParser$AjavaAnnotationCollectorVisitor.visitClass(AnnotationFileParser.java:3113)
    at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
    at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitLists(JointJavacJavaParserVisitor.java:2300)
    at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitCompilationUnit(JointJavacJavaParserVisitor.java:668)
    at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitCompilationUnit(JointJavacJavaParserVisitor.java:188)
    at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:591)
    at org.checkerframework.framework.stub.AnnotationFileParser.processCompilationUnit(AnnotationFileParser.java:825)
    at org.checkerframework.framework.stub.AnnotationFileParser.processStubUnit(AnnotationFileParser.java:790)
    at org.checkerframework.framework.stub.AnnotationFileParser.process(AnnotationFileParser.java:779)
    at org.checkerframework.framework.stub.AnnotationFileParser.parseAjavaFile(AnnotationFileParser.java:696)
    at org.checkerframework.framework.stub.AnnotationFileElementTypes.parseAjavaFileWithTree(AnnotationFileElementTypes.java:286)
    at org.checkerframework.framework.type.AnnotatedTypeFactory.setRoot(AnnotatedTypeFactory.java:1013)
    ... 18 more

Reproducer

reproducer.zip

iamsanjaymalakar commented 4 months ago

I'm encountering an issue where the code is crashing at an assert statement in JointJavacJavaParserVisitor.java on line 562. The problem involves the following class:

public class Test {

    ArrayList<Qw> qws = new ArrayList<Qw>();

    public static void main(String[] args) throws FileNotFoundException, UnsupportedEncodingException {
        // ....
    }
}

This class has two members. While JavaParser correctly identifies these members, Javac does not detect any members. To debug this issue, I tracked where the class members might be getting lost in Javac.

Here is the trace I followed:

  1. visitCompilationUnit:647, JointJavacJavaParserVisitor (org.checkerframework.framework.ajava)
  2. visitCompilationUnit:188, JointJavacJavaParserVisitor (org.checkerframework.framework.ajava)
  3. accept:591, JCTree$JCCompilationUnit (com.sun.tools.javac.tree)
  4. processCompilationUnit:825, AnnotationFileParser (org.checkerframework.framework.stub)

In AnnotationFileParser::processCompilationUnit on line 825, the AjavaAnnotationCollectorVisitor is called with the compilation unit using:

root.accept(new AjavaAnnotationCollectorVisitor(), cu);

Following this, the JointJavacJavaParserVisitor is invoked, but at this point, the class members are missing for Javac. I could not find where from the compilation unit cu, the JointJavacJavaParserVisitor is getting invoked with javac and javaparser arguments.

I need help locating the specific invocation of JointJavacJavaParserVisitor to understand why Javac is not recognizing the class members from the compilation unit.

iamsanjaymalakar commented 4 months ago

@smillst, could you please provide your expertise on this matter?

smillst commented 4 months ago

I tried the reproducer, but did not get the crash. Can you make a smaller example the reproduces the crash?

iamsanjaymalakar commented 3 months ago

After thorough investigation, I am closing this issue as I was unable to reproduce it on other machines. The original issue was encountered on a Linux machine. However, after testing on a new Linux machine with the same configuration and cleaning the metadata on the original machine, the issue did not recur in either case.

iamsanjaymalakar commented 2 months ago

I've encountered this compiler crash again in some projects in the NJR benchmark. This time I could reproduce it in my local maching (M2 mac). Also, @msridhar was able to reproduce it as well.

An exception has occurred in the compiler (11.0.23). Please file a bug against the Java compiler via the Java bug reporting page (https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com) for duplicates. Include your program and the following diagnostic in your report. Thank you.
com.sun.tools.javac.util.ClientCodeException: java.lang.Error: Problem while parsing url433bec1e56_sarobe_DifficultyInvestigation_tgz-pJ8-fr_inria_optimization_cmaes_examples_CMAExample1J8/wpi-out/fr/inria/optimization/cmaes/examples/CMAExample1-org.checkerframework.checker.mustcall.MustCallChecker.ajava that corresponds to url433bec1e56_sarobe_DifficultyInvestigation_tgz-pJ8-fr_inria_optimization_cmaes_examples_CMAExample1J8/src/fr/inria/optimization/cmaes/examples/CMAExample1.java
        at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:832)
        at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1417)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1374)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:973)
        at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:311)
        at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:170)
        at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:57)
        at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:43)
Caused by: java.lang.Error: Problem while parsing url433bec1e56_sarobe_DifficultyInvestigation_tgz-pJ8-fr_inria_optimization_cmaes_examples_CMAExample1J8/wpi-out/fr/inria/optimization/cmaes/examples/CMAExample1-org.checkerframework.checker.mustcall.MustCallChecker.ajava that corresponds to url433bec1e56_sarobe_DifficultyInvestigation_tgz-pJ8-fr_inria_optimization_cmaes_examples_CMAExample1J8/src/fr/inria/optimization/cmaes/examples/CMAExample1.java
        at org.checkerframework.framework.type.AnnotatedTypeFactory.setRoot(AnnotatedTypeFactory.java:1015)
        at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.setRoot(GenericAnnotatedTypeFactory.java:443)
        at org.checkerframework.checker.mustcall.MustCallAnnotatedTypeFactory.setRoot(MustCallAnnotatedTypeFactory.java:153)
        at org.checkerframework.common.basetype.BaseTypeVisitor.setRoot(BaseTypeVisitor.java:387)
        at org.checkerframework.framework.source.SourceChecker.setRoot(SourceChecker.java:685)
        at org.checkerframework.common.basetype.BaseTypeChecker.setRoot(BaseTypeChecker.java:167)
        at org.checkerframework.framework.source.SourceChecker.typeProcess(SourceChecker.java:1074)
        at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:559)
        at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:552)
        at org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:188)
        at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:828)
        ... 8 more
Caused by: java.lang.AssertionError
        at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitClassMembers(JointJavacJavaParserVisitor.java:570)
        at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitClass(JointJavacJavaParserVisitor.java:479)
        at org.checkerframework.framework.stub.AnnotationFileParser$AjavaAnnotationCollectorVisitor.visitClass(AnnotationFileParser.java:3123)
        at org.checkerframework.framework.stub.AnnotationFileParser$AjavaAnnotationCollectorVisitor.visitClass(AnnotationFileParser.java:3113)
        at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
        at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitLists(JointJavacJavaParserVisitor.java:2300)
        at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitCompilationUnit(JointJavacJavaParserVisitor.java:668)
        at org.checkerframework.framework.ajava.JointJavacJavaParserVisitor.visitCompilationUnit(JointJavacJavaParserVisitor.java:188)
        at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:591)
        at org.checkerframework.framework.stub.AnnotationFileParser.processCompilationUnit(AnnotationFileParser.java:825)
        at org.checkerframework.framework.stub.AnnotationFileParser.processStubUnit(AnnotationFileParser.java:790)
        at org.checkerframework.framework.stub.AnnotationFileParser.process(AnnotationFileParser.java:779)
        at org.checkerframework.framework.stub.AnnotationFileParser.parseAjavaFile(AnnotationFileParser.java:696)
        at org.checkerframework.framework.stub.AnnotationFileElementTypes.parseAjavaFileWithTree(AnnotationFileElementTypes.java:286)
        at org.checkerframework.framework.type.AnnotatedTypeFactory.setRoot(AnnotatedTypeFactory.java:1013)
        ... 18 more

I'm attaching a google drive link with the updated script and project (project was too large to upload into GH): https://drive.google.com/file/d/1rXsOIEmjSj2D1siZ4TzjD8R7I8_fWwC4/view?usp=sharing

To run:

kelloggm commented 2 months ago

I can reproduce the problem with the new reproduction package and I'm looking into it.

kelloggm commented 2 months ago

I've spent the last few hours investigating this problem, and it seems very serious to me. I'm writing notes here to both document my progress and hopefully get some ideas from others (@msridhar and @mernst, in particular) about where we might look next.

The stack trace indicates that JointJavacJavaParserVisitor is failing an assertion related to the contents of a class: javac and JavaParser have a different number of members when reading the problematic file. The problematic file (CMAExample1.java) looks like this:

class Rosenbrock implements IObjectiveFunction { // meaning implements methods valueOf and isFeasible
        public double valueOf (double[] x) {
                double res = 0;
                for (int i = 0; i < x.length-1; ++i)
                        res += 100 * (x[i]*x[i] - x[i+1]) * (x[i]*x[i] - x[i+1]) +
                        (x[i] - 1.) * (x[i] - 1.);
                return res;
        }
        public boolean isFeasible(double[] x) {return true; } // entire R^n is feasible
}

public class CMAExample1 {
        public static void main(String[] args) {
                IObjectiveFunction fitfun = new Rosenbrock();
                // ~60 lines of code using this Rosenbrock.
        }
}

The problem occurs when processing the compilation unit for CMAExample1: javac reports an empty Rosenbrock class with no members, but JavaParser (correctly, IMO) reports two members in Rosenbrock: the two methods defined in the source code above.

I traced this emptiness all the way up the stack trace; in AbstractTypeProcessor.java, around 180, the TaskEvent that the Checker Framework gets from javac contains a compilation unit that already has an empty Rosenbrock class. I speculate that this has something to do with the fact that Rosenbrock is a top-level type declaration but not the target element being analyzed; however, my reading of the definition of a compilation unit in the JLS 7.3 is that Rosenbrock and all of its members should be part of the compilation unit. So, it's possible that this is a bug in javac. There must be more to it than just a top-level class, though, or I'd think that we'd have found this long ago.

The question is what we should do about it. Here are some possibilities:

I can reproduce this behavior with any typechecker (I was concerned because the stack trace does include MustCallAnnotatedTypeFactory#setRoot); I tried both the Called Methods Checker on its own and the Index Checker.

I can reproduce the behavior with Java 11 and with Java 21, so if this is a javac bug, it's been there for a long time.

msridhar commented 2 months ago

I traced this emptiness all the way up the stack trace; in AbstractTypeProcessor.java, around 180, the TaskEvent that the Checker Framework gets from javac contains a compilation unit that already has an empty Rosenbrock class. I speculate that this has something to do with the fact that Rosenbrock is a top-level type declaration but not the target element being analyzed; however, my reading of the definition of a compilation unit in the JLS 7.3 is that Rosenbrock and all of its members should be part of the compilation unit. So, it's possible that this is a bug in javac. There must be more to it than just a top-level class, though, or I'd think that we'd have found this long ago.

This is indeed surprising. I looked over the AbstractTypeProcessor code you mentioned and I'm not exactly sure how it is supposed to work. Is the finished(TaskEvent) method supposed to be invoked once per type or once per compilation unit? I.e., is it invoked separately for Rosenbrock and CMAExample1 for this case? I don't fully follow the logic around the elements set in AbstractTypeProcessor and I wonder if something is slightly off there for a case like this. That said, it is weird to be seeing an empty AST for Rosenbrock. Possibly @mernst or @smillst might have some idea how that could happen.

@kelloggm is it the case right now that if there is code that should trigger a checker error within Rosenbrock, no such error will be reported, due to the empty AST?

msridhar commented 2 months ago

@kelloggm I did a bit more debugging of this case and here is what I found. AbstractTypeProcessor.AttributionTaskListener#finished gets called twice for CMAExample1.java, corresponding to the two top-level classes in the file. When it gets called for class Rosenbrock, the defs list inside the JCClassDecl for that class has the expected entries for valueOf and isFeasible. However, when finished gets called for the CMAExample1 class, the defs list for the Rosenbruck class is empty, as you observed. This might be to help guarantee that classes only get processed once by annotation processors? In any case, it seems the logic for parsing .ajava files is not set up for this (somewhat unexpected) behavior. But I don't think this impacts regular type checking, since the AbstractTypeProcessor#typeProcess method gets invoked once per top-level class in the source file, and in each case the AST for the corresponding class looks intact.

kelloggm commented 2 months ago

AbstractTypeProcessor.AttributionTaskListener#finished gets called twice for CMAExample1.java, corresponding to the two top-level classes in the file.

@msridhar thanks for digging into this. The behavior that you've described definitely explains the bug.

But I don't think this impacts regular type checking

I agree with your analysis. This is only a problem for WPI because we wrote assertions that require the javac AST and the AST parsed from the .ajava file to stay in-sync.

I have two remaining concerns:

On the first point, I wrote an ainfer test that I expected to crash, if it triggered the strange behavior from javac:

// Test case for https://github.com/typetools/checker-framework/issues/6630.
// No assertions; just checking for a crash.

class AnotherType {
  void method() {

  }
}

public class TwoTypesInCompilationUnit {
  public static void test() {
    AnotherType a = new AnotherType();
    a.method();
  }
}

To my surprise, this test does trigger the weird javac behavior but does not trigger a crash. There is a call to AbstractTypeProcessor#finished with this compilation unit:

    class AnotherType {
    }
    public class TwoTypesInCompilationUnit {

        public TwoTypesInCompilationUnit() {
            super();
        }

        public static void test() {
            AnotherType a = new AnotherType();
            a.method();
        }
    }

Note how this compilation unit has AnotherType as an empty class. However, we don't get a crash or assertion violation when reading the associated .ajava file, even though it definitely contains a body for the AnotherType class. I'm not sure as to why, but I have a lead: it doesn't seem like the assertions are even being reached in the above test case. One possible reason is that ajava parsing is only triggered if the checker's root has changed, and I don't think the above example changes the root between the two classes. So, the next step is to try to find a test case that triggers a root change during the processing of the first class (the one that will be empty); I suspect that either the fact that the original example extends a superclass and/or implements an interface is relevant. I'll try to investigate this more when I have another block of free time.

kelloggm commented 2 months ago

I've confirmed my hypothesis that the crash is triggered by the extends clause, with the following test (with two files):

// Test case for https://github.com/typetools/checker-framework/issues/6630.
// No assertions; just checking for a crash.

class AnotherType extends TypeInOtherCompilationUnit {
  void method() {

  }
}

public class TwoTypesInCompilationUnit {
  public static void test() {
    AnotherType a = new AnotherType();
    a.method();
  }
}
public class TypeInOtherCompilationUnit {

}

This causes the same assertion error that was previously reported when added to one of the ainfer test suites (I tested using ainferNullnessAjavaTest, since it is small and therefore fast, but it should probably eventually go in the ainferTestChecker suite).

smillst commented 1 month ago

I think the problem is that setRoot is called for each class in the CompilationUnitTree. If the class has already been processed, then that ClassTree has missing information. (We have actually encountered this in the past and have had to change code not to look at trees outside the current class.)

So, if we move the calls to BaseTypeVisitor#testJointJavacJavaParserVisitor() and BaseTypeVisitor#testAnnotationInsertion() to BaseTypeVisitor#visitClass. (And change the implementation to only look at the current class, not the whole compilation unit.) I think the crash will go away.

Is this only a problem when -AajavaChecks is used, right? That options is always passed in our test suite. But, it should not be used in the real world. So, I think if that option is removed, the crash should go away without changing the Checker Framework.

smillst commented 1 month ago

Ok, I see that in the reproduction that @iamsanjaymalakar supplied, -AajavaChecks isn't used and this is still a problem. That crash is triggered in AnnotatedTypeFactory#setRoot by currentFileAjavaTypes.parseAjavaFileWithTree(ajavaPath, root);. The problem still is:

setRoot is called for each class in the CompilationUnitTree. If the class has already been processed, then that ClassTree has missing information.

Can you change currentFileAjavaTypes.parseAjavaFileWithTree(ajavaPath, root); so that is only parses for the current class? Maybe by moving the call to GenericAnnotatedTypeFactory#preProcessClassTree?