From 55d363b451ac77c550f4cc0405831a2189de5a23 Mon Sep 17 00:00:00 2001 From: Loic Ottet Date: Mon, 13 Mar 2023 15:51:45 +0100 Subject: [PATCH] Simplify missing reflection registration options --- .../svm/core/hub/ClassForNameSupport.java | 2 +- .../com/oracle/svm/core/hub/DynamicHub.java | 40 +++--- .../MissingReflectionRegistrationUtils.java | 128 +++++++++++++----- .../Target_java_lang_reflect_Constructor.java | 2 +- .../Target_java_lang_reflect_Method.java | 2 +- .../svm/truffle/tck/PermissionsFeature.java | 1 + 6 files changed, 117 insertions(+), 58 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java index f08b4d7b8e8d..58a8850632fe 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java @@ -114,7 +114,7 @@ private static Class forName(String className, ClassLoader classLoader, boole } } else if (result == null) { if (throwMissingRegistrationErrors()) { - throw MissingReflectionRegistrationUtils.forClass(className); + MissingReflectionRegistrationUtils.forClass(className); } if (returnNullOnException) { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java index bd4ddb0dcfe1..e015eea7ea4e 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java @@ -527,7 +527,7 @@ public void setReflectionMetadata(int fieldsEncodingIndex, int methodsEncodingIn private void checkClassFlag(int mask, String methodName) { if (throwMissingRegistrationErrors() && !isClassFlagSet(mask)) { - throw MissingReflectionRegistrationUtils.forBulkQuery(DynamicHub.toClass(this), methodName); + MissingReflectionRegistrationUtils.forBulkQuery(DynamicHub.toClass(this), methodName); } } @@ -995,22 +995,22 @@ private void checkField(String fieldName, Field field, boolean publicOnly) throw Class clazz = DynamicHub.toClass(this); if (field == null) { if (throwMissingErrors && !allElementsRegistered(publicOnly, ALL_DECLARED_FIELDS_FLAG, ALL_FIELDS_FLAG)) { - throw MissingReflectionRegistrationUtils.forField(clazz, fieldName); - } else { - /* - * If getDeclaredFields (or getFields for a public field) is registered, we know for - * sure that the field does indeed not exist if we don't find it. - */ - throw new NoSuchFieldException(fieldName); + MissingReflectionRegistrationUtils.forField(clazz, fieldName); } + /* + * If getDeclaredFields (or getFields for a public field) is registered, we know for + * sure that the field does indeed not exist if we don't find it. + */ + throw new NoSuchFieldException(fieldName); } else { ReflectionMetadataDecoder decoder = ImageSingletons.lookup(ReflectionMetadataDecoder.class); int fieldModifiers = field.getModifiers(); boolean negative = decoder.isNegative(fieldModifiers); boolean hiding = decoder.isHiding(fieldModifiers); if (throwMissingErrors && hiding) { - throw MissingReflectionRegistrationUtils.forField(clazz, fieldName); - } else if (negative || hiding) { + MissingReflectionRegistrationUtils.forField(clazz, fieldName); + } + if (negative || hiding) { throw new NoSuchFieldException(fieldName); } } @@ -1029,22 +1029,22 @@ private void checkMethod(String methodName, Class[] parameterTypes, Executabl Class clazz = DynamicHub.toClass(this); if (method == null) { if (throwMissingErrors && !allElementsRegistered(publicOnly, ALL_DECLARED_METHODS_FLAG, ALL_METHODS_FLAG)) { - throw MissingReflectionRegistrationUtils.forMethod(clazz, methodName, parameterTypes); - } else { - /* - * If getDeclaredMethods (or getMethods for a public method) is registered, we know - * for sure that the method does indeed not exist if we don't find it. - */ - throw new NoSuchMethodException(methodToString(methodName, parameterTypes)); + MissingReflectionRegistrationUtils.forMethod(clazz, methodName, parameterTypes); } + /* + * If getDeclaredMethods (or getMethods for a public method) is registered, we know for + * sure that the method does indeed not exist if we don't find it. + */ + throw new NoSuchMethodException(methodToString(methodName, parameterTypes)); } else { ReflectionMetadataDecoder decoder = ImageSingletons.lookup(ReflectionMetadataDecoder.class); int methodModifiers = method.getModifiers(); boolean negative = decoder.isNegative(methodModifiers); boolean hiding = decoder.isHiding(methodModifiers); if (throwMissingErrors && hiding) { - throw MissingReflectionRegistrationUtils.forMethod(clazz, methodName, parameterTypes); - } else if (negative || hiding) { + MissingReflectionRegistrationUtils.forMethod(clazz, methodName, parameterTypes); + } + if (negative || hiding) { throw new NoSuchMethodException(methodToString(methodName, parameterTypes)); } } @@ -1487,7 +1487,7 @@ public DynamicHub arrayType() { throw new UnsupportedOperationException(new IllegalArgumentException()); } if (arrayHub == null) { - throw MissingReflectionRegistrationUtils.forClass(getTypeName() + "[]"); + MissingReflectionRegistrationUtils.forClass(getTypeName() + "[]"); } return arrayHub; } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java index bb3072cefed1..b35137d46fcc 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java @@ -28,7 +28,9 @@ import java.lang.reflect.Executable; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.util.Set; import java.util.StringJoiner; +import java.util.concurrent.ConcurrentHashMap; import org.graalvm.compiler.options.Option; import org.graalvm.nativeimage.MissingReflectionRegistrationError; @@ -39,37 +41,43 @@ public final class MissingReflectionRegistrationUtils { public static class Options { - @Option(help = "Enable termination caused by missing metadata.")// - public static final HostedOptionKey ExitOnMissingReflectionRegistration = new HostedOptionKey<>(false); + @Option(help = {"Select the mode in which the missing reflection registrations will be reported.", + "Possible values are:", + "\"Throw\" (default): Throw a MissingReflectionRegistrationError;", + "\"Exit\": Call System.exit() to avoid accidentally catching the error;", + "\"Warn\": Print a message to stdout, including a stack trace to see what caused the issue."})// + public static final HostedOptionKey MissingRegistrationReportingMode = new HostedOptionKey<>(ReportingMode.Throw); + } - @Option(help = "Simulate exiting the program with an exception instead of calling System.exit() (for testing)")// - public static final HostedOptionKey ExitWithExceptionOnMissingReflectionRegistration = new HostedOptionKey<>(false); + public enum ReportingMode { + Warn, + Throw, + ExitTest, + Exit } public static boolean throwMissingRegistrationErrors() { return SubstrateOptions.ThrowMissingRegistrationErrors.getValue(); } - public static MissingReflectionRegistrationError forClass(String className) { + public static ReportingMode missingRegistrationReportingMode() { + return Options.MissingRegistrationReportingMode.getValue(); + } + + public static void forClass(String className) { MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("access class", className), Class.class, null, className, null); - if (MissingReflectionRegistrationUtils.Options.ExitOnMissingReflectionRegistration.getValue()) { - exitOnMissingMetadata(exception); - } - return exception; + report(exception); } - public static MissingReflectionRegistrationError forField(Class declaringClass, String fieldName) { + public static void forField(Class declaringClass, String fieldName) { MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("access field", declaringClass.getTypeName() + "#" + fieldName), Field.class, declaringClass, fieldName, null); - if (MissingReflectionRegistrationUtils.Options.ExitOnMissingReflectionRegistration.getValue()) { - exitOnMissingMetadata(exception); - } - return exception; + report(exception); } - public static MissingReflectionRegistrationError forMethod(Class declaringClass, String methodName, Class[] paramTypes) { + public static void forMethod(Class declaringClass, String methodName, Class[] paramTypes) { StringJoiner paramTypeNames = new StringJoiner(", ", "(", ")"); for (Class paramType : paramTypes) { paramTypeNames.add(paramType.getTypeName()); @@ -77,29 +85,25 @@ public static MissingReflectionRegistrationError forMethod(Class declaringCla MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("access method", declaringClass.getTypeName() + "#" + methodName + paramTypeNames), Method.class, declaringClass, methodName, paramTypes); - if (MissingReflectionRegistrationUtils.Options.ExitOnMissingReflectionRegistration.getValue()) { - exitOnMissingMetadata(exception); - } - return exception; + report(exception); } - public static MissingReflectionRegistrationError forQueriedOnlyExecutable(Executable executable) { + public static void forQueriedOnlyExecutable(Executable executable) { MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("invoke method", executable.toString()), executable.getClass(), executable.getDeclaringClass(), executable.getName(), executable.getParameterTypes()); - if (MissingReflectionRegistrationUtils.Options.ExitOnMissingReflectionRegistration.getValue()) { - exitOnMissingMetadata(exception); - } - return exception; + report(exception); + /* + * If report doesn't throw, we throw the exception anyway since this is a Native + * Image-specific error that is unrecoverable in any case. + */ + throw exception; } - public static MissingReflectionRegistrationError forBulkQuery(Class declaringClass, String methodName) { + public static void forBulkQuery(Class declaringClass, String methodName) { MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("access", declaringClass.getTypeName() + "." + methodName + "()"), null, declaringClass, methodName, null); - if (MissingReflectionRegistrationUtils.Options.ExitOnMissingReflectionRegistration.getValue()) { - exitOnMissingMetadata(exception); - } - return exception; + report(exception); } private static String errorMessage(String failedAction, String elementDescriptor) { @@ -108,15 +112,69 @@ private static String errorMessage(String failedAction, String elementDescriptor "See https://www.graalvm.org/latest/reference-manual/native-image/metadata/#reflection for help."; } - private static void exitOnMissingMetadata(MissingReflectionRegistrationError exception) { - if (Options.ExitWithExceptionOnMissingReflectionRegistration.getValue()) { - throw new ExitException(exception); - } else { - exception.printStackTrace(System.out); - System.exit(ExitStatus.MISSING_METADATA.getValue()); + private static final int CONTEXT_LINES = 4; + + private static final Set seenOutputs = Options.MissingRegistrationReportingMode.getValue() == ReportingMode.Warn ? ConcurrentHashMap.newKeySet() : null; + + private static void report(MissingReflectionRegistrationError exception) { + switch (missingRegistrationReportingMode()) { + case Throw -> { + throw exception; + } + case Exit -> { + exception.printStackTrace(System.out); + System.exit(ExitStatus.MISSING_METADATA.getValue()); + } + case ExitTest -> { + throw new ExitException(exception); + } + case Warn -> { + StackTraceElement[] stackTrace = exception.getStackTrace(); + int printed = 0; + StackTraceElement entryPoint = null; + StringBuilder sb = new StringBuilder(exception.toString()); + sb.append("\n"); + for (StackTraceElement stackTraceElement : stackTrace) { + if (printed == 0) { + String moduleName = stackTraceElement.getModuleName(); + /* + * Skip internal stack trace entries to include only the relevant part of + * the trace in the output. The heuristic used is that any JDK and Graal + * code is excluded except the first element, so that the rest of the trace + * consists of meaningful application code entries. + */ + if (moduleName != null && (moduleName.equals("java.base") || moduleName.startsWith("org.graalvm"))) { + entryPoint = stackTraceElement; + } else { + printLine(sb, entryPoint); + printed++; + } + } + if (printed > 0) { + printLine(sb, stackTraceElement); + printed++; + } + if (printed >= CONTEXT_LINES) { + break; + } + } + if (seenOutputs.isEmpty()) { + /* First output, we print an explanation message */ + System.out.println("Note: this run will print partial stack traces of the locations where a MissingReflectionRegistrationError would be thrown " + + "when the -H:+ThrowMissingRegistrationErrors option is set. The trace stops at the first entry of JDK code and provides 4 lines of context."); + } + String output = sb.toString(); + if (seenOutputs.add(output)) { + System.out.print(output); + } + } } } + private static void printLine(StringBuilder sb, Object object) { + sb.append(" ").append(object).append(System.lineSeparator()); + } + public static final class ExitException extends Error { @Serial// private static final long serialVersionUID = -3638940737396726143L; diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Constructor.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Constructor.java index dc0d9b700f97..37feffb2c32c 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Constructor.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Constructor.java @@ -71,7 +71,7 @@ public native void constructor(Class declaringClass, Class[] parameterType @Substitute Target_jdk_internal_reflect_ConstructorAccessor acquireConstructorAccessor() { if (constructorAccessor == null) { - throw MissingReflectionRegistrationUtils.forQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class)); + MissingReflectionRegistrationUtils.forQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class)); } return constructorAccessor; } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Method.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Method.java index bd90ba19007b..420071579f90 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Method.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Method.java @@ -74,7 +74,7 @@ public native void constructor(Class declaringClass, String name, Class[] @Substitute public Target_jdk_internal_reflect_MethodAccessor acquireMethodAccessor() { if (methodAccessor == null) { - throw MissingReflectionRegistrationUtils.forQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class)); + MissingReflectionRegistrationUtils.forQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class)); } return methodAccessor; } diff --git a/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java b/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java index 9e5429a83f5f..ced4ce3b6e1c 100644 --- a/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java +++ b/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java @@ -139,6 +139,7 @@ public boolean getAsBoolean() { compilerPackages = new HashSet<>(); compilerPackages.add("org.graalvm."); compilerPackages.add("com.oracle.graalvm."); + compilerPackages.add("com.oracle.svm.core."); compilerPackages.add("com.oracle.truffle.api."); compilerPackages.add("com.oracle.truffle.polyglot."); compilerPackages.add("com.oracle.truffle.host.");