search

eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
161 stars 130 forks source link

OperandStack throws "java.lang.AssertionError: Attempt to push null on operand stack!" #2624

Closed alban-auzeill closed 4 months ago

alban-auzeill commented 4 months ago

Note

The reproducer is a corner case where the compliance java version is > '8' and the target platform version < '1.5'. It should not happen often, but it may help to fix a bug that was difficult to reproduce otherwise, like the closed issue #2484

Reproducer

On the R4_32_maintenance branch Add the following unit test to org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/dom/StandAloneASTParserTest.java

    public void test_push_null_on_operand_reproducer() throws Exception {
        String contents =
            "public class X {\n" +
            "  int f;\n" +
            "  java.util.function.Consumer<Object> foo(int a) {\n" +
            "    return p -> foo(f);\n" +
            "  }\n" +
            "}";

        ASTParser parser = ASTParser.newParser(AST_JLS_LATEST);
        Map<String, String> options = JavaCore.getOptions();
        JavaCore.setComplianceOptions(JavaCore.VERSION_22, options);
        options.put("org.eclipse.jdt.core.compiler.codegen.targetPlatform", JavaCore.VERSION_1_5);
        options.put("org.eclipse.jdt.core.compiler.debug.localVariable", "do not generate");
        options.put("org.eclipse.jdt.core.compiler.codegen.simulateOperandStack", "enabled");

        parser.setCompilerOptions(options);
        parser.setSource(contents.toCharArray());
        parser.setEnvironment(new String[] {}, new String[] {}, new String[] {}, true);
        parser.setResolveBindings(true);
        parser.setUnitName("X.java");
        parser.setStatementsRecovery(true);
        parser.setBindingsRecovery(true);

        ASTNode node = parser.createAST(null);
        assertTrue("Should be a compilation unit", node instanceof CompilationUnit);
        CompilationUnit cu = (CompilationUnit) node;
        assertTrue("No problems in compilation", cu.getProblems().length == 0);
    }

Run the test:

mvn -f org.eclipse.jdt.core.tests.compiler/pom.xml verify -Dtest=org.eclipse.jdt.core.tests.dom.StandAloneASTParserTest

The test fails with the following error:

org.eclipse.jdt.core.tests.dom.StandAloneASTParserTest.test_push_null_on_operand_reproducer -- Time elapsed: 0.005 s <<< FAILURE!
java.lang.AssertionError: Attempt to push null on operand stack!
    at org.eclipse.jdt.internal.compiler.codegen.OperandStack.push(OperandStack.java:62)
    at org.eclipse.jdt.internal.compiler.codegen.CodeStream.pushTypeBinding(CodeStream.java:7714)
    at org.eclipse.jdt.internal.compiler.codegen.CodeStream.aload_0(CodeStream.java:346)
    at org.eclipse.jdt.internal.compiler.ast.ThisReference.generateCode(ThisReference.java:90)
    at org.eclipse.jdt.internal.compiler.ast.MessageSend.generateCode(MessageSend.java:618)
    at org.eclipse.jdt.internal.compiler.ast.LambdaExpression.generateCode(LambdaExpression.java:1297)
    at org.eclipse.jdt.internal.compiler.ast.LambdaExpression.generateCode(LambdaExpression.java:1238)
    at org.eclipse.jdt.internal.compiler.ClassFile.addSpecialMethods(ClassFile.java:1137)
    at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:766)
    at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:832)
    at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.generateCode(CompilationUnitDeclaration.java:412)
    at org.eclipse.jdt.core.dom.CompilationUnitResolver.resolve(CompilationUnitResolver.java:1332)
    at org.eclipse.jdt.core.dom.CompilationUnitResolver.resolve(CompilationUnitResolver.java:791)
    at org.eclipse.jdt.core.dom.ASTParser.internalCreateASTCached(ASTParser.java:1248)
    at org.eclipse.jdt.core.dom.ASTParser.lambda$0(ASTParser.java:1126)
    at org.eclipse.jdt.internal.core.JavaModelManager.cacheZipFiles(JavaModelManager.java:5765)
    at org.eclipse.jdt.core.dom.ASTParser.internalCreateAST(ASTParser.java:1126)
    at org.eclipse.jdt.core.dom.ASTParser.createAST(ASTParser.java:874)
    at org.eclipse.jdt.core.tests.dom.StandAloneASTParserTest.test_push_null_on_operand_reproducer(StandAloneASTParserTest.java:1983)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at junit.framework.TestCase.runTest(TestCase.java:177)
    at org.eclipse.jdt.core.tests.junit.extension.TestCase.runTest(TestCase.java:972)
    at junit.framework.TestCase.runBare(TestCase.java:142)
    at junit.framework.TestResult$1.protect(TestResult.java:122)
    at junit.framework.TestResult.runProtected(TestResult.java:142)
    at junit.framework.TestResult.run(TestResult.java:125)
    at junit.framework.TestCase.run(TestCase.java:130)
    at junit.framework.TestSuite.runTest(TestSuite.java:241)
    at junit.framework.TestSuite.run(TestSuite.java:236)
    at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:90)
    at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
    at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:240)
    at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:214)
    at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:155)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.apache.maven.surefire.api.util.ReflectionUtils.invokeMethodWithArray2(ReflectionUtils.java:137)
    at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:148)
    at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:88)
    at org.eclipse.tycho.surefire.osgibooter.OsgiSurefireBooter.run(OsgiSurefireBooter.java:140)
    at org.eclipse.tycho.surefire.osgibooter.HeadlessTestApplication.start(HeadlessTestApplication.java:29)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:208)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:143)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:109)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:439)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:271)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:668)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:605)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1481)
    at org.eclipse.equinox.launcher.Main.main(Main.java:1454)

Failures: 
  StandAloneASTParserTest>TestCase.runTest:972->test_push_null_on_operand_reproducer:1983 Attempt to push null on operand stack!

Tests run: 25, Failures: 1, Errors: 0, Skipped: 0
iloveeclipse commented 4 months ago

We plan to stop supporting Java targets < 1.8 in the current release, see #2536.

srikanth-sankaran commented 4 months ago

(1) Do you still get the problem if this line options.put("org.eclipse.jdt.core.compiler.codegen.simulateOperandStack", "enabled"); is deleted ?

I am surprised to see this option being enabled! This is a purely internal option that forces exercising of certain code paths and is not meant to be a user option. This internal option has served its purpose and is withdrawn - on master it doesn't exist.

In any case, on master the operand stack creation code looks like:

private OperandStack createOperandStack(CompilerOptions compilerOptions) {
    if ((this.generateAttributes & (ClassFileConstants.ATTR_VARS
            | ClassFileConstants.ATTR_STACK_MAP_TABLE
            | ClassFileConstants.ATTR_STACK_MAP)) == 0) {
        return new OperandStack.NullStack();
    }

    return this.targetLevel >= ClassFileConstants.JDK1_6 && JavaFeature.SWITCH_EXPRESSIONS.isSupported(compilerOptions.sourceLevel, compilerOptions.enablePreviewFeatures) ?
                                        new OperandStack(this.classFile) : new OperandStack.NullStack();
}

OIOW, a functional operand stack is simulated only when target level >=1.6 - which means in your snippet's case it would only create the dummy NullStack which acts as a sink for any value including null.

org.eclipse.jdt.internal.compiler.codegen.OperandStack.push(TypeBinding) starts outs with:

        if (typeBinding == null) {
            throw new AssertionError("Attempt to push null on operand stack!"); //$NON-NLS-1$
        }

while org.eclipse.jdt.internal.compiler.codegen.OperandStack.NullStack.push(TypeBinding) simply says return

So I don't see this problem happening on the evolving 4.33 branch

(2) As @iloveeclipse mentioned, in any case soon levels 1.7 and below are not supported.

So I plan to close this as can't repro - unless you have a scenario that manifests the problem on master.

srikanth-sankaran commented 4 months ago

Summary: things have already moved along quite a ways that this is not an issue on master HEAD on date and for sometime.

alban-auzeill commented 4 months ago

(1) Do you still get the problem if this line options.put("org.eclipse.jdt.core.compiler.codegen.simulateOperandStack", "enabled"); is deleted ?

No, if one of those conditions is true, the problem is not present:

I am surprised to see this option being enabled!

It was not intentional. It is due to misuse of ASTParser#setCompilerOptions(options);. The tool I'm maintaining initializes options using new HashMap<>() instead of JavaCore. getOptions() (see here). Now that I have read the setCompilerOptions javadoc, I understand our mistake. That said, I'm surprised that a missing property in options like org.eclipse.jdt.core.compiler.codegen.simulateOperandStack is enabled when missing. It would be more forgivable if missing properties could have the same default as JavaCore.getOptions().

The same problem applies to org.eclipse.jdt.core.compiler.codegen.targetPlatform; we didn't want to provide an unsupported version 1.5 or below. But while parsing Java 21 source code we only define org.eclipse.jdt.core.compiler.compliance and org.eclipse.jdt.core.compiler.source and didn't provide any value for org.eclipse.jdt.core.compiler.codegen.targetPlatform. We will fix our mistake by using JavaCore.setComplianceOptions(JavaCore.VERSION_21, options);

That's why I mentioned in the issue description that it's a corner case. I believe that once we fix our usage of the compiler options, we should not be impacted by this issue.

srikanth-sankaran commented 4 months ago

Thanks for the detailed explanation @alban-auzeill - I'll close this without change since as already documented 1.7 and below support is going away very soon.