# HG changeset patch # User Doug Simon # Date 1401959146 -7200 # Node ID bddb3eb57e90c6eceabfed37e6a6bd6cf7643aac # Parent 8bbfddf8483f445c15915f3b7ac7e10782df56dc moved verification of OptionValue declaring classes from run time to build time diff -r 8bbfddf8483f -r bddb3eb57e90 graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java --- a/graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java Thu Jun 05 11:04:36 2014 +0200 +++ b/graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java Thu Jun 05 11:05:46 2014 +0200 @@ -37,6 +37,26 @@ */ public class GenGraalRuntimeInlineHpp { + private static final ZipFile graalJar; + + static { + String path = null; + String classPath = System.getProperty("java.class.path"); + for (String e : classPath.split(File.pathSeparator)) { + if (e.endsWith("graal.jar")) { + path = e; + break; + } + } + ZipFile zipFile = null; + try { + zipFile = new ZipFile(Objects.requireNonNull(path, "Could not find graal.jar on class path: " + classPath)); + } catch (IOException e) { + throw new InternalError(e); + } + graalJar = zipFile; + } + public static void main(String[] args) { PrintStream out = System.out; try { @@ -52,17 +72,8 @@ * Generates code for {@code GraalRuntime::get_service_impls()}. */ private static void genGetServiceImpls(PrintStream out) throws Exception { - String graalJar = null; - String classPath = System.getProperty("java.class.path"); - for (String e : classPath.split(File.pathSeparator)) { - if (e.endsWith("graal.jar")) { - graalJar = e; - break; - } - } final List> services = new ArrayList<>(); - final ZipFile zipFile = new ZipFile(new File(Objects.requireNonNull(graalJar, "Could not find graal.jar on class path: " + classPath))); - for (final Enumeration e = zipFile.entries(); e.hasMoreElements();) { + for (final Enumeration e = graalJar.entries(); e.hasMoreElements();) { final ZipEntry zipEntry = e.nextElement(); String name = zipEntry.getName(); if (name.startsWith("META-INF/services/")) { @@ -191,7 +202,14 @@ static SortedMap getOptions() throws Exception { Field field = Class.forName("com.oracle.graal.hotspot.HotSpotOptionsLoader").getDeclaredField("options"); field.setAccessible(true); - return (SortedMap) field.get(null); + SortedMap options = (SortedMap) field.get(null); + + Set> checked = new HashSet<>(); + for (final OptionDescriptor option : options.values()) { + Class cls = option.getDeclaringClass(); + OptionsVerifier.checkClass(cls, option, checked, graalJar); + } + return options; } private static Class getFieldType(OptionDescriptor desc) throws Exception { diff -r 8bbfddf8483f -r bddb3eb57e90 graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/OptionsVerifier.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/OptionsVerifier.java Thu Jun 05 11:05:46 2014 +0200 @@ -0,0 +1,186 @@ +/* + * Copyright (c) 2014, 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 com.oracle.graal.hotspot.sourcegen; + +import static java.lang.String.*; + +import java.io.*; +import java.lang.reflect.*; +import java.util.*; +import java.util.zip.*; + +import jdk.internal.org.objectweb.asm.*; +import jdk.internal.org.objectweb.asm.Type; + +import com.oracle.graal.options.*; + +/** + * A {@link ClassVisitor} that verifies a class declaring one or more {@linkplain OptionValue + * options} has a class initializer that only initializes the option(s). This sanity check mitigates + * the possibility of an option value being used before the code that sets the value (e.g., from the + * command line) has been executed. + */ +final class OptionsVerifier extends ClassVisitor { + + public static void checkClass(Class cls, OptionDescriptor option, Set> checked, ZipFile graalJar) throws IOException { + if (!checked.contains(cls)) { + checked.add(cls); + Class superclass = cls.getSuperclass(); + if (superclass != null && !superclass.equals(Object.class)) { + checkClass(superclass, option, checked, graalJar); + } + + String classFilePath = cls.getName().replace('.', '/') + ".class"; + ZipEntry entry = Objects.requireNonNull(graalJar.getEntry(classFilePath), "Could not find class file for " + cls.getName()); + ClassReader cr = new ClassReader(graalJar.getInputStream(entry)); + + ClassVisitor cv = new OptionsVerifier(cls, option); + cr.accept(cv, 0); + } + } + + /** + * The option field context of the verification. + */ + private final OptionDescriptor option; + + /** + * The class in which {@link #option} is declared or a super-class of that class. This is the + * class whose {@code } method is being verified. + */ + private final Class cls; + + /** + * Source file context for error reporting. + */ + String sourceFile = null; + + /** + * Line number for error reporting. + */ + int lineNo = -1; + + final Class[] boxingTypes = {Boolean.class, Byte.class, Short.class, Character.class, Integer.class, Float.class, Long.class, Double.class}; + + private static Class resolve(String name) { + try { + return Class.forName(name.replace('/', '.')); + } catch (ClassNotFoundException e) { + throw new InternalError(e); + } + } + + OptionsVerifier(Class cls, OptionDescriptor desc) { + super(Opcodes.ASM5); + this.cls = cls; + this.option = desc; + } + + @Override + public void visitSource(String source, String debug) { + this.sourceFile = source; + } + + void verify(boolean condition, String message) { + if (!condition) { + error(message); + } + } + + void error(String message) { + String errorMessage = format("%s:%d: Illegal code in %s. which may be executed when %s.%s is initialized:%n%n %s%n%n" + "The recommended solution is to move " + option.getName() + + " into a separate class (e.g., %s.Options).%n", sourceFile, lineNo, cls.getSimpleName(), option.getDeclaringClass().getSimpleName(), option.getName(), message, + option.getDeclaringClass().getSimpleName()); + throw new InternalError(errorMessage); + + } + + @Override + public MethodVisitor visitMethod(int access, String name, String d, String signature, String[] exceptions) { + if (name.equals("")) { + return new MethodVisitor(Opcodes.ASM5) { + + @Override + public void visitLineNumber(int line, Label start) { + lineNo = line; + } + + @Override + public void visitFieldInsn(int opcode, String owner, String fieldName, String fieldDesc) { + if (opcode == Opcodes.PUTFIELD || opcode == Opcodes.PUTSTATIC) { + verify(resolve(owner).equals(option.getDeclaringClass()), format("store to field %s.%s", resolve(owner).getSimpleName(), fieldName)); + verify(opcode != Opcodes.PUTFIELD, format("store to non-static field %s.%s", resolve(owner).getSimpleName(), fieldName)); + } + } + + private Executable resolveMethod(String owner, String methodName, String methodDesc) { + Class declaringClass = resolve(owner); + if (methodName.equals("")) { + for (Constructor c : declaringClass.getDeclaredConstructors()) { + if (methodDesc.equals(Type.getConstructorDescriptor(c))) { + return c; + } + } + } else { + Type[] argumentTypes = Type.getArgumentTypes(methodDesc); + for (Method m : declaringClass.getDeclaredMethods()) { + if (m.getName().equals(methodName)) { + if (Arrays.equals(argumentTypes, Type.getArgumentTypes(m))) { + if (Type.getReturnType(methodDesc).equals(Type.getReturnType(m))) { + return m; + } + } + } + } + } + throw new NoSuchMethodError(declaringClass + "." + methodName + methodDesc); + } + + /** + * Checks whether a given method is allowed to be called. + */ + private boolean checkInvokeTarget(Executable method) { + Class holder = method.getDeclaringClass(); + if (method instanceof Constructor) { + if (OptionValue.class.isAssignableFrom(holder)) { + return true; + } + } else if (Arrays.asList(boxingTypes).contains(holder)) { + return method.getName().equals("valueOf"); + } else if (method.getDeclaringClass().equals(Class.class)) { + return method.getName().equals("desiredAssertionStatus"); + } + return false; + } + + @Override + public void visitMethodInsn(int opcode, String owner, String methodName, String methodDesc, boolean itf) { + Executable callee = resolveMethod(owner, methodName, methodDesc); + verify(checkInvokeTarget(callee), "invocation of " + callee); + } + }; + } else { + return null; + } + } +} \ No newline at end of file diff -r 8bbfddf8483f -r bddb3eb57e90 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotGraalRuntime.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotGraalRuntime.java Thu Jun 05 11:04:36 2014 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotGraalRuntime.java Thu Jun 05 11:05:46 2014 +0200 @@ -52,7 +52,6 @@ import com.oracle.graal.hotspot.events.*; import com.oracle.graal.hotspot.logging.*; import com.oracle.graal.hotspot.meta.*; -import com.oracle.graal.java.*; import com.oracle.graal.options.*; import com.oracle.graal.printer.*; import com.oracle.graal.replacements.*; @@ -147,9 +146,6 @@ } } - final HotSpotProviders hostProviders = hostBackend.getProviders(); - assert VerifyOptionsPhase.checkOptions(hostProviders.getMetaAccess()); - // Complete initialization of backends try (InitTimer st = timer(hostBackend.getTarget().arch.getName(), ".completeInitialization")) { hostBackend.completeInitialization(); diff -r 8bbfddf8483f -r bddb3eb57e90 graal/com.oracle.graal.java/src/com/oracle/graal/java/VerifyOptionsPhase.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/VerifyOptionsPhase.java Thu Jun 05 11:04:36 2014 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,144 +0,0 @@ -/* - * Copyright (c) 2011, 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 com.oracle.graal.java; - -import static com.oracle.graal.api.meta.MetaUtil.*; -import java.util.*; - -import com.oracle.graal.api.meta.*; -import com.oracle.graal.compiler.common.*; -import com.oracle.graal.graph.*; -import com.oracle.graal.nodes.*; -import com.oracle.graal.nodes.java.*; -import com.oracle.graal.nodes.util.*; -import com.oracle.graal.options.*; -import com.oracle.graal.phases.*; - -/** - * Verifies that a class declaring one or more {@linkplain OptionValue options} has a class - * initializer that only initializes the option(s). This sanity check mitigates the possibility of - * an option value being used before the code that sets the value (e.g., from the command line) has - * been executed. - */ -public class VerifyOptionsPhase extends Phase { - - public static boolean checkOptions(MetaAccessProvider metaAccess) { - ServiceLoader sl = ServiceLoader.loadInstalled(Options.class); - Set checked = new HashSet<>(); - for (Options opts : sl) { - for (OptionDescriptor desc : opts) { - ResolvedJavaType holder = metaAccess.lookupJavaType(desc.getDeclaringClass()); - checkType(holder, desc, metaAccess, checked); - } - } - return true; - } - - private static void checkType(ResolvedJavaType type, OptionDescriptor option, MetaAccessProvider metaAccess, Set checked) { - if (!checked.contains(type)) { - checked.add(type); - ResolvedJavaType superType = type.getSuperclass(); - if (superType != null && !MetaUtil.isJavaLangObject(superType)) { - checkType(superType, option, metaAccess, checked); - } - ResolvedJavaMethod clinit = type.getClassInitializer(); - if (clinit != null) { - StructuredGraph graph = new StructuredGraph(clinit); - new GraphBuilderPhase.Instance(metaAccess, GraphBuilderConfiguration.getEagerDefault(), OptimisticOptimizations.ALL).apply(graph); - new VerifyOptionsPhase(type, metaAccess, option).apply(graph); - } - } - } - - private final MetaAccessProvider metaAccess; - private final ResolvedJavaType declaringClass; - private final ResolvedJavaType optionValueType; - private final Set boxingTypes; - private final OptionDescriptor option; - - public VerifyOptionsPhase(ResolvedJavaType declaringClass, MetaAccessProvider metaAccess, OptionDescriptor option) { - this.metaAccess = metaAccess; - this.declaringClass = declaringClass; - this.optionValueType = metaAccess.lookupJavaType(OptionValue.class); - this.option = option; - this.boxingTypes = new HashSet<>(); - for (Class c : new Class[]{Boolean.class, Byte.class, Short.class, Character.class, Integer.class, Float.class, Long.class, Double.class}) { - this.boxingTypes.add(metaAccess.lookupJavaType(c)); - } - } - - /** - * Checks whether a given method is allowed to be called. - */ - private boolean checkInvokeTarget(ResolvedJavaMethod method) { - ResolvedJavaType holder = method.getDeclaringClass(); - if (method.isConstructor()) { - if (optionValueType.isAssignableFrom(holder)) { - return true; - } - } else if (boxingTypes.contains(holder)) { - return method.getName().equals("valueOf"); - } else if (method.getDeclaringClass().equals(metaAccess.lookupJavaType(Class.class))) { - return method.getName().equals("desiredAssertionStatus"); - } else if (method.getDeclaringClass().equals(declaringClass)) { - return (method.getName().equals("$jacocoInit")); - } - return false; - } - - @Override - protected void run(StructuredGraph graph) { - for (ValueNode node : graph.getNodes().filter(ValueNode.class)) { - if (node instanceof StoreFieldNode) { - ResolvedJavaField field = ((StoreFieldNode) node).field(); - verify(field.getDeclaringClass().equals(declaringClass), node, "store to field " + format("%H.%n", field)); - verify(field.isStatic(), node, "store to field " + format("%H.%n", field)); - if (optionValueType.isAssignableFrom((ResolvedJavaType) field.getType())) { - verify(field.isFinal(), node, "option field " + format("%H.%n", field) + " not final"); - } else { - verify((field.isSynthetic()), node, "store to non-synthetic field " + format("%H.%n", field)); - } - } else if (node instanceof Invoke) { - Invoke invoke = (Invoke) node; - MethodCallTargetNode callTarget = (MethodCallTargetNode) invoke.callTarget(); - ResolvedJavaMethod targetMethod = callTarget.targetMethod(); - verify(checkInvokeTarget(targetMethod), node, "invocation of " + format("%H.%n(%p)", targetMethod)); - } - } - } - - private void verify(boolean condition, Node node, String message) { - if (!condition) { - error(node, message); - } - } - - private void error(Node node, String message) { - String loc = GraphUtil.approxSourceLocation(node); - throw new GraalInternalError(String.format("The " + option.getName() + " option is declared in " + option.getDeclaringClass() + - " whose class hierarchy contains a class initializer (in %s) with a code pattern at or near %s implying an action other than initialization of an option:%n%n %s%n%n" + - "The recommended solution is to move " + option.getName() + " into a separate class (e.g., " + option.getDeclaringClass().getName() + ".Options).%n", - toJavaName(declaringClass), loc, message)); - } -}