From 69f47d13f97302ddea87a44cd4adaf93e6907731 Mon Sep 17 00:00:00 2001 From: Matt D'Souza Date: Thu, 21 Nov 2024 15:39:40 -0500 Subject: [PATCH] Fix ConfigurationType method subtraction --- .../oracle/svm/configure/test/AddExports.java | 44 +++++++++++++++++++ .../test/config/OmitPreviousConfigTests.java | 14 +++++- .../test/config/config-dir/jni-config.json | 10 +++++ .../config/prev-config-dir/jni-config.json | 10 +++++ .../config/ConfigurationMemberInfo.java | 9 ++-- .../configure/config/ConfigurationType.java | 10 ++++- 6 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/AddExports.java diff --git a/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/AddExports.java b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/AddExports.java new file mode 100644 index 000000000000..8bffae533d69 --- /dev/null +++ b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/AddExports.java @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2024, 2024, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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 com.oracle.svm.configure.test; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Specifies packages concealed in JDK modules used by a test. The mx unit test runner will ensure + * the packages are exported to the module containing annotated test class. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface AddExports { + /** + * The qualified name of the concealed package in {@code /} format (e.g., + * "jdk.internal.vm.ci/jdk.vm.ci.code"). + */ + String[] value() default ""; +} diff --git a/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/OmitPreviousConfigTests.java b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/OmitPreviousConfigTests.java index d29330488c6e..8194335436f4 100644 --- a/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/OmitPreviousConfigTests.java +++ b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/OmitPreviousConfigTests.java @@ -52,10 +52,12 @@ import com.oracle.svm.configure.config.ResourceConfiguration; import com.oracle.svm.configure.config.SerializationConfiguration; import com.oracle.svm.configure.config.TypeConfiguration; +import com.oracle.svm.configure.test.AddExports; import com.oracle.svm.core.configure.ConfigurationTypeDescriptor; import com.oracle.svm.core.configure.NamedConfigurationTypeDescriptor; import com.oracle.svm.core.util.VMError; +@AddExports({"org.graalvm.nativeimage/org.graalvm.nativeimage.impl", "jdk.graal.compiler/jdk.graal.compiler.util"}) public class OmitPreviousConfigTests { private static final String PREVIOUS_CONFIG_DIR_NAME = "prev-config-dir"; @@ -117,7 +119,7 @@ public void testConfigDifference() { doTestResourceConfig(config.getResourceConfiguration()); - doTestSerializationConfig(config.getSerializationConfiguration()); + doTestSerializationConfig(config); doTestPredefinedClassesConfig(config.getPredefinedClassesConfiguration()); } @@ -181,6 +183,8 @@ private static void doTestMethods(TypeConfiguration typeConfig) { Assert.assertNull(ConfigurationType.TestBackdoor.getMethodInfoIfPresent(methodTestType, new ConfigurationMethod("", "(I)V"))); Assert.assertNotNull(ConfigurationType.TestBackdoor.getMethodInfoIfPresent(methodTestType, new ConfigurationMethod("method", "()V"))); + Assert.assertNull(ConfigurationType.TestBackdoor.getMethodInfoIfPresent(methodTestType, new ConfigurationMethod("method2", "()V"))); + Assert.assertNotNull(ConfigurationType.TestBackdoor.getMethodInfoIfPresent(methodTestType, new ConfigurationMethod("method3", "()V"))); } private static void doTestProxyConfig(ProxyConfiguration proxyConfig) { @@ -198,7 +202,13 @@ private static void doTestResourceConfig(ResourceConfiguration resourceConfig) { Assert.assertTrue(resourceConfig.anyBundleMatches(condition, "unseenBundle")); } - private static void doTestSerializationConfig(SerializationConfiguration serializationConfig) { + /* + * Note: the parameter cannot be a SerializationConfiguration because the type depends on some + * module exports (see the AddExports annotation) which only get applied _after_ the class is + * loaded. + */ + private static void doTestSerializationConfig(ConfigurationSet config) { + SerializationConfiguration serializationConfig = config.getSerializationConfiguration(); UnresolvedConfigurationCondition condition = UnresolvedConfigurationCondition.alwaysTrue(); Assert.assertFalse(serializationConfig.contains(condition, "seenType", null)); Assert.assertTrue(serializationConfig.contains(condition, "unseenType", null)); diff --git a/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/config-dir/jni-config.json b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/config-dir/jni-config.json index cbd1bab6e395..0b21c0bceaab 100644 --- a/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/config-dir/jni-config.json +++ b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/config-dir/jni-config.json @@ -55,6 +55,16 @@ { "name": "method", "parameterTypes": [] + }, + { + "name": "method3", + "parameterTypes": [] + } + ], + "queriedMethods": [ + { + "name": "method2", + "parameterTypes": [] } ] } diff --git a/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/prev-config-dir/jni-config.json b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/prev-config-dir/jni-config.json index 1ec4eaa4962d..48495e38ca2b 100644 --- a/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/prev-config-dir/jni-config.json +++ b/substratevm/src/com.oracle.svm.configure.test/src/com/oracle/svm/configure/test/config/prev-config-dir/jni-config.json @@ -49,6 +49,16 @@ "parameterTypes": [ "int" ] + }, + { + "name": "method2", + "parameterTypes": [] + } + ], + "queriedMethods": [ + { + "name": "method3", + "parameterTypes": [] } ] } diff --git a/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationMemberInfo.java b/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationMemberInfo.java index d2a9af19949d..aedbbbe05990 100644 --- a/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationMemberInfo.java +++ b/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationMemberInfo.java @@ -92,10 +92,11 @@ private ConfigurationMemberDeclaration union(ConfigurationMemberDeclaration othe } public boolean includes(ConfigurationMemberDeclaration other) { - if (equals(DECLARED_AND_PUBLIC)) { - return DECLARED.equals(other) || PUBLIC.equals(other); - } - return equals(other); + return switch (this) { + case PRESENT -> true; + case DECLARED_AND_PUBLIC -> other == DECLARED || other == PUBLIC; + default -> this == other; + }; } } diff --git a/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationType.java b/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationType.java index 39d0bcf00ca1..8d5e620eb639 100644 --- a/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationType.java +++ b/substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationType.java @@ -264,7 +264,15 @@ private void removeMethods(ConfigurationType other) { maybeRemoveMethods(allDeclaredMethodsAccess.combine(other.allDeclaredMethodsAccess), allPublicMethodsAccess.combine(other.allPublicMethodsAccess), allDeclaredConstructorsAccess.combine(other.allDeclaredConstructorsAccess), allPublicConstructorsAccess.combine(other.allPublicConstructorsAccess)); if (methods != null && other.methods != null) { - methods.entrySet().removeAll(other.methods.entrySet()); + for (Map.Entry entry : other.methods.entrySet()) { + ConfigurationMemberInfo otherMethodInfo = entry.getValue(); + methods.computeIfPresent(entry.getKey(), (method, methodInfo) -> { + if (otherMethodInfo.includes(methodInfo)) { + return null; // remove + } + return methodInfo; + }); + } if (methods.isEmpty()) { methods = null; }