From e9b1b83fc3c9d611020fed97bd0b0161fe75862b Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Thu, 19 Dec 2019 19:50:58 -0800 Subject: [PATCH] Provide a good error message when MethodHandle.invoke methods are reachable --- .../Target_java_lang_invoke_MethodHandle.java | 58 ---------- .../svm/hosted/NativeImageGenerator.java | 2 +- ...trinsifyMethodHandlesInvocationPlugin.java | 101 +++++++++++++----- .../IntrinsificationPluginRegistry.java | 96 +++++++++++++++++ .../hosted/snippets/ReflectionPlugins.java | 75 +------------ 5 files changed, 177 insertions(+), 155 deletions(-) delete mode 100644 substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_invoke_MethodHandle.java create mode 100644 substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/IntrinsificationPluginRegistry.java diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_invoke_MethodHandle.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_invoke_MethodHandle.java deleted file mode 100644 index aebb45ba53ca..000000000000 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_invoke_MethodHandle.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright (c) 2019, 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. 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.core.jdk; - -import com.oracle.svm.core.annotate.Delete; -import com.oracle.svm.core.annotate.TargetClass; - -@TargetClass(className = "java.lang.invoke.MethodHandle") -final class Target_java_lang_invoke_MethodHandle { - - /* - * We are defensive and handle native methods by marking them as deleted. If they are reachable, - * the user is certainly doing something wrong. But we do not want to fail with a linking error. - */ - - @Delete - private native Object invokeExact(Object... args) throws Throwable; - - @Delete - private native Object invoke(Object... args) throws Throwable; - - @Delete - private native Object invokeBasic(Object... args) throws Throwable; - - @Delete - private static native Object linkToVirtual(Object... args) throws Throwable; - - @Delete - private static native Object linkToStatic(Object... args) throws Throwable; - - @Delete - private static native Object linkToSpecial(Object... args) throws Throwable; - - @Delete - private static native Object linkToInterface(Object... args) throws Throwable; -} diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index 64b22cfdd72b..fc2bf768e836 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -1091,7 +1091,7 @@ public static void registerGraphBuilderPlugins(FeatureHandler featureHandler, Ru SubstrateReplacements replacements = (SubstrateReplacements) providers.getReplacements(); plugins.appendInlineInvokePlugin(replacements); - plugins.appendNodePlugin(new IntrinsifyMethodHandlesInvocationPlugin(providers, aUniverse, hUniverse)); + plugins.appendNodePlugin(new IntrinsifyMethodHandlesInvocationPlugin(analysis, providers, aUniverse, hUniverse)); plugins.appendNodePlugin(new DeletedFieldsPlugin()); plugins.appendNodePlugin(new InjectedAccessorsPlugin()); plugins.appendNodePlugin(new ConstantFoldLoadFieldPlugin(classInitializationSupport)); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java index a2b8d086f7aa..1a60b8fa6972 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java @@ -29,7 +29,10 @@ import java.lang.invoke.MethodHandle; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.HashSet; import java.util.Objects; +import java.util.Set; import org.graalvm.compiler.api.replacements.SnippetReflectionProvider; import org.graalvm.compiler.core.common.type.ObjectStamp; @@ -84,6 +87,7 @@ import org.graalvm.compiler.replacements.MethodHandlePlugin; import org.graalvm.compiler.serviceprovider.JavaVersionUtil; import org.graalvm.compiler.word.WordOperationPlugin; +import org.graalvm.nativeimage.ImageSingletons; import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException; import com.oracle.graal.pointsto.meta.AnalysisUniverse; @@ -100,6 +104,7 @@ import com.oracle.svm.hosted.SVMHost; import com.oracle.svm.hosted.c.GraalAccess; import com.oracle.svm.hosted.meta.HostedUniverse; +import com.oracle.svm.hosted.snippets.IntrinsificationPluginRegistry; import com.oracle.svm.util.ReflectionUtil; import jdk.vm.ci.meta.Constant; @@ -144,6 +149,11 @@ * HotSpot world. */ public class IntrinsifyMethodHandlesInvocationPlugin implements NodePlugin { + + static class IntrinsificationRegistry extends IntrinsificationPluginRegistry { + } + + private final boolean analysis; private final Providers originalProviders; private final Providers universeProviders; private final AnalysisUniverse aUniverse; @@ -151,6 +161,11 @@ public class IntrinsifyMethodHandlesInvocationPlugin implements NodePlugin { private final ClassInitializationPlugin classInitializationPlugin; + private final IntrinsificationRegistry intrinsificationRegistry; + + private final ResolvedJavaType methodHandleType; + private final Set methodHandleInvokeMethodNames; + private final Class varHandleClass; private final ResolvedJavaType varHandleType; private final Field varHandleVFormField; @@ -158,7 +173,8 @@ public class IntrinsifyMethodHandlesInvocationPlugin implements NodePlugin { private static final Method unsupportedFeatureMethod = ReflectionUtil.lookupMethod(VMError.class, "unsupportedFeature", String.class); - public IntrinsifyMethodHandlesInvocationPlugin(Providers providers, AnalysisUniverse aUniverse, HostedUniverse hUniverse) { + public IntrinsifyMethodHandlesInvocationPlugin(boolean analysis, Providers providers, AnalysisUniverse aUniverse, HostedUniverse hUniverse) { + this.analysis = analysis; this.aUniverse = aUniverse; this.hUniverse = hUniverse; this.universeProviders = providers; @@ -166,6 +182,16 @@ public IntrinsifyMethodHandlesInvocationPlugin(Providers providers, AnalysisUniv this.classInitializationPlugin = new SubstrateClassInitializationPlugin((SVMHost) aUniverse.hostVM()); + if (analysis) { + intrinsificationRegistry = new IntrinsificationRegistry(); + ImageSingletons.add(IntrinsificationRegistry.class, intrinsificationRegistry); + } else { + intrinsificationRegistry = ImageSingletons.lookup(IntrinsificationRegistry.class); + } + + methodHandleType = universeProviders.getMetaAccess().lookupJavaType(java.lang.invoke.MethodHandle.class); + methodHandleInvokeMethodNames = new HashSet<>(Arrays.asList("invokeExact", "invoke", "invokeBasic", "linkToVirtual", "linkToStatic", "linkToSpecial", "linkToInterface")); + if (JavaVersionUtil.JAVA_SPEC >= 11) { try { varHandleClass = Class.forName("java.lang.invoke.VarHandle"); @@ -193,7 +219,19 @@ public boolean handleInvoke(GraphBuilderContext b, ResolvedJavaMethod method, Va if (b.getInvokeKind().isDirect() && (hasMethodHandleArgument(args) || isVarHandleMethod(method, args))) { processInvokeWithMethodHandle(b, universeProviders.getReplacements(), method, args); return true; + + } else if (methodHandleType.equals(method.getDeclaringClass()) && methodHandleInvokeMethodNames.contains(method.getName())) { + /* + * The native methods defined in the class MethodHandle are currently not implemented at + * all. Normally, we would mark them as @Delete to give the user a good error message. + * Unfortunately, that does not work for the MethodHandle methods because they are + * signature polymorphic, i.e., they exist in every possible signature. Therefore, we + * must only look at the declaring class and the method name here. + */ + reportUnsupportedFeature(b, method); + return true; } + return false; } @@ -452,39 +490,30 @@ private void processInvokeWithMethodHandle(GraphBuilderContext b, Replacements r for (Node node : graph.getNodes()) { if (node == graph.start() || node instanceof ParameterNode || node instanceof ConstantNode || node instanceof FrameState) { /* Ignore the allowed framework around the nodes we care about. */ - continue; } else if (node instanceof MethodCallTargetNode) { /* We check the Invoke, so we can ignore the call target. */ - continue; } else if ((node instanceof Invoke || node instanceof LoadFieldNode || node instanceof StoreFieldNode) && singleFunctionality == null) { singleFunctionality = node; - continue; } else if (node instanceof ReturnNode && singleReturn == null) { singleReturn = (ReturnNode) node; - continue; - } - - String message = "Invoke with MethodHandle argument could not be reduced to at most a single call: " + methodHandleMethod.format("%H.%n(%p)"); - - if (NativeImageOptions.ReportUnsupportedElementsAtRuntime.getValue()) { - /* - * Ensure that we have space on the expression stack for the (unused) return - * value of the invoke. - */ - ((BytecodeParser) b).getFrameStateBuilder().clearStack(); - b.handleReplacedInvoke(InvokeKind.Static, b.getMetaAccess().lookupJavaMethod(unsupportedFeatureMethod), - new ValueNode[]{ConstantNode.forConstant(SubstrateObjectConstant.forObject(message), b.getMetaAccess(), b.getGraph())}, false); - /* The invoked method throws an exception and therefore never returns. */ - b.append(new DeadEndNode()); - return; - } else { - throw new UnsupportedFeatureException(message + System.lineSeparator() + "To diagnose the issue, you can add the option " + - SubstrateOptionsParser.commandArgument(NativeImageOptions.ReportUnsupportedElementsAtRuntime, "+") + - ". The error is then reported at run time when the invoke is executed."); + reportUnsupportedFeature(b, methodHandleMethod); + return; } } + /* + * When parsing for compilation, we must not intrinsify method handles that were not + * intrinsified during analysis. Otherwise new code that was no seen as reachable by the + * static analysis would be compiled. + */ + if (analysis) { + intrinsificationRegistry.add(b.getMethod(), b.bci(), Boolean.TRUE); + } else if (intrinsificationRegistry.get(b.getMethod(), b.bci()) != Boolean.TRUE) { + reportUnsupportedFeature(b, methodHandleMethod); + return; + } + JavaKind returnResultKind = b.getInvokeReturnType().getJavaKind().getStackKind(); ValueNode transplantedSingleFunctionality = null; if (singleFunctionality instanceof Invoke) { @@ -551,6 +580,30 @@ private void processInvokeWithMethodHandle(GraphBuilderContext b, Replacements r } } + private static void reportUnsupportedFeature(GraphBuilderContext b, ResolvedJavaMethod methodHandleMethod) { + String message = "Invoke with MethodHandle argument could not be reduced to at most a single call or single field access. " + + "The method handle must be a compile time constant, e.g., be loaded from a `static final` field. " + + "Method that contains the method handle invocation: " + methodHandleMethod.format("%H.%n(%p)"); + + if (NativeImageOptions.ReportUnsupportedElementsAtRuntime.getValue()) { + /* + * Ensure that we have space on the expression stack for the (unused) return value of + * the invoke. + */ + ((BytecodeParser) b).getFrameStateBuilder().clearStack(); + b.handleReplacedInvoke(InvokeKind.Static, b.getMetaAccess().lookupJavaMethod(unsupportedFeatureMethod), + new ValueNode[]{ConstantNode.forConstant(SubstrateObjectConstant.forObject(message), b.getMetaAccess(), b.getGraph())}, false); + /* The invoked method throws an exception and therefore never returns. */ + b.append(new DeadEndNode()); + return; + + } else { + throw new UnsupportedFeatureException(message + System.lineSeparator() + "To diagnose the issue, you can add the option " + + SubstrateOptionsParser.commandArgument(NativeImageOptions.ReportUnsupportedElementsAtRuntime, "+") + + ". The error is then reported at run time when the invoke is executed."); + } + } + @SuppressWarnings("try") private static ValueNode maybeEmitClassCast(GraphBuilderContext b, ObjectStamp classCastStamp, ValueNode object) { if (classCastStamp == null) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/IntrinsificationPluginRegistry.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/IntrinsificationPluginRegistry.java new file mode 100644 index 000000000000..db623dd4efaf --- /dev/null +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/IntrinsificationPluginRegistry.java @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2019, 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. 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.hosted.snippets; + +import java.util.concurrent.ConcurrentHashMap; + +import com.oracle.graal.pointsto.meta.AnalysisMethod; +import com.oracle.svm.core.util.VMError; +import com.oracle.svm.hosted.meta.HostedMethod; + +import jdk.vm.ci.meta.ResolvedJavaMethod; + +public class IntrinsificationPluginRegistry { + + static final class CallSiteDescriptor { + private final AnalysisMethod method; + private final int bci; + + private CallSiteDescriptor(ResolvedJavaMethod method, int bci) { + this.method = toAnalysisMethod(method); + this.bci = bci; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof CallSiteDescriptor) { + CallSiteDescriptor other = (CallSiteDescriptor) obj; + return other.bci == this.bci && other.method.equals(this.method); + } + return false; + } + + @Override + public int hashCode() { + return method.hashCode() ^ bci; + } + + private static AnalysisMethod toAnalysisMethod(ResolvedJavaMethod method) { + if (method instanceof HostedMethod) { + return ((HostedMethod) method).wrapped; + } else { + VMError.guarantee(method instanceof AnalysisMethod); + return (AnalysisMethod) method; + } + } + } + + private static final Object NULL_MARKER = new Object(); + + /** + * Contains all the elements intrinsified during analysis. Only these elements will be + * intrinsified during compilation. We cannot intrinsify an element during compilation if it was + * not intrinsified during analysis since it can lead to compiling code that was not seen during + * analysis. + */ + private final ConcurrentHashMap analysisElements = new ConcurrentHashMap<>(); + + public void add(ResolvedJavaMethod method, int bci, Object element) { + Object nonNullElement = element != null ? element : NULL_MARKER; + Object previous = analysisElements.put(new CallSiteDescriptor(method, bci), nonNullElement); + + /* + * New elements can only be added when the intrinsification is executed during the analysis. + * If an intrinsified element was already registered that's an error. + */ + VMError.guarantee(previous == null, "Detected previously intrinsified element"); + } + + @SuppressWarnings("unchecked") + public T get(ResolvedJavaMethod method, int bci) { + Object nonNullElement = analysisElements.get(new CallSiteDescriptor(method, bci)); + return nonNullElement != NULL_MARKER ? (T) nonNullElement : null; + } +} diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java index 3b82437a72f0..b64aa4092ffe 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java @@ -29,8 +29,6 @@ import java.lang.reflect.Executable; import java.lang.reflect.Field; import java.lang.reflect.Method; -import java.util.Objects; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -46,16 +44,13 @@ import org.graalvm.nativeimage.ImageSingletons; import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider; -import com.oracle.graal.pointsto.meta.AnalysisMethod; import com.oracle.svm.core.annotate.Delete; import com.oracle.svm.core.option.HostedOptionKey; -import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.ExceptionSynthesizer; import com.oracle.svm.hosted.ImageClassLoader; import com.oracle.svm.hosted.NativeImageOptions; import com.oracle.svm.hosted.SVMHost; import com.oracle.svm.hosted.c.GraalAccess; -import com.oracle.svm.hosted.meta.HostedMethod; import com.oracle.svm.hosted.phases.SubstrateClassInitializationPlugin; import com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor; @@ -66,62 +61,7 @@ public class ReflectionPlugins { - static final class CallSiteDescriptor { - ResolvedJavaMethod method; - int bci; - - private CallSiteDescriptor(ResolvedJavaMethod method, int bci) { - Objects.requireNonNull(method); - this.method = method; - this.bci = bci; - } - - @Override - public boolean equals(Object obj) { - if (obj instanceof CallSiteDescriptor) { - CallSiteDescriptor other = (CallSiteDescriptor) obj; - return other.bci == this.bci && other.method.equals(this.method); - } - return false; - } - - @Override - public int hashCode() { - return method.hashCode() ^ bci; - } - } - - static class ReflectionPluginRegistry { - - /** - * Contains all the classes, methods, fields intrinsified by this plugin during analysis. - * Only these elements will be intrinsified during compilation. We cannot intrinsify an - * element during compilation if it was not intrinsified during analysis since it can lead - * to compiling code that was not seen during analysis. - */ - ConcurrentHashMap analysisElements = new ConcurrentHashMap<>(); - - public void add(ResolvedJavaMethod method, int bci, Object element) { - add(new CallSiteDescriptor(method, bci), element); - } - - public void add(CallSiteDescriptor location, Object element) { - Object previous = analysisElements.put(location, element); - /* - * New elements can only be added when the reflection plugins are executed during the - * analysis. If an intrinsified element was already registered that's an error. - */ - VMError.guarantee(previous == null, "Detected previously intrinsified reflectively accessed element. "); - } - - public T get(ResolvedJavaMethod method, int bci) { - return get(new CallSiteDescriptor(method, bci)); - } - - @SuppressWarnings("unchecked") - public T get(CallSiteDescriptor location) { - return (T) analysisElements.get(location); - } + static class ReflectionPluginRegistry extends IntrinsificationPluginRegistry { } static class Options { @@ -394,20 +334,11 @@ private static T getIntrinsic(boolean analysis, boolean hosted, GraphBuilder } } /* We are during analysis, we should intrinsify and cache the intrinsified object. */ - ImageSingletons.lookup(ReflectionPluginRegistry.class).add(toAnalysisMethod(context.getMethod()), context.bci(), element); + ImageSingletons.lookup(ReflectionPluginRegistry.class).add(context.getMethod(), context.bci(), element); return element; } /* We are during compilation, we only intrinsify if intrinsified during analysis. */ - return ImageSingletons.lookup(ReflectionPluginRegistry.class).get(toAnalysisMethod(context.getMethod()), context.bci()); - } - - private static ResolvedJavaMethod toAnalysisMethod(ResolvedJavaMethod method) { - if (method instanceof HostedMethod) { - return ((HostedMethod) method).wrapped; - } else { - VMError.guarantee(method instanceof AnalysisMethod); - return method; - } + return ImageSingletons.lookup(ReflectionPluginRegistry.class).get(context.getMethod(), context.bci()); } private static void pushConstant(GraphBuilderContext b, ResolvedJavaMethod reflectionMethod, JavaConstant constant, String targetElement) {