[Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

JiaYanwei
Hi all,

I had found & fixed a javac issue like https://bugs.openjdk.java.net/browse/JDK-8229862 , and created a PR on Github: https://github.com/openjdk/jdk/pull/1479 .

I have no account to create JIRA issue, and I'm not sure whether the code style as well as the solution itself are appropriate. Could anyone sponsor me? Thanks a lot.

Best Regards.

---

The Issue:
javac crashes with a NPE when compiling such code:

- a local class captured a reference type local variable of a enclosed lambda (which contains multiple local variables with different types)
- instance the local class in a nested lambda
Affects all versions from Java 8 to 16-ea, here is a example output with OpenJDK 15.0.1:

An exception has occurred in the compiler (15.0.1). Please file a bug against the Java compiler via the Java bug reporting page (http://bugreport.java.com) after checking the Bug Database (http://bugs.java.com) for duplicates. Include your program, the following diagnostic, and the parameters passed to the Java compiler in your report. Thank you.
java.lang.NullPointerException: Cannot read field "sym" because "this.lvar[1]" is null
    at jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)
    at jdk.compiler/com.sun.tools.javac.jvm.Items$LocalItem.load(Items.java:399)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genArgs(Gen.java:889)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitNewClass(Gen.java:1942)
    at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCNewClass.accept(JCTree.java:1800)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genExpr(Gen.java:864)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitExec(Gen.java:1723)
    at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCExpressionStatement.accept(JCTree.java:1532)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:597)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:632)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:618)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStats(Gen.java:669)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitBlock(Gen.java:1084)
    at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1047)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:597)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:632)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genMethod(Gen.java:954)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitMethodDef(Gen.java:917)
    at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:893)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:597)
    at jdk.compiler/com.sun.tools.javac.jvm.Gen.genClass(Gen.java:2395)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.genCode(JavaCompiler.java:756)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.generate(JavaCompiler.java:1644)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.generate(JavaCompiler.java:1612)
    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:317)
    at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:176)
    at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:59)
    at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:45)
printing javac parameters to: /tmp/javac.20201126_141249.args

The minimal reproducing code is:

class Main {
    static Runnable runnable = () -> {
        boolean b0 = false;
        String s0 = "hello";

        class Local {
            int i = s0.length();
        }

        Runnable dummy = () -> new Local();
    };
}

If the captured variable is a primitive type, the code will compile but cause a runtime crash at startup. e.g.:

public class CaptureInt {
    static Runnable runnable = () -> {
        boolean b0 = false;
        int i0 = 5;

        class Local {
            int i = i0 + 2;
        }

        Runnable dummy = () -> new Local();
    };

    public static void main(String args[]) {
    }
}

Reason & Solution:

During compilation, the captured variable was correctly synthetized as a synthetic argument in the synthetic methods for the nested lambda after desugar().
But the synthetic argument was not used when loading free variables, and it always use the original symbol for the original variable as if not in a nested lambda.
My experimental solution is substituting the symbols in free variables that were captured as synthetic arguments in nested lambda.
Welcome any advice and better solution.


Reply | Threaded
Open this post in threaded view
|

Re: [Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

B. Blaser
Hi,

On Sun, 20 Dec 2020 at 10:49, JiaYanwei <[hidden email]> wrote:
>
> Hi all,
>
> I had found & fixed a javac issue like https://bugs.openjdk.java.net/browse/JDK-8229862 , and created a PR on Github: https://github.com/openjdk/jdk/pull/1479 .
>
> I have no account to create JIRA issue, and I'm not sure whether the code style as well as the solution itself are appropriate. Could anyone sponsor me? Thanks a lot.
>
> Best Regards.

The problem seems to be that 'lambdaTranslationMap' links the
untranslated local variable to its capture. So, what do you think of
the following solution to keep the translated local variable's mapping
with the original symbol (both examples and langtools:tier1 are OK on
jdk14u)?

Thanks,
Bernard

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
@@ -2057,7 +2057,13 @@
                         };
                         break;
                     case LOCAL_VAR:
-                        ret = new VarSymbol(sym.flags() & FINAL,
sym.name, sym.type, translatedSym);
+                        ret = new VarSymbol(sym.flags() & FINAL,
sym.name, sym.type, translatedSym) {
+                            @Override
+                            public Symbol baseSymbol() {
+                                //keep mapping with original symbol
+                                return sym;
+                            }
+                        };
                         ((VarSymbol) ret).pos = ((VarSymbol) sym).pos;
                         break;
                     case PARAM:
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
@@ -1221,7 +1221,7 @@
                 //sym is a local variable - check the lambda translation map to
                 //see if sym has been translated to something else in
the current
                 //scope (by LambdaToMethod)
-                Symbol translatedSym = lambdaTranslationMap.get(sym);
+                Symbol translatedSym =
lambdaTranslationMap.get(sym.baseSymbol());
                 if (translatedSym != null) {
                     tree = make.at(tree.pos).Ident(translatedSym);
                 }
Reply | Threaded
Open this post in threaded view
|

Re: [Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

JiaYanwei
Great! I think it's the best solution and can also be directly backported to 16, 11 & 8.

Should I close (i.e. stop/cancel) the Github PR, or may I commit this code with you(B. Blaser) as the author - just like what Jan Lahoda did in Github PR 1221?


B. Blaser <[hidden email]> 于2020年12月22日周二 上午8:56写道:
Hi,

On Sun, 20 Dec 2020 at 10:49, JiaYanwei <[hidden email]> wrote:
>
> Hi all,
>
> I had found & fixed a javac issue like https://bugs.openjdk.java.net/browse/JDK-8229862 , and created a PR on Github: https://github.com/openjdk/jdk/pull/1479 .
>
> I have no account to create JIRA issue, and I'm not sure whether the code style as well as the solution itself are appropriate. Could anyone sponsor me? Thanks a lot.
>
> Best Regards.

The problem seems to be that 'lambdaTranslationMap' links the
untranslated local variable to its capture. So, what do you think of
the following solution to keep the translated local variable's mapping
with the original symbol (both examples and langtools:tier1 are OK on
jdk14u)?

Thanks,
Bernard

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
@@ -2057,7 +2057,13 @@
                         };
                         break;
                     case LOCAL_VAR:
-                        ret = new VarSymbol(sym.flags() & FINAL,
sym.name, sym.type, translatedSym);
+                        ret = new VarSymbol(sym.flags() & FINAL,
sym.name, sym.type, translatedSym) {
+                            @Override
+                            public Symbol baseSymbol() {
+                                //keep mapping with original symbol
+                                return sym;
+                            }
+                        };
                         ((VarSymbol) ret).pos = ((VarSymbol) sym).pos;
                         break;
                     case PARAM:
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
@@ -1221,7 +1221,7 @@
                 //sym is a local variable - check the lambda translation map to
                 //see if sym has been translated to something else in
the current
                 //scope (by LambdaToMethod)
-                Symbol translatedSym = lambdaTranslationMap.get(sym);
+                Symbol translatedSym =
lambdaTranslationMap.get(sym.baseSymbol());
                 if (translatedSym != null) {
                     tree = make.at(tree.pos).Ident(translatedSym);
                 }
Reply | Threaded
Open this post in threaded view
|

Re: [Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

B. Blaser
Hi Jia,

On Wed, 23 Dec 2020 at 05:49, JiaYanwei <[hidden email]> wrote:
>
> Great! I think it's the best solution and can also be directly backported to 16, 11 & 8.
>
> Should I close (i.e. stop/cancel) the Github PR, or may I commit this code with you(B. Blaser) as the author - just like what Jan Lahoda did in Github PR 1221?

Thanks for your feedback, I filed
https://bugs.openjdk.java.net/browse/JDK-8258897 and assigned it to
Jan (I hope he won't mind). So, I'll let him or maybe Jon advise on
what to do with the pull request.

Regards,
Bernard
Reply | Threaded
Open this post in threaded view
|

Re: [Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

JiaYanwei
Hi Bernard,

Thank you!

Best Regards,
Yanwei

B. Blaser <[hidden email]> 于2020年12月23日周三 下午11:32写道:
Hi Jia,

On Wed, 23 Dec 2020 at 05:49, JiaYanwei <[hidden email]> wrote:
>
> Great! I think it's the best solution and can also be directly backported to 16, 11 & 8.
>
> Should I close (i.e. stop/cancel) the Github PR, or may I commit this code with you(B. Blaser) as the author - just like what Jan Lahoda did in Github PR 1221?

Thanks for your feedback, I filed
https://bugs.openjdk.java.net/browse/JDK-8258897 and assigned it to
Jan (I hope he won't mind). So, I'll let him or maybe Jon advise on
what to do with the pull request.

Regards,
Bernard
Reply | Threaded
Open this post in threaded view
|

Re: [Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

Jonathan Gibbons
In reply to this post by B. Blaser
Bernard, Jia,

You need an expert in this part of the code to do the review, and the likely
experts are on vacation through the end of the year and may not be able
to respond before the first week of next year.

As a matter of policy, if folk (you+reviewers) think this should go in 16,
you need to file the PR against the jdk16 repo. From there, it will
eventually
and automatically be forward-ported to the main jdk rep.

Note there will only be a limited window in the new year to fix bugs in 16.
See the schedule for JDK 16 here:
https://openjdk.java.net/projects/jdk/16/

-- Jon

On 12/23/20 7:32 AM, B. Blaser wrote:

> Hi Jia,
>
> On Wed, 23 Dec 2020 at 05:49, JiaYanwei <[hidden email]> wrote:
>> Great! I think it's the best solution and can also be directly backported to 16, 11 & 8.
>>
>> Should I close (i.e. stop/cancel) the Github PR, or may I commit this code with you(B. Blaser) as the author - just like what Jan Lahoda did in Github PR 1221?
> Thanks for your feedback, I filed
> https://bugs.openjdk.java.net/browse/JDK-8258897 and assigned it to
> Jan (I hope he won't mind). So, I'll let him or maybe Jon advise on
> what to do with the pull request.
>
> Regards,
> Bernard
Reply | Threaded
Open this post in threaded view
|

Re: [Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

B. Blaser
Hi,

It seems the following fix is still not integrated although the pull
request has been reviewed and looks ready for integration:

https://git.openjdk.java.net/jdk/pull/1479

Is any sponsor available?

Thanks,
Bernard

On Wed, 23 Dec 2020 at 20:39, Jonathan Gibbons
<[hidden email]> wrote:

>
> Bernard, Jia,
>
> You need an expert in this part of the code to do the review, and the likely
> experts are on vacation through the end of the year and may not be able
> to respond before the first week of next year.
>
> As a matter of policy, if folk (you+reviewers) think this should go in 16,
> you need to file the PR against the jdk16 repo. From there, it will
> eventually
> and automatically be forward-ported to the main jdk rep.
>
> Note there will only be a limited window in the new year to fix bugs in 16.
> See the schedule for JDK 16 here:
> https://openjdk.java.net/projects/jdk/16/
>
> -- Jon
>
> On 12/23/20 7:32 AM, B. Blaser wrote:
> > Hi Jia,
> >
> > On Wed, 23 Dec 2020 at 05:49, JiaYanwei <[hidden email]> wrote:
> >> Great! I think it's the best solution and can also be directly backported to 16, 11 & 8.
> >>
> >> Should I close (i.e. stop/cancel) the Github PR, or may I commit this code with you(B. Blaser) as the author - just like what Jan Lahoda did in Github PR 1221?
> > Thanks for your feedback, I filed
> > https://bugs.openjdk.java.net/browse/JDK-8258897 and assigned it to
> > Jan (I hope he won't mind). So, I'll let him or maybe Jon advise on
> > what to do with the pull request.
> >
> > Regards,
> > Bernard
Reply | Threaded
Open this post in threaded view
|

Re: [External] : Re: [Ask for sponsoring] Fix another NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)

Jan Lahoda
Done. Sorry I missed it.


Jan


On 26. 02. 21 14:07, B. Blaser wrote:

> Hi,
>
> It seems the following fix is still not integrated although the pull
> request has been reviewed and looks ready for integration:
>
> https://git.openjdk.java.net/jdk/pull/1479
>
> Is any sponsor available?
>
> Thanks,
> Bernard
>
> On Wed, 23 Dec 2020 at 20:39, Jonathan Gibbons
> <[hidden email]> wrote:
>> Bernard, Jia,
>>
>> You need an expert in this part of the code to do the review, and the likely
>> experts are on vacation through the end of the year and may not be able
>> to respond before the first week of next year.
>>
>> As a matter of policy, if folk (you+reviewers) think this should go in 16,
>> you need to file the PR against the jdk16 repo. From there, it will
>> eventually
>> and automatically be forward-ported to the main jdk rep.
>>
>> Note there will only be a limited window in the new year to fix bugs in 16.
>> See the schedule for JDK 16 here:
>> https://openjdk.java.net/projects/jdk/16/
>>
>> -- Jon
>>
>> On 12/23/20 7:32 AM, B. Blaser wrote:
>>> Hi Jia,
>>>
>>> On Wed, 23 Dec 2020 at 05:49, JiaYanwei <[hidden email]> wrote:
>>>> Great! I think it's the best solution and can also be directly backported to 16, 11 & 8.
>>>>
>>>> Should I close (i.e. stop/cancel) the Github PR, or may I commit this code with you(B. Blaser) as the author - just like what Jan Lahoda did in Github PR 1221?
>>> Thanks for your feedback, I filed
>>> https://bugs.openjdk.java.net/browse/JDK-8258897 and assigned it to
>>> Jan (I hope he won't mind). So, I'll let him or maybe Jon advise on
>>> what to do with the pull request.
>>>
>>> Regards,
>>> Bernard