JDK-8227415 : [nestmates] IllegalAccessError using a protected method reference declared in super class in different package
  • Type: Enhancement
  • Component: tools
  • Sub-Component: javac
  • Affected Version: repo-valhalla
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-07-08
  • Updated: 2020-04-03
  • Resolved: 2019-09-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
repo-valhallaFixed
Related Reports
Blocks :  
Relates :  
Relates :  
Relates :  
Description
In the valhalla nestmates branch, lambda metafactory is updated and replace the use of 
`Unsafe::defineAnonymousClass` with `Lookup::defineHiddenClass`.
The lambda proxy class is a hidden nestmate of the target class and it follows the proper access control checks.

In javac, LambdaToMethod::isProtectedInSuperClassOfEnclosingClassInOtherPackage determines
when to desugar a protected method reference to a static method.  In the following example, 
p.T.Sub and p.T.Sub1 are a subclass of q.I whereas p.T is not a subclass of q.I.

In Sub::test, super::readFile will get desugared to a static method in p.T.Sub to invoke q.I::readFile.
However, this::readFile does not get desugared and such a protected method reference passed to 
p.T::m, p.T has no access to q.I::readFile.   When the lambda proxy class of p.T is defined as a hidden
nestmate class, IAE is thrown as it fails to access q.I::readFile.

In Sub1::test, such a protected method reference passed to Sub1::test, Sub1 has access to q.I::readFile but the lambda proxy class does not since it's not a subclass of q.I.   In this case, it should be desugared as well.

------------------- p/T.java
package p;
import q.I;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.function.Function;

public class T {
    public static void main(String... args) throws Throwable {
        Path p = Paths.get("test");
        Sub sub = new Sub();
        sub.test(p);
        Sub1 sub1 = new Sub1();
        sub1.test(p);
    }

    private static void m(Function<Path,String> fileReader, Path path) {
        System.out.println(fileReader.apply(path));
    }

    public static class Sub extends I {
        public void test(Path outputDir) {
            // this method reference is desugared to a static method:
            //   REF_invokeSpecial p/T$Sub.lambda$test$0:(Ljava/nio/file/Path;)Ljava/lang/String;
            // as Sub is in package p, a different package from its its superclass
            m(super::readFile, outputDir);

            // this compiles to indy LMF with:
            //   REF_invokeVirtual q/I.readFile:(Ljava/nio/file/Path;)Ljava/lang/String;
            //
            // p.T is not a subclass of q.I and so it has no access to q.I::readFile 
            m(this::readFile, outputDir);
        }
    }

    public static class Sub1 extends I {
        public void test(Path outputDir) {
            test(this::readFile, outputDir);
        }
        private void test(Function<Path,String> fileReader, Path path) {
            System.out.println(fileReader.apply(path));
        }
    }
}

----- q/I.java 

package q;
import java.nio.file.Path;
public class I {
    protected String readFile(Path file) {
        return file.toString();
    }
}

-----------------------

Attached test.zip reproduces the error when running with the binary built from valhalla nestmates branch.
$ javac p/*.java q/*.java
$ java p.Main

Exception in thread "main" java.lang.IllegalAccessError: class p.Main$$Lambda$1$$/0x0000000800b76a70 tried to access protected method 'java.lang.String q.I.readFile(java.nio.file.Path)' (p.Main$$Lambda$1$$/0x0000000800b76a70 and q.I are in unnamed module of loader 'app')
	at q.J.checkFile(J.java:14)
	at q.J.check(J.java:18)
	at p.Main.test(Main.java:31)
	at p.Main.main(Main.java:13)

test/langtools/jdk/javadoc/doclet/testSingletonLists/TestSingletonLists.java also fails because of this issue.
Comments
There is an incompatibility concern when LambdaMetaFactory migrates to use hidden classes due to the missing access bridge to access the protected method reference declared in super class in different package. This fixes javac to generate an access bridge as p.Main for a protected method reference to q.I::readFile like this: private static java.lang.String lambda$test$0(p.Main, java.nio.file.Path); descriptor: (Lp/Main;Ljava/nio/file/Path;)Ljava/lang/String; flags: (0x100a) ACC_PRIVATE, ACC_STATIC, ACC_SYNTHETIC Code: stack=2, locals=2, args_size=2 0: aload_0 1: aload_1 2: invokevirtual #35 // Method readFile:(Ljava/nio/file/Path;)Ljava/lang/String; 5: areturn and p.Main::test method body has invokedynamic to LamdaMetaFactory with target method handle on lambda$test$0 rather than q.I::readFile. For classes compiled with existing releases, IllegalAccessError will be thrown when the lambda proxy class on behalf of p.Main accesses the protected method reference in a different package. Should we consider backporting this fix to an existing release such that existing application/library can recompile their classes to prepare in advance for the incompatibility? See https://mail.openjdk.java.net/pipermail/valhalla-dev/2019-October/006500.html
03-04-2020

Per JLS 13.4.7, changing `public` to `protected` on a member declaration is binary compatible for some accessors and not binary compatible for other accessors. Consulted with Alex on the following scenario should be binary compatible and no linkage error when a method in class D is changed from `public` to `protected` while the accessor class C is a supertype of D is not recompiled. When the accessor class `C` is a subtype of the accessee class D which declares the `protected` member; moreover, the code in `C` accesses the `protected` member through a receiver which is an instance of the accessor class (`C`). This meets the requirements for access to a `protected` member. As such, separately recompiling the supertype, so that it declares a `protected` member instead of a `public` member, must not cause a linkage error for the code in `C`. The change in the supertype is binary compatible from the point of view of this subtype; obviously the change in the supertype would not be binary compatible for millions of other classes. A JCK test uses a method reference of this form: `<instance of C> :: <method of C's supertype>` Because this method reference is in the subtype, and the "receiver" of the method reference is an instance of the subtype, the method reference must evaluate without error at run time, even if the method in the supertype is changed from `public` to `protected`. Having to recompile the subtype (`C`) to accommodate the recompiled supertype D is very unfortunate. Recompiling the supertype so that its member is inaccessible to most code but still accessible to subtypes, is a supported scenario in the Java language. For example, if the supertype was recompiled so that its member was package-private, you would expect code in the same package to be able to access it without recompiling.
20-02-2020

Fix and tests pushed here: http://hg.openjdk.java.net/valhalla/valhalla/rev/15515009b454
12-09-2019

VM anonymous class grants access to a protected member in its super class in a different package of the host class. The new Lookup::defineHiddenClass conforms to the proper access control checks. The lambda proxy class is a nestmate of the target class and it gets IAE when invoking super::protectedMember where its super class is in a different package from nestmate class. javac needs to desugar a cross-package method reference.
11-09-2019

Hi Mandy, Good to know your tests are passing in addition to the new ones I wrote. >>Can you help me understand why `!owner.enclClass().isSubClass(tree.sym.owner, types))` is needed when lambda proxy class is >>defined as unsafe VM anonymous class? This was introduced on behalf of JDK-8138667 - the condition was chosen to be as close to the problem there as possible. The whole needsConversionToLambda() infrastructure is to workaround various quirks/limitations/defects in the lambda metafactory/bootstrap mechanism. It just chooses an alternate code generation scheme that does not triggers those quirks/limitations/defects. That said, I don't have a very precise problem statement there as to why your code change replacing use of `Unsafe::defineAnonymousClass` with `Lookup::defineClass` should have triggered this problem in the first place. Given the code generated before this patch is identical in jdk/jdk tip where your test snippet just runs fine, do you have a theory as to why your code change brought about the failure ? >> Is there any other rule that should no longer apply when lambda proxy class conforms to VM access control rules? Not as far as I can tell.
11-09-2019

[~sadayapalam]. I applied the patch in the nestmates branch [1] and verified that this fixes test/langtools/jdk/javadoc/doclet/testSingletonLists/TestSingletonLists.java (that uncovered this issue). In addition, test/langtools/jdk/javadoc/doclet/testSearchScript/TestSearchScript.java and test/langtools/tools/javac/lambda/* all passed. boolean isProtectedInSuperClassOfEnclosingClassInOtherPackage() { return ((tree.sym.flags() & PROTECTED) != 0 && - tree.sym.packge() != owner.packge() && - !owner.enclClass().isSubClass(tree.sym.owner, types)); + tree.sym.packge() != owner.packge()); } Can you help me understand why `!owner.enclClass().isSubClass(tree.sym.owner, types))` is needed when lambda proxy class is defined as unsafe VM anonymous class? Is there any other rule that should no longer apply when lambda proxy class conforms to VM access control rules? [1] http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-8227415
10-09-2019

The following patch fixes the problem and passes langtools test except for one failure which I am investigating: (test/langtools/jdk/javadoc/doclet/testSearchScript/TestSearchScript.java) diff -r 4be2cb65666f 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 Thu Aug 29 19:32:19 2019 -0700 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java Tue Sep 10 14:49:24 2019 +0530 @@ -2254,8 +2254,7 @@ boolean isProtectedInSuperClassOfEnclosingClassInOtherPackage() { return ((tree.sym.flags() & PROTECTED) != 0 && - tree.sym.packge() != owner.packge() && - !owner.enclClass().isSubClass(tree.sym.owner, types)); + tree.sym.packge() != owner.packge()); } /** diff -r 4be2cb65666f test/langtools/tools/javac/lambda/methodReference/ProtectedInaccessibleMethodRefTest2.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/lambda/methodReference/ProtectedInaccessibleMethodRefTest2.java Tue Sep 10 14:49:24 2019 +0530 @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8227415 + * @summary javac needs to desugar a wrapper method for a method reference to a protected method declared in super class + * @run main ProtectedInaccessibleMethodRefTest2 + */ + +import pack.I; +import pack.J; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.function.Function; + +import java.util.concurrent.Callable; + +public final class ProtectedInaccessibleMethodRefTest2 extends I { + + public static void main(String... args) { + ProtectedInaccessibleMethodRefTest2 m = new ProtectedInaccessibleMethodRefTest2(); + m.test(Paths.get("test")); + } + + void test(Path outputDir) { + Sub c = new Sub(this::readFile); + c.check(outputDir); + } + public class Sub extends J { + Sub(Function<Path,String> fileReader) { + super(fileReader); + } + } +} diff -r 4be2cb65666f test/langtools/tools/javac/lambda/methodReference/pack/I.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/lambda/methodReference/pack/I.java Tue Sep 10 14:49:24 2019 +0530 @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package pack; + +import java.nio.file.Path; + +public class I { + protected String readFile(Path file) { + return file.toString(); + } +} diff -r 4be2cb65666f test/langtools/tools/javac/lambda/methodReference/pack/J.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/lambda/methodReference/pack/J.java Tue Sep 10 14:49:24 2019 +0530 @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package pack; + +import java.nio.file.Path; +import java.util.function.Function; + +public class J { + protected final Function<Path,String> fileReader; + + public J(Function<Path,String> fileReader) { + this.fileReader = fileReader; + } + + protected void checkFile(Path file) { + fileReader.apply(file); + } + + public void check(Path file) { + checkFile(file); + } +}
10-09-2019

Very similar to JDK-8138667. However the new change in nestmates branch necessitates further change
10-09-2019

Here are some observations: The code generated is identical between jdk/jdk and valhalla nestmates branch. However, the submitted test case passes fine on jdk/jdk while fails with java.lang.IllegalAccessError in valhalla branch. I conclude that this is due to a valhalla nestmates branch specific change (per above "In the valhalla nestmates branch, lambda metafactory is updated not to use `Unsafe::defineAnonymousClass` and replaced with `Lookup::defineClass`. The lambda proxy class is a nestmate of the target class and it follows the proper access control checks.) As a result, I conclude that the fix ought to be specific to valhalla/nestmates. I do have such a fix under test. Mandy, do you disagree with a nest mates specific branch for some reason despite the test passing fine on jdk/jdk ? I can push the fix after testing concludes and after the nestmates branch's broken merge changeset issue is addressed and we have a go ahead for push/pulls
10-09-2019

Documenting private communication with Mandy: Q: Could you clarify where a fix is expected ? In valhalla/nestmates branch ? I ask because Michel has targetted it for 14. A: This can be pushed to 14 while the test case should be verified in valhalla/nestmates branch to ensure the issue is resolved.
10-09-2019