# HG changeset patch # User Doug Simon # Date 1370907107 -7200 # Node ID 4f542ceb5fedea9ebbd4a40bdd2e9bea2a5f7ad8 # Parent e6bd1004082a51eddcc026fb147c0c1e6bd66baa added VerifyHotSpotOptionsPhase to ensure that global state is not initialized from options prior to command line parsing diff -r e6bd1004082a -r 4f542ceb5fed graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotOptions.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotOptions.java Tue Jun 11 01:18:57 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotOptions.java Tue Jun 11 01:31:47 2013 +0200 @@ -48,7 +48,7 @@ ServiceLoader sl = ServiceLoader.loadInstalled(Options.class); for (Options opts : sl) { for (OptionDescriptor desc : opts) { - if (desc.getClass().getName().startsWith("com.oracle.graal")) { + if (isHotSpotOption(desc)) { String name = desc.getName(); OptionDescriptor existing = options.put(name, desc); assert existing == null : "Option named \"" + name + "\" has multiple definitions: " + existing.getLocation() + " and " + desc.getLocation(); @@ -58,6 +58,13 @@ } /** + * Determines if a given option is a HotSpot command line option. + */ + public static boolean isHotSpotOption(OptionDescriptor desc) { + return desc.getClass().getName().startsWith("com.oracle.graal"); + } + + /** * Loads default option value overrides from a {@code graal.options} file if it exists. Each * line in this file starts with {@code "#"} and is ignored or must have the format of a Graal * command line option without the leading {@code "-G:"} prefix. These option value are set diff -r e6bd1004082a -r 4f542ceb5fed graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java Tue Jun 11 01:18:57 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java Tue Jun 11 01:31:47 2013 +0200 @@ -183,6 +183,8 @@ DebugEnvironment.initialize(log); } + assert VerifyHotSpotOptionsPhase.checkOptions(); + // Install intrinsics. final HotSpotRuntime runtime = graalRuntime.getCapability(HotSpotRuntime.class); final Replacements replacements = graalRuntime.getCapability(Replacements.class); diff -r e6bd1004082a -r 4f542ceb5fed graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/VerifyHotSpotOptionsPhase.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/VerifyHotSpotOptionsPhase.java Tue Jun 11 01:31:47 2013 +0200 @@ -0,0 +1,140 @@ +/* + * 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.hotspot.phases; + +import static com.oracle.graal.api.meta.MetaUtil.*; +import static com.oracle.graal.hotspot.HotSpotGraalRuntime.*; +import static java.lang.reflect.Modifier.*; + +import java.util.*; + +import com.oracle.graal.api.meta.*; +import com.oracle.graal.graph.*; +import com.oracle.graal.hotspot.*; +import com.oracle.graal.hotspot.meta.*; +import com.oracle.graal.java.*; +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 that declares one or more HotSpot {@linkplain OptionValue options} has a + * class initializer that only initializes the option(s). This sanity check prevents an option being + * read to initialize some global state before it is parsed on the command line. The latter occurs + * if an option declaring class has a class initializer that reads options or triggers other class + * initializers that read options. + */ +public class VerifyHotSpotOptionsPhase extends Phase { + + public static boolean checkOptions() { + HotSpotRuntime runtime = graalRuntime().getRuntime(); + ServiceLoader sl = ServiceLoader.loadInstalled(Options.class); + Set checked = new HashSet<>(); + for (Options opts : sl) { + for (OptionDescriptor desc : opts) { + if (HotSpotOptions.isHotSpotOption(desc)) { + HotSpotResolvedObjectType holder = (HotSpotResolvedObjectType) runtime.lookupJavaType(desc.getDeclaringClass()); + if (!checked.contains(holder)) { + checked.add(holder); + for (ResolvedJavaMethod method : holder.getMethods()) { + if (method.isClassInitializer()) { + StructuredGraph graph = new StructuredGraph(method); + new GraphBuilderPhase(runtime, GraphBuilderConfiguration.getEagerDefault(), OptimisticOptimizations.ALL).apply(graph); + new VerifyHotSpotOptionsPhase(holder, runtime).apply(graph); + } + } + } + } + } + } + return true; + } + + private final HotSpotRuntime runtime; + private final ResolvedJavaType declaringClass; + private final ResolvedJavaType optionValueType; + private final Set boxingTypes; + + public VerifyHotSpotOptionsPhase(ResolvedJavaType declaringClass, HotSpotRuntime runtime) { + this.runtime = runtime; + this.declaringClass = declaringClass; + this.optionValueType = runtime.lookupJavaType(OptionValue.class); + 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(runtime.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() == runtime.lookupJavaType(Class.class)) { + return method.getName().equals("desiredAssertionStatus"); + } + return false; + } + + @Override + protected void run(StructuredGraph graph) { + for (ValueNode node : graph.getNodes().filter(ValueNode.class)) { + if (node instanceof StoreFieldNode) { + HotSpotResolvedJavaField field = (HotSpotResolvedJavaField) ((StoreFieldNode) node).field(); + verify(field.getDeclaringClass() == declaringClass, node, "store to field " + format("%H.%n", field)); + verify(isStatic(field.getModifiers()), node, "store to field " + format("%H.%n", field)); + if (optionValueType.isAssignableFrom((ResolvedJavaType) field.getType())) { + verify(isFinal(field.getModifiers()), 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("HotSpot option declarer %s contains code pattern implying action other than initialization of an option:%n %s%n %s", + toJavaName(declaringClass), loc, message)); + } +} diff -r e6bd1004082a -r 4f542ceb5fed graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionDescriptor.java --- a/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionDescriptor.java Tue Jun 11 01:18:57 2013 +0200 +++ b/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionDescriptor.java Tue Jun 11 01:31:47 2013 +0200 @@ -32,14 +32,16 @@ protected final Class type; protected final String help; protected final OptionValue option; - protected final String location; + protected final Class declaringClass; + protected final String fieldName; - public OptionDescriptor(String name, Class type, String help, String location, OptionValue option) { + public OptionDescriptor(String name, Class type, String help, Class declaringClass, String fieldName, OptionValue option) { this.name = name; this.type = type; this.help = help; this.option = option; - this.location = location; + this.declaringClass = declaringClass; + this.fieldName = fieldName; } /** @@ -71,11 +73,18 @@ return option; } + public Class getDeclaringClass() { + return declaringClass; + } + + public String getFieldName() { + return fieldName; + } + /** * Gets a description of the location where this option is stored. */ public String getLocation() { - return location; - + return getDeclaringClass().getName() + "." + getFieldName(); } } diff -r e6bd1004082a -r 4f542ceb5fed graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionProcessor.java --- a/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionProcessor.java Tue Jun 11 01:18:57 2013 +0200 +++ b/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionProcessor.java Tue Jun 11 01:31:47 2013 +0200 @@ -123,9 +123,9 @@ private void createFiles(OptionsInfo info) { String pkg = ((PackageElement) info.topDeclaringType.getEnclosingElement()).getQualifiedName().toString(); - Name declaringClass = info.topDeclaringType.getSimpleName(); + Name topDeclaringClass = info.topDeclaringType.getSimpleName(); - String optionsClassName = declaringClass + "_" + Options.class.getSimpleName(); + String optionsClassName = topDeclaringClass + "_" + Options.class.getSimpleName(); Element[] originatingElements = info.originatingElements.toArray(new Element[info.originatingElements.size()]); Filer filer = processingEnv.getFiler(); @@ -133,7 +133,7 @@ out.println("// CheckStyle: stop header check"); out.println("// GENERATED CONTENT - DO NOT EDIT"); - out.println("// Source: " + declaringClass + ".java"); + out.println("// Source: " + topDeclaringClass + ".java"); out.println("package " + pkg + ";"); out.println(""); out.println("import java.util.*;"); @@ -141,8 +141,9 @@ out.println(""); out.println("public class " + optionsClassName + " implements " + Options.class.getSimpleName() + " {"); out.println(" @Override"); - out.println(" public Iterator<" + OptionDescriptor.class.getSimpleName() + "> iterator() {"); - out.println(" List<" + OptionDescriptor.class.getSimpleName() + "> options = Arrays.asList("); + String desc = OptionDescriptor.class.getSimpleName(); + out.println(" public Iterator<" + desc + "> iterator() {"); + out.println(" List<" + desc + "> options = Arrays.asList("); boolean needPrivateFieldAccessor = false; int i = 0; @@ -157,9 +158,10 @@ String name = option.name; String type = option.type; String help = option.help; - String location = pkg + "." + option.declaringClass + "." + option.field.getSimpleName(); + String declaringClass = option.declaringClass; + Name fieldName = option.field.getSimpleName(); String comma = i == info.options.size() - 1 ? "" : ","; - out.printf(" new %s(\"%s\", %s.class, \"%s\", \"%s\", %s)%s\n", OptionDescriptor.class.getSimpleName(), name, type, help, location, optionValue, comma); + out.printf(" new %s(\"%s\", %s.class, \"%s\", %s.class, \"%s\", %s)%s\n", desc, name, type, help, declaringClass, fieldName, optionValue, comma); i++; } out.println(" );");