typetools / checker-framework

Pluggable type-checking for Java
http://checkerframework.org/
Other
990 stars 347 forks source link

Checker Framework crashed with NPE #6030

Closed jacek-lewandowski closed 11 months ago

jacek-lewandowski commented 11 months ago

Reproduce with https://github.com/jacek-lewandowski/cassandra/tree/ecj-testing

JDK 11

ant cf -Dcf.check.only=org/apache/cassandra/service/reads/DataResolver.java

    [javac] error: SourceChecker.typeProcess: unexpected Throwable (NullPointerException) while processing /home/jlewandowski/dev/cassandra/c18190-ecj/src/java/org/apache/cassandra/service/reads/DataResolver.java
    [javac]   ; The Checker Framework crashed.  Please report the crash.
    [javac]   Compilation unit: /home/jlewandowski/dev/cassandra/c18190-ecj/src/java/org/apache/cassandra/service/reads/DataResolver.java
    [javac]   Last visited tree at line 63 column 1:
    [javac]   public class DataResolver<E extends Endpoints<E>, P extends ReplicaPlan.ForRead<E, P>> extends ResponseResolver<E, P>
    [javac]   Exception: java.lang.NullPointerException; java.lang.NullPointerException
    [javac]     at org.checkerframework.framework.flow.CFAbstractStore.<init>(CFAbstractStore.java:154)
    [javac]     at org.checkerframework.framework.flow.CFStore.<init>(CFStore.java:16)
    [javac]     at org.checkerframework.framework.flow.CFAnalysis.createCopiedStore(CFAnalysis.java:30)
    [javac]     at org.checkerframework.framework.flow.CFAnalysis.createCopiedStore(CFAnalysis.java:9)
    [javac]     at org.checkerframework.framework.flow.CFAbstractTransfer.initialStore(CFAbstractTransfer.java:287)
    [javac]     at org.checkerframework.framework.flow.CFAbstractTransfer.initialStore(CFAbstractTransfer.java:97)
    [javac]     at org.checkerframework.dataflow.analysis.ForwardAnalysisImpl.initInitialInputs(ForwardAnalysisImpl.java:346)
    [javac]     at org.checkerframework.dataflow.analysis.AbstractAnalysis.init(AbstractAnalysis.java:376)
    [javac]     at org.checkerframework.dataflow.analysis.ForwardAnalysisImpl.performAnalysis(ForwardAnalysisImpl.java:102)
    [javac]     at org.checkerframework.framework.flow.CFAbstractAnalysis.performAnalysis(CFAbstractAnalysis.java:147)
    [javac]     at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.analyze(GenericAnnotatedTypeFactory.java:1541)
    [javac]     at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.performFlowAnalysis(GenericAnnotatedTypeFactory.java:1458)
    [javac]     at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.checkAndPerformFlowAnalysis(GenericAnnotatedTypeFactory.java:1927)
    [javac]     at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.preProcessClassTree(GenericAnnotatedTypeFactory.java:421)
    [javac]     at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:533)
    [javac]     at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:183)
    [javac]     at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
    [javac]     at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
    [javac]     at org.checkerframework.framework.source.SourceVisitor.visit(SourceVisitor.java:86)
    [javac]     at org.checkerframework.framework.source.SourceChecker.typeProcess(SourceChecker.java:1030)
    [javac]     at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:560)
    [javac]     at org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:188)
    [javac]     at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:828)
    [javac]     at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
    [javac]     at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1414)
    [javac]     at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1371)
    [javac]     at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:973)
    [javac]     at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:311)
    [javac]     at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:170)
    [javac]     at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:57)
    [javac]     at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:43)
mernst commented 11 months ago

Unfortunately, I cannot reproduce the problem.

I tried each of these commands, on both JDK 11 and JDK 17:

ant clean && ant cf -Dcf.check.only=org/apache/cassandra/service/reads/DataResolver.java

ant clean && ant cf

Did you do something different? Could you confirm that you can still reproduce the problem, and give me more detailed instructions?

@msridhar You gave a thumbs-up emoji to the bug report. Does that mean you were able to reproduce it?

msridhar commented 11 months ago

Hey, I was not able to reproduce the problem. I just tried now on JDK 11. @jacek-lewandowski I see there are more recent commits on the branch. Maybe you could share the exact commit SHA that reproduces the problem?

jacek-lewandowski commented 11 months ago

Sorry guys, this is wip branch and I excluded the affected files entirely, let me provide you a dedicated branch:

https://github.com/jacek-lewandowski/cassandra/tree/cf-testing

ant cf-only -Dcf.check.only=org/apache/cassandra/service/reads/DataResolver.java
ant cf-only -Dcf.check.only=org/apache/cassandra/io/sstable/format/SSTableScanner.java

That ^ two files fail the analysis with exception

msridhar commented 11 months ago

Thanks! I can now reproduce the crashes on JDK 11.

msridhar commented 11 months ago

With the latest master branch I do not get a crash on DataResolver.java. For SSTableScanner.java I still get a crash. Here is a reduced version:

// Test case for https://github.com/typetools/checker-framework/issues/6030

import java.util.*;
import org.checkerframework.checker.mustcall.qual.Owning;

public class Issue6030 {

  interface CloseableIterator<T> extends Iterator<T>, java.io.Closeable {}

  static class MyScanner<T, I extends MyScanner<T, I>> implements CloseableIterator<T> {
    @Owning I iterator;

    public boolean hasNext() {
      if (iterator == null) iterator = createIterator();
      return iterator.hasNext();
    }

    public T next() {
      return null;
    }

    private I createIterator() {
      return null;
    }

    public void close() {}
  }
}

The crash stack is:

SourceChecker.typeProcess: unexpected Throwable (NullPointerException) while processing /Users/msridhar/git-repos/checker-framework/checker/tests/resourceleak/Issue6030.java
  ; The Checker Framework crashed.  Please report the crash.
  Compilation unit: /Users/msridhar/git-repos/checker-framework/checker/tests/resourceleak/Issue6030.java
  Last visited tree at line 6 column 1:
  public class Issue6030 {
  Exception: java.lang.NullPointerException; java.lang.NullPointerException
    at org.checkerframework.javacutil.AnnotationUtils.getElementValueArray(AnnotationUtils.java:1005)
    at org.checkerframework.checker.resourceleak.MustCallConsistencyAnalyzer.checkReassignmentToField(MustCallConsistencyAnalyzer.java:1446)
    at org.checkerframework.checker.resourceleak.MustCallConsistencyAnalyzer.updateObligationsForAssignment(MustCallConsistencyAnalyzer.java:1089)
    at org.checkerframework.checker.resourceleak.MustCallConsistencyAnalyzer.analyze(MustCallConsistencyAnalyzer.java:528)
[...]

Branch is here.

I think there are two issues:

  1. Our checker is not discovering that the I type will have a @MustCall("close") obligation (due to its upper bound).
  2. Even in cases where @Owning is written on a field with no @MustCall obligation, we shouldn't crash.

@kelloggm do you think you might get a chance to take a first look at the above?

@jacek-lewandowski in the Cassandra case, I think the desired close() obligation might come from the CloseableIterator type. But, this type extends java.lang.AutoCloseable, and the RLC does not directly treat that type as @MustCall("close") (as it would lead to many false positives, e.g., for most Streams). Instead, the RLC treates java.io.Closeable as @MustCall("close"). So, to get the desired behavior (assuming we can fix the checker issues), you may need to add an explicit @MustCall("close") annotation somewhere in the type hierarchy.

kelloggm commented 11 months ago

The reason that the obligation isn't discovered (item 1 in @msridhar's list) is #5908. #5912 will fix that, but comes with some other nasty side-effects that have prevented it from being merged so far. I'm working on those.

6051 fixes item 2 in @msridhar's list.

msridhar commented 11 months ago

I am going to go ahead and close this one. The crash is now fixed on master, and the other aspect of the issue is a duplicate of #5908. Thanks again for the report!

jacek-lewandowski commented 11 months ago

Thanks for solving that! When the next release planned?

mernst commented 11 months ago

We make a release on the first business day of each month. If this is a hardship for you, we can make a snapshot release, or (with a bit more effort) make an out-of-band release.

msridhar commented 11 months ago

@jacek-lewandowski you will most likely want a build with #5912 landed as well, as I could imagine that having a significant impact for the type of code you are trying to check.

kelloggm commented 11 months ago

5912 should land as soon as its CI passes, some time this afternoon. I agree that it will probably impact @jacek-lewandowski's work (hopefully mostly in a good way!)

jacek-lewandowski commented 11 months ago

great, first business day next month is fine with me as well... maybe more tickets will be solved by that time :)

jacek-lewandowski commented 11 months ago

thank you

Calvin-L commented 10 months ago

I'd like to reopen this issue. I am getting a stack trace very similar to @jacek-lewandowski's original report using both the latest release (3.36.0) and current master.

The problem does not seem to be be specific to the resource leak checker. I can reproduce it with the nullness checker as well.

Repro script:

#!/usr/bin/env bash

# Settings
CF_VERSION='3.36.0'
GENERATED_FILES=(
    Main.{java,class}
)

set -ex

# Clean up
rm -rf "${GENERATED_FILES[@]}"

# Download the checker framework
if [[ ! -d "checker-framework-${CF_VERSION}" ]]; then
    curl -LOf "https://checkerframework.org/checker-framework-${CF_VERSION}.zip"
    unzip "checker-framework-${CF_VERSION}.zip"
fi

# Create classes
cat >Main.java <<EOF
public class Main {
    public void m() {
        try {
        } catch (Exception e) {
            Runnable r = () -> {};
        }
    }
}
EOF

# Try to compile it
CF_ARGV=(
    # -processor 'org.checkerframework.checker.resourceleak.ResourceLeakChecker'
    -processor nullness
    Main.java
)
exec "./checker-framework-${CF_VERSION}/checker/bin/javac" "${CF_ARGV[@]}"

Output:

error: SourceChecker.typeProcess: unexpected Throwable (NullPointerException) while processing Main.java
  ; The Checker Framework crashed.  Please report the crash.
  Compilation unit: Main.java
  Last visited tree at line 1 column 1:
  public class Main {
  Exception: java.lang.NullPointerException; java.lang.NullPointerException
    at org.checkerframework.framework.flow.CFAbstractStore.<init>(CFAbstractStore.java:154)
    at org.checkerframework.checker.nullness.KeyForStore.<init>(KeyForStore.java:13)
    at org.checkerframework.checker.nullness.KeyForAnalysis.createCopiedStore(KeyForAnalysis.java:30)
    at org.checkerframework.checker.nullness.KeyForAnalysis.createCopiedStore(KeyForAnalysis.java:11)
    at org.checkerframework.framework.flow.CFAbstractTransfer.initialStore(CFAbstractTransfer.java:288)
    at org.checkerframework.framework.flow.CFAbstractTransfer.initialStore(CFAbstractTransfer.java:98)
    at org.checkerframework.dataflow.analysis.ForwardAnalysisImpl.initInitialInputs(ForwardAnalysisImpl.java:347)
    at org.checkerframework.dataflow.analysis.AbstractAnalysis.init(AbstractAnalysis.java:376)
    at org.checkerframework.dataflow.analysis.ForwardAnalysisImpl.performAnalysis(ForwardAnalysisImpl.java:102)
    at org.checkerframework.framework.flow.CFAbstractAnalysis.performAnalysis(CFAbstractAnalysis.java:147)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.analyze(GenericAnnotatedTypeFactory.java:1544)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.performFlowAnalysis(GenericAnnotatedTypeFactory.java:1461)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.checkAndPerformFlowAnalysis(GenericAnnotatedTypeFactory.java:1933)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.preProcessClassTree(GenericAnnotatedTypeFactory.java:423)
    at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:536)
    at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:184)
    at com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
    at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
    at org.checkerframework.framework.source.SourceVisitor.visit(SourceVisitor.java:86)
    at org.checkerframework.framework.source.SourceChecker.typeProcess(SourceChecker.java:1043)
    at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:558)
    at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:551)
    at org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:188)
    at com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:828)
    at com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:120)
    at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1404)
    at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1363)
    at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:959)
    at com.sun.tools.javac.main.Main.compile(Main.java:302)
    at com.sun.tools.javac.main.Main.compile(Main.java:162)
    at com.sun.tools.javac.Main.compile(Main.java:57)
    at com.sun.tools.javac.Main.main(Main.java:43)
error: SourceChecker.typeProcess: unexpected Throwable (NullPointerException) while processing Main.java
  ; The Checker Framework crashed.  Please report the crash.
  Compilation unit: Main.java
  Last visited tree at line 1 column 1:
  public class Main {
  Exception: java.lang.NullPointerException; java.lang.NullPointerException
    at org.checkerframework.framework.flow.CFAbstractStore.<init>(CFAbstractStore.java:154)
    at org.checkerframework.checker.initialization.InitializationStore.<init>(InitializationStore.java:122)
    at org.checkerframework.checker.nullness.NullnessStore.<init>(NullnessStore.java:58)
    at org.checkerframework.checker.nullness.NullnessAnalysis.createCopiedStore(NullnessAnalysis.java:34)
    at org.checkerframework.checker.nullness.NullnessAnalysis.createCopiedStore(NullnessAnalysis.java:14)
    at org.checkerframework.framework.flow.CFAbstractTransfer.initialStore(CFAbstractTransfer.java:288)
    at org.checkerframework.framework.flow.CFAbstractTransfer.initialStore(CFAbstractTransfer.java:98)
    at org.checkerframework.dataflow.analysis.ForwardAnalysisImpl.initInitialInputs(ForwardAnalysisImpl.java:347)
    at org.checkerframework.dataflow.analysis.AbstractAnalysis.init(AbstractAnalysis.java:376)
    at org.checkerframework.dataflow.analysis.ForwardAnalysisImpl.performAnalysis(ForwardAnalysisImpl.java:102)
    at org.checkerframework.framework.flow.CFAbstractAnalysis.performAnalysis(CFAbstractAnalysis.java:147)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.analyze(GenericAnnotatedTypeFactory.java:1544)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.performFlowAnalysis(GenericAnnotatedTypeFactory.java:1461)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.checkAndPerformFlowAnalysis(GenericAnnotatedTypeFactory.java:1933)
    at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.preProcessClassTree(GenericAnnotatedTypeFactory.java:423)
    at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:536)
    at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:184)
    at com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
    at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
    at org.checkerframework.framework.source.SourceVisitor.visit(SourceVisitor.java:86)
    at org.checkerframework.framework.source.SourceChecker.typeProcess(SourceChecker.java:1043)
    at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:558)
    at org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:188)
    at com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:828)
    at com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:120)
    at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1404)
    at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1363)
    at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:959)
    at com.sun.tools.javac.main.Main.compile(Main.java:302)
    at com.sun.tools.javac.main.Main.compile(Main.java:162)
    at com.sun.tools.javac.Main.compile(Main.java:57)
    at com.sun.tools.javac.Main.main(Main.java:43)
2 errors

In case it's useful, I notice that the stack trace goes through CFAbstractTransfer.initialStore(CFAbstractTransfer.java:288), which seems related to handling lambdas:

    } else if (underlyingAST.getKind() == UnderlyingAST.Kind.LAMBDA) {
      // Create a copy and keep only the field values (nothing else applies).
      store = analysis.createCopiedStore(fixedInitialStore);
smillst commented 10 months ago

@Calvin-L I opened a new issue for this test: https://github.com/typetools/checker-framework/issues/6104.