JDK-8189335: NPE in Lower due to class name clash
B. Blaser
bsrbnd at gmail.com
Sun Feb 25 12:50:08 UTC 2018
On 24 February 2018 at 21:39, B. Blaser <bsrbnd at gmail.com> wrote:
> Hi Vicente,
>
> Thanks for your answer.
>
> On 23 February 2018 at 22:24, Vicente Romero <vicente.romero at oracle.com> wrote:
>> Hi Bernard,
>>
>> Usually when we touch this area of the compiler we do the following test:
>> build the whole JDK, another big corpus could make it too, with and without
>> the fix applied and then we compare the class files obtained, if no new tag
>> classes are generated for cases where they shouldn't and if there are no
>> unjustified changes to the rest of the classes, then we are more confident
>> about the fix.
>
> You're right, this would be a very good test but I probably won't have
> time to do that soon.
> I also know that all jdk tests have to pass successfully and I only
> ran javac tests, sorry...
> However, I hope my work will still be helpful.
Moreover, I note that several existing javac issues & tests are
largely covering this area which make me rather confident about the
fix, especially:
https://bugs.openjdk.java.net/browse/JDK-6917288
test/langtools/tools/javac/6917288/GraphicalInstallerTest.java
test/langtools/tools/javac/6917288/T6917288.java
https://bugs.openjdk.java.net/browse/JDK-7199823
test/langtools/tools/javac/7199823/InnerClassCannotBeVerified.java
https://bugs.openjdk.java.net/browse/JDK-8040129
test/langtools/tools/javac/classfiles/attributes/SourceFile/SyntheticClassTest.java
https://bugs.openjdk.java.net/browse/JDK-8044537
http://hg.openjdk.java.net/jdk/jdk/file/02404e27d356/test/langtools/tools/javac/classfiles/attributes/Synthetic/AccessToPrivateInnerClassMembersTest.java#l39
test/langtools/tools/javac/classfiles/attributes/Synthetic/BridgeMethodsForLambdaTest.java
https://bugs.openjdk.java.net/browse/JDK-8042251
test/langtools/tools/javac/classfiles/attributes/innerclasses/InnerClassesInInnerClassTest.java
test/langtools/tools/javac/classfiles/attributes/innerclasses/InnerClassesInInnerEnumTest.java
test/langtools/tools/javac/classfiles/attributes/innerclasses/InnerClassesTest.java
Bernard
> Bernard
>
>> Thanks,
>> Vicente
>>
>>
>> On 02/22/2018 03:09 PM, B. Blaser wrote:
>>>
>>> On 21 February 2018 at 16:58, B. Blaser <bsrbnd at gmail.com> wrote:
>>>>
>>>> On 19 February 2018 at 15:53, B. Blaser <bsrbnd at gmail.com> wrote:
>>>>>
>>>>> On 19 February 2018 at 00:42, B. Blaser <bsrbnd at gmail.com> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> As explained in JDK-8189335, the following example crashes javac with
>>>>>> a NPE in Lower:
>>>>>>
>>>>>> class R {
>>>>>> private class R$1 {}
>>>>>> void f() {
>>>>>> new R$1();
>>>>>> }
>>>>>> }
>>>>>> class R$1 {}
>>>>
>>>> Some precisions related to my laconic explanation about the problem and
>>>> the fix.
>>>>
>>>> All starts in 'Lower.accessConstructorTag()' when searching for the
>>>> anonymous class 'R$1':
>>>>
>>>>
>>>> http://hg.openjdk.java.net/jdk/jdk/file/02404e27d356/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java#l1238
>>>>
>>>> The top level class 'R$1' (not anonymous) is found and wrongly added
>>>> to 'accessConstrTags'.
>>>>
>>>> Then, when checking the access constructor tags in
>>>> 'Lower.checkAccessConstructorTags()', the 'outermostClass()' of the
>>>> top level class 'R$1' is requested:
>>>>
>>>>
>>>> http://hg.openjdk.java.net/jdk/jdk/file/02404e27d356/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java#l778
>>>>
>>>> 'R$1' being a top level class, its outermost class doesn't exist,
>>>> causing the NPE in 'Lower.makeEmptyClass()'.
>>>
>>> Errata: 'R$1' being a top level class, its outermost class definition
>>> ('R$1') doesn't exist within 'R', causing the NPE in
>>> 'Lower.makeEmptyClass()':
>>>
>>>
>>> http://hg.openjdk.java.net/jdk/jdk/file/02404e27d356/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java#l658
>>>
>>>> The fix I suggested two days ago (here under) is then quite
>>>> straightforward. If a class 'R$1' exists but isn't anonymous, it has
>>>> to be skipped and a fresh anonymous class 'R$2' has to be created as
>>>> access constructor tag:
>>>>
>>>>
>>>> http://hg.openjdk.java.net/jdk/jdk/file/02404e27d356/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java#l1239
>>>>
>>>> Nothing seems to be broken and the provided example compiles
>>>> successfully.
>>>
>>> Here is a small variant of the previous patch that makes sure to reuse
>>> the same tag when multiple private constructors are present and 'R$1'
>>> isn't anonymous, for example:
>>>
>>> class R {
>>> private class A {}
>>> private class B {}
>>> void f() {
>>> new A();
>>> new B();
>>> }
>>> }
>>> class R$1{}
>>>
>>> Any comment or review is welcome!
>>>
>>> Cheers,
>>> Bernard
>>>
>>>
>>> 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
>>> @@ -1232,15 +1232,19 @@
>>> ClassSymbol accessConstructorTag() {
>>> ClassSymbol topClass = currentClass.outermostClass();
>>> ModuleSymbol topModle = topClass.packge().modle;
>>> - Name flatname = names.fromString("" + topClass.getQualifiedName()
>>> +
>>> - target.syntheticNameChar() +
>>> - "1");
>>> - ClassSymbol ctag = chk.getCompiled(topModle, flatname);
>>> - if (ctag == null)
>>> - ctag = makeEmptyClass(STATIC | SYNTHETIC, topClass).sym;
>>> - // keep a record of all tags, to verify that all are
>>> generated as required
>>> - accessConstrTags = accessConstrTags.prepend(ctag);
>>> - return ctag;
>>> + for (int i = 1; ; i++) {
>>> + Name flatname = names.fromString("" +
>>> topClass.getQualifiedName() +
>>> + target.syntheticNameChar() +
>>> + i);
>>> + ClassSymbol ctag = chk.getCompiled(topModle, flatname);
>>> + if (ctag == null)
>>> + ctag = makeEmptyClass(STATIC | SYNTHETIC, topClass).sym;
>>> + else if (!ctag.isAnonymous())
>>> + continue;
>>> + // keep a record of all tags, to verify that all are
>>> generated as required
>>> + accessConstrTags = accessConstrTags.prepend(ctag);
>>> + return ctag;
>>> + }
>>> }
>>>
>>> /** Add all required access methods for a private symbol to
>>> enclosing class.
>>>
>>>> What do you think (I hope this is clearer)?
>>>>
>>>> Thanks,
>>>> Bernard
>>>>
>>>>
>>>>> 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
>>>>> @@ -1236,7 +1236,7 @@
>>>>> target.syntheticNameChar() +
>>>>> "1");
>>>>> ClassSymbol ctag = chk.getCompiled(topModle, flatname);
>>>>> - if (ctag == null)
>>>>> + if (ctag == null || !ctag.isAnonymous())
>>>>> ctag = makeEmptyClass(STATIC | SYNTHETIC, topClass).sym;
>>>>> // keep a record of all tags, to verify that all are
>>>>> generated as required
>>>>> accessConstrTags = accessConstrTags.prepend(ctag);
>>
>>
More information about the compiler-dev
mailing list