Error-prone: issues with by-file compilation policy

[boris-petrov]

     error-prone version: 2.1.2
     Stack Trace:
     java.lang.IndexOutOfBoundsException
        at java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:580)
        at java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:42)
        at com.google.errorprone.bugpatterns.ParameterName.checkArguments(ParameterName.java:85)
        at com.google.errorprone.bugpatterns.ParameterName.matchNewClass(ParameterName.java:70)
        at com.google.errorprone.scanner.ErrorProneScanner.visitNewClass(ErrorProneScanner.java:952)
        at com.google.errorprone.scanner.ErrorProneScanner.visitNewClass(ErrorProneScanner.java:146)
        at com.sun.tools.javac.tree.JCTree$JCNewClass.accept(JCTree.java:1705)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:42)
        at com.sun.source.util.TreeScanner.visitReturn(TreeScanner.java:469)
        at com.google.errorprone.scanner.ErrorProneScanner.visitReturn(ErrorProneScanner.java:1021)
        at com.google.errorprone.scanner.ErrorProneScanner.visitReturn(ErrorProneScanner.java:146)
        at com.sun.tools.javac.tree.JCTree$JCReturn.accept(JCTree.java:1548)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:42)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.visitBlock(TreeScanner.java:248)
        at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:530)
        at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:146)
        at com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1026)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:42)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
        at com.sun.source.util.TreeScanner.visitMethod(TreeScanner.java:206)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:898)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:146)
        at com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:898)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:42)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:113)
        at com.sun.source.util.TreeScanner.visitClass(TreeScanner.java:187)
        at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:590)
        at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:146)
        at com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:42)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:113)
        at com.sun.source.util.TreeScanner.visitCompilationUnit(TreeScanner.java:144)
        at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:605)
        at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:146)
        at com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:591)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:64)
        at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:41)
        at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:145)
        at com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:120)
        at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1425)
        at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1374)
        at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:973)
        at com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:100)
        at com.sun.tools.javac.api.JavacTaskImpl.handleExceptions(JavacTaskImpl.java:142)
        at com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:96)
        at com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:90)
        at com.google.errorprone.BaseErrorProneCompiler.run(BaseErrorProneCompiler.java:137)
        at com.google.errorprone.BaseErrorProneCompiler.run(BaseErrorProneCompiler.java:108)
        at com.google.errorprone.ErrorProneCompiler.run(ErrorProneCompiler.java:118)
        at com.google.errorprone.ErrorProneCompiler.compile(ErrorProneCompiler.java:65)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at net.ltgt.gradle.errorprone.ErrorProneCompiler.execute(ErrorProneCompiler.java:66)
        at net.ltgt.gradle.errorprone.ErrorProneCompiler.execute(ErrorProneCompiler.java:23)
        at org.gradle.api.internal.tasks.compile.NormalizingJavaCompiler.delegateAndHandleErrors(NormalizingJavaCompiler.java:99)
        at org.gradle.api.internal.tasks.compile.NormalizingJavaCompiler.execute(NormalizingJavaCompiler.java:52)
        at org.gradle.api.internal.tasks.compile.NormalizingJavaCompiler.execute(NormalizingJavaCompiler.java:37)
        at org.gradle.api.internal.tasks.compile.CleaningJavaCompilerSupport.execute(CleaningJavaCompilerSupport.java:35)
        at org.gradle.api.internal.tasks.compile.CleaningJavaCompilerSupport.execute(CleaningJavaCompilerSupport.java:25)
        at org.gradle.api.tasks.compile.JavaCompile.performCompilation(JavaCompile.java:204)
        at org.gradle.api.tasks.compile.JavaCompile.compile(JavaCompile.java:189)
        at org.gradle.api.tasks.compile.JavaCompile.compile(JavaCompile.java:121)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:73)
        at org.gradle.api.internal.project.taskfactory.DefaultTaskClassInfoStore$IncrementalTaskAction.doExecute(DefaultTaskClassInfoStore.java:179)
        at org.gradle.api.internal.project.taskfactory.DefaultTaskClassInfoStore$StandardTaskAction.execute(DefaultTaskClassInfoStore.java:135)
        at org.gradle.api.internal.project.taskfactory.DefaultTaskClassInfoStore$StandardTaskAction.execute(DefaultTaskClassInfoStore.java:122)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter$1.run(ExecuteActionsTaskExecuter.java:121)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:336)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:328)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:199)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:110)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeAction(ExecuteActionsTaskExecuter.java:110)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:92)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:70)
        at org.gradle.api.internal.tasks.execution.SkipUpToDateTaskExecuter.execute(SkipUpToDateTaskExecuter.java:62)
        at org.gradle.api.internal.tasks.execution.ResolveTaskOutputCachingStateExecuter.execute(ResolveTaskOutputCachingStateExecuter.java:54)
        at org.gradle.api.internal.tasks.execution.ValidatingTaskExecuter.execute(ValidatingTaskExecuter.java:58)
        at org.gradle.api.internal.tasks.execution.SkipEmptySourceFilesTaskExecuter.execute(SkipEmptySourceFilesTaskExecuter.java:97)
        at org.gradle.api.internal.tasks.execution.CleanupStaleOutputsExecuter.execute(CleanupStaleOutputsExecuter.java:87)
        at org.gradle.api.internal.tasks.execution.ResolveTaskArtifactStateTaskExecuter.execute(ResolveTaskArtifactStateTaskExecuter.java:52)
        at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:52)
        at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:54)
        at org.gradle.api.internal.tasks.execution.ExecuteAtMostOnceTaskExecuter.execute(ExecuteAtMostOnceTaskExecuter.java:43)
        at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:34)
        at org.gradle.execution.taskgraph.DefaultTaskGraphExecuter$EventFiringTaskWorker$1.run(DefaultTaskGraphExecuter.java:248)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:336)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:328)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:199)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:110)
        at org.gradle.execution.taskgraph.DefaultTaskGraphExecuter$EventFiringTaskWorker.execute(DefaultTaskGraphExecuter.java:241)
        at org.gradle.execution.taskgraph.DefaultTaskGraphExecuter$EventFiringTaskWorker.execute(DefaultTaskGraphExecuter.java:230)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.processTask(DefaultTaskPlanExecutor.java:123)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.access$200(DefaultTaskPlanExecutor.java:79)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:104)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:98)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionPlan.execute(DefaultTaskExecutionPlan.java:625)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionPlan.executeWithTask(DefaultTaskExecutionPlan.java:580)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.run(DefaultTaskPlanExecutor.java:98)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
        at java.lang.Thread.run(Thread.java:748)

Please tell me if you need any more information.

P.S. This is probably a duplicate of https://github.com/google/error-prone/issues/780 but I'll leave you to decide if it is.

[cushon]

This is probably a duplicate of #780 but I'll leave you to decide if it is.

I think so, assuming you're using lombok?

[boris-petrov]

No, I’m not. Do you need some more information?

[cushon]

Do you have a self-contained example that reproduces the crash?

[boris-petrov]

I will have to try. I’ll write tomorrow.

[boris-petrov]

@cushon - OK, it took me an hour or so, it was horribly complicated to extract a reproduction, so you owe me a beer! :) Or Google. :D

Here goes:

package a;

import java.util.function.Function;

class Bar {
    private final Function<String, String> args;

    public Bar(Function<String, String> args) {
        this.args = args;
    }
}

public abstract class AbstractFoo {
    protected abstract String getFoo();

    private String getCommandArguments(String parameters) {
        return null;
    }

    public AbstractFoo() {
        new Bar(this::getCommandArguments);
    }
}

package a.b;

import a.AbstractFoo;

class Baz extends AbstractFoo {
    @Override
    protected String getFoo() {
        return "foo";
    }
}

A few things to note - the second file MUST be in a different package and also in a child package. Also, AbstractFoo MUST have Abstract in the name - otherwise it doesn't blow up. Thirdly - calling Bar's constructor MUST pass a method reference...

That's a pretty insane bug. :)

[cushon]

@boris-petrov thanks! 🍺🍺🍺

Verified with jdk1.8.0_91.jdk and Error Prone 2.1.2:

java -Xbootclasspath/p:error_prone_ant-2.1.2.jar com.google.errorprone.ErrorProneCompiler a/Baz.java a/AbstractFoo.java
...
a/AbstractFoo.java:21: error: An unhandled exception was thrown by the Error Prone static analysis plugin.
    new Bar(this::getCommandArguments);
...
     Stack Trace:
     java.lang.IndexOutOfBoundsException
    at java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:580)
    at java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:42)
    at com.google.errorprone.bugpatterns.ParameterName.checkArguments(ParameterName.java:85)

The AST node it's crashing on is:

new Bar(java.lang.invoke.LambdaMetafactory.metafactory(this))

which means that Error Prone is seeing a compilation unit after javac has already lowered the method reference, which isn't supposed to happen. This is a javac bug with the default compilation policy that can be worked around by setting -XDcompilePolicy=simple:

java -Xbootclasspath/p:error_prone_ant-2.1.2.jar com.google.errorprone.ErrorProneCompiler -XDcompilePolicy=simple a/Baz.java a/AbstractFoo.java
...
OK

We should make sure the build system integrations are doing that, and also have Error Prone check the policy when it runs and complain if it's one of the broken ones.

We already do some of that here, but at the time that was added we though the by_file policy was OK: https://github.com/google/error-prone/blob/d4b291e19a40d3292c4fa28ce2fec009363f3e76/core/src/main/java/com/google/errorprone/ErrorProneCompiler.java#L203

[msridhar]

@cushon are there any downsides to passing -XDcompilePolicy=simple? Do you recommend anyone using Error Prone to pass that flag?

[cushon]

It uses slightly more memory, but not an amount that I suspect matters. If you want to verify that for your usage, I'd be interested in what you find.

Internally we're using a fix to by_file that didn't make it into upstream javac (see the linked thread), which we could include in error-prone-javac until this is fixed. Last I heard there were some changes coming in the JDK 10 javac that would address this in a more principled way.

[msridhar]

We'll give it a try and if we see any serious memory regression we'll report back. +1 to including your internal fix in error-prone-javac 😄

[boris-petrov]

I'm not sure I followed the conversation. What do you suggest we do to fix this issue? I am hitting it on Java version 1.8.0_152.

[cushon]

Passing the javac flag -XDcompilePolicy=simple works around the problem.

I think we should either make Error Prone always pass that flag, or check that the flag is enabled before running, or make a fix to the javac we redistribute.

[cushon]

This is fixed by fcbbd714958586a73389409f6bfa0557db557ed5 (specifically https://github.com/google/error-prone-javac/commit/b5e09462b0a48e20f55784bedd185118eda83a99).