changeset 9986:4f542ceb5fed

added VerifyHotSpotOptionsPhase to ensure that global state is not initialized from options prior to command line parsing
author Doug Simon <doug.simon@oracle.com>
date Tue, 11 Jun 2013 01:31:47 +0200
parents e6bd1004082a
children b270f0856a39
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotOptions.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/VerifyHotSpotOptionsPhase.java graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionDescriptor.java graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionProcessor.java
diffstat 5 files changed, 173 insertions(+), 13 deletions(-) [+]
line wrap: on
line diff
--- 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<Options> 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
--- 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);
--- /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<Options> sl = ServiceLoader.loadInstalled(Options.class);
+        Set<HotSpotResolvedObjectType> 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<ResolvedJavaType> 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));
+    }
+}
--- 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();
     }
 }
--- 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("        );");