From d832edb7ce3a3e95d6f5cb61d724239dbbeb0c1b Mon Sep 17 00:00:00 2001 From: Christian Humer Date: Fri, 14 Mar 2025 12:12:10 +0100 Subject: [PATCH] Restore public access from inherited classes when public access is enabled. --- truffle/CHANGELOG.md | 1 + .../META-INF/native-image/reflect-config.json | 12 ++ .../truffle/api/test/host/GR63075Test.java | 113 ++++++++++++++++++ .../truffle/api/test/host/VisibilityTest.java | 6 +- .../test/polyglot/ValueHostInteropTest.java | 2 +- .../oracle/truffle/host/HostClassDesc.java | 32 ++++- 6 files changed, 159 insertions(+), 7 deletions(-) create mode 100644 truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/GR63075Test.java diff --git a/truffle/CHANGELOG.md b/truffle/CHANGELOG.md index 81d4856eb07b..1bc53b7d1400 100644 --- a/truffle/CHANGELOG.md +++ b/truffle/CHANGELOG.md @@ -5,6 +5,7 @@ This changelog summarizes major changes between Truffle versions relevant to lan ## Version 25.0 * GR-31495 Added ability to specify language and instrument specific options using `Source.Builder.option(String, String)`. Languages may describe available source options by implementing `TruffleLanguage.getSourceOptionDescriptors()` and `TruffleInstrument.getSourceOptionDescriptors()` respectively. * GR-61493 Added `RootNode.prepareForCall` which allows root nodes to prepare themselves for use as a call target (or to validate whether they can be used as a call target). +* GR-63075 Java host interop again inherits public method methods from non-public base classes if public access is enabled. This was originally changed in 24.1. ## Version 24.2.0 * GR-60636 Truffle now stops compiling when the code cache fills up on HotSpot. A warning is printed when that happens. diff --git a/truffle/src/com.oracle.truffle.api.test/src/META-INF/native-image/reflect-config.json b/truffle/src/com.oracle.truffle.api.test/src/META-INF/native-image/reflect-config.json index 304b507b6618..a3a84e0b6fd7 100644 --- a/truffle/src/com.oracle.truffle.api.test/src/META-INF/native-image/reflect-config.json +++ b/truffle/src/com.oracle.truffle.api.test/src/META-INF/native-image/reflect-config.json @@ -271,5 +271,17 @@ "methods": [ { "name": "getIsolate", "parameterTypes": ["java.lang.Object"] } ] + }, + { + "name": "com.oracle.truffle.api.test.host.GR63075Test$BaseClass", + "allPublicMethods": true + }, + { + "name": "com.oracle.truffle.api.test.host.GR63075Test$SubClass", + "allPublicMethods": true + }, + { + "name": "com.oracle.truffle.api.test.host.GR63075Test$NonPublicSubClass", + "allPublicMethods": true } ] diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/GR63075Test.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/GR63075Test.java new file mode 100644 index 000000000000..dfed50bc0c7b --- /dev/null +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/GR63075Test.java @@ -0,0 +1,113 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.truffle.api.test.host; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.HostAccess; +import org.graalvm.polyglot.HostAccess.Export; +import org.graalvm.polyglot.Value; +import org.junit.Test; + +public class GR63075Test { + + static class BaseClass { + + @Export + public String publicAbstractMethod() { + return "publicAbstractMethod"; + } + + } + + public static class SubClass extends BaseClass { + + @Export + public String publicMethod() { + return "publicMethod"; + } + + } + + static class NonPublicSubClass extends BaseClass { + + public String publicMethod() { + return "publicMethod"; + } + + } + + @Test + public void testPublicAccess() { + try (Context c = Context.newBuilder().allowHostAccess(HostAccess.ALL).build()) { + SubClass b = new SubClass(); + Value v = c.asValue(b); + + assertEquals("publicMethod", v.invokeMember("publicMethod").asString()); + assertEquals("publicAbstractMethod", v.invokeMember("publicAbstractMethod").asString()); + } + } + + @Test + public void testNonPublicAccess() { + try (Context c = Context.newBuilder().allowHostAccess(HostAccess.ALL).build()) { + NonPublicSubClass b = new NonPublicSubClass(); + Value v = c.asValue(b); + + assertNull(v.getMember("publicMethod")); + assertNull(v.getMember("publicAbstractMethod")); + } + } + + @Test + public void testExplicitAccess() { + try (Context c = Context.newBuilder().allowHostAccess(HostAccess.EXPLICIT).build()) { + SubClass b = new SubClass(); + Value v = c.asValue(b); + + assertEquals("publicMethod", v.invokeMember("publicMethod").asString()); + assertNull(v.getMember("publicAbstractMethod")); + } + } + +} diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/VisibilityTest.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/VisibilityTest.java index a769522a6549..5a17d8f15e50 100644 --- a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/VisibilityTest.java +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/host/VisibilityTest.java @@ -256,10 +256,8 @@ public void testStaticMethodNotInherited() throws InteropException { } @Test - public void testPublicClassBridgeMethod() { - // GR-42882: public bridge method B1.run() for A3.run() no longer exposed. - // invokeRun(new B1(), A3.class); - Assert.assertFalse(INTEROP.isMemberExisting(asTruffleObject(new B1()), "run")); + public void testPublicClassBridgeMethod() throws InteropException { + invokeRun(new B1(), A3.class); } @Test diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/ValueHostInteropTest.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/ValueHostInteropTest.java index a0a2fc2d1335..b6a2b1ac1d53 100644 --- a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/ValueHostInteropTest.java +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/ValueHostInteropTest.java @@ -52,7 +52,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import java.awt.*; +import java.awt.Color; import java.awt.image.BufferedImage; import java.lang.reflect.Method; import java.nio.ByteBuffer; diff --git a/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostClassDesc.java b/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostClassDesc.java index 3556a3abc1aa..7f9e2a438d14 100644 --- a/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostClassDesc.java +++ b/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostClassDesc.java @@ -217,6 +217,7 @@ private static void collectPublicMethods(HostClassCache hostAccess, Class typ Class startType) { boolean isPublicType = getLookupForPublicClass(type, hostAccess) != null && !Proxy.isProxyClass(type); boolean includeInherited = hostAccess.allowsPublicAccess || hostAccess.allowsAccessInheritance; + List bridgeMethods = null; /** * If we do not allow inheriting access, we discover all accessible methods through the * public methods of this class. That way, (possibly inaccessible) method overrides hide @@ -231,8 +232,26 @@ private static void collectPublicMethods(HostClassCache hostAccess, Class typ } else if (getLookupForPublicClass(declaringClass, hostAccess) == null && !Proxy.isProxyClass(declaringClass)) { // the declaring class and the method itself must be public and accessible continue; + } else if (m.isBridge() && hostAccess.allowsPublicAccess && hostAccess.allowsAccess(m)) { + /* + * Bridge methods for varargs methods generated by javac may not have the + * varargs modifier, so we must not use the bridge method in that case since + * it would be then treated as non-varargs. + * + * As a workaround, stash away all bridge methods and only consider them at + * the end if no equivalent public non-bridge method was found. + */ + if (bridgeMethods == null) { + bridgeMethods = new ArrayList<>(); + } + bridgeMethods.add(m); + continue; } else if (m.isBridge()) { - // skip all bridge methods + /* + * GR-42882] Without public access we skip all bridge methods. This is + * needed due to inconsistent behavior between javac and JDT. JDT does not + * inherit exported methods for bridge methods. + */ continue; } if (hostAccess.allowsAccess(m)) { @@ -255,9 +274,18 @@ private static void collectPublicMethods(HostClassCache hostAccess, Class typ } } } + // Add bridge methods for public methods inherited from non-public superclasses. + // See https://bugs.openjdk.java.net/browse/JDK-6342411 + if (bridgeMethods != null && !bridgeMethods.isEmpty()) { + for (Method m : bridgeMethods) { + assert hostAccess.allowsAccess(m); // already checked above + collectPublicMethod(hostAccess, methodMap, staticMethodMap, visited, m); + } + } } - private static void collectPublicMethod(HostClassCache hostAccess, Map methodMap, Map staticMethodMap, Map visited, Method m) { + private static void collectPublicMethod(HostClassCache hostAccess, Map methodMap, Map staticMethodMap, Map visited, + Method m) { MethodInfo methodInfo = methodInfo(m); if (!visited.containsKey(methodInfo)) { visited.put(methodInfo, methodInfo);