changeset 16870:11b22ccafccd

Correctly parse string option values that start with + or -.
author Roland Schatz <roland.schatz@oracle.com>
date Wed, 20 Aug 2014 15:35:27 +0200
parents 6754b5b64978
children e728b9d4905c
files graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java src/share/vm/graal/graalRuntime.cpp src/share/vm/graal/graalRuntime.hpp
diffstat 3 files changed, 56 insertions(+), 41 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java	Wed Aug 20 15:17:17 2014 +0200
+++ b/graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java	Wed Aug 20 15:35:27 2014 +0200
@@ -146,7 +146,8 @@
     }
 
     /**
-     * Generates code for {@code GraalRuntime::set_option()}.
+     * Generates code for {@code GraalRuntime::set_option()} and
+     * {@code GraalRuntime::set_option_bool()}.
      */
     private static void genSetOption(PrintStream out) throws Exception {
         SortedMap<String, OptionDescriptor> options = getOptions();
@@ -157,21 +158,20 @@
         }
         lengths.add("PrintFlags".length());
 
+        out.println("bool GraalRuntime::set_option_bool(KlassHandle hotSpotOptionsClass, char* name, int name_len, char value, TRAPS) {");
+        out.println("  bool check_only = hotSpotOptionsClass.is_null();");
+        genMatchers(out, lengths, options, true);
+        out.println("  return false;");
+        out.println("}");
         out.println("bool GraalRuntime::set_option(KlassHandle hotSpotOptionsClass, char* name, int name_len, const char* value, TRAPS) {");
         out.println("  bool check_only = hotSpotOptionsClass.is_null();");
-        out.println("  if (value != NULL && (value[0] == '+' || value[0] == '-')) {");
-        out.println("    // boolean options");
-        genMatchers(out, lengths, options, true);
-        out.println("  } else {");
-        out.println("    // non-boolean options");
         genMatchers(out, lengths, options, false);
-        out.println("  }");
         out.println("  return false;");
         out.println("}");
     }
 
     protected static void genMatchers(PrintStream out, Set<Integer> lengths, SortedMap<String, OptionDescriptor> options, boolean isBoolean) throws Exception {
-        out.println("    switch (name_len) {");
+        out.println("  switch (name_len) {");
         for (int len : lengths) {
             boolean printedCase = false;
 
@@ -179,56 +179,56 @@
             // null terminated for <name>=<value> style options.
             if (len == "PrintFlags".length() && isBoolean) {
                 printedCase = true;
-                out.println("    case " + len + ":");
-                out.printf("      if (strncmp(name, \"PrintFlags\", %d) == 0) {%n", len);
-                out.println("        if (value[0] == '+') {");
-                out.println("          if (check_only) {");
-                out.println("            TempNewSymbol name = SymbolTable::new_symbol(\"Lcom/oracle/graal/hotspot/HotSpotOptions;\", CHECK_(true));");
-                out.println("            hotSpotOptionsClass = SystemDictionary::resolve_or_fail(name, true, CHECK_(true));");
-                out.println("          }");
-                out.println("          set_option_helper(hotSpotOptionsClass, name, name_len, Handle(), '?', Handle(), 0L);");
+                out.println("  case " + len + ":");
+                out.printf("    if (strncmp(name, \"PrintFlags\", %d) == 0) {%n", len);
+                out.println("      if (value == '+') {");
+                out.println("        if (check_only) {");
+                out.println("          TempNewSymbol name = SymbolTable::new_symbol(\"Lcom/oracle/graal/hotspot/HotSpotOptions;\", CHECK_(true));");
+                out.println("          hotSpotOptionsClass = SystemDictionary::resolve_or_fail(name, true, CHECK_(true));");
                 out.println("        }");
-                out.println("        return true;");
+                out.println("        set_option_helper(hotSpotOptionsClass, name, name_len, Handle(), '?', Handle(), 0L);");
                 out.println("      }");
+                out.println("      return true;");
+                out.println("    }");
             }
             for (Map.Entry<String, OptionDescriptor> e : options.entrySet()) {
                 OptionDescriptor desc = e.getValue();
                 if (e.getKey().length() == len && ((desc.getType() == Boolean.class) == isBoolean)) {
                     if (!printedCase) {
                         printedCase = true;
-                        out.println("    case " + len + ":");
+                        out.println("  case " + len + ":");
                     }
-                    out.printf("      if (strncmp(name, \"%s\", %d) == 0) {%n", e.getKey(), len);
+                    out.printf("    if (strncmp(name, \"%s\", %d) == 0) {%n", e.getKey(), len);
                     Class<?> declaringClass = desc.getDeclaringClass();
                     if (isBoolean) {
-                        out.printf("        Handle option = get_OptionValue(\"L%s;\", \"%s\", \"L%s;\", CHECK_(true));%n", toInternalName(declaringClass), desc.getFieldName(),
+                        out.printf("      Handle option = get_OptionValue(\"L%s;\", \"%s\", \"L%s;\", CHECK_(true));%n", toInternalName(declaringClass), desc.getFieldName(),
+                                        toInternalName(getFieldType(desc)));
+                        out.println("      if (!check_only) {");
+                        out.println("        set_option_helper(hotSpotOptionsClass, name, name_len, option, value, Handle(), 0L);");
+                        out.println("      }");
+                    } else if (desc.getType() == String.class) {
+                        out.println("      check_required_value(name, name_len, value, CHECK_(true));");
+                        out.printf("      Handle option = get_OptionValue(\"L%s;\", \"%s\", \"L%s;\", CHECK_(true));%n", toInternalName(declaringClass), desc.getFieldName(),
                                         toInternalName(getFieldType(desc)));
-                        out.println("        if (!check_only) {");
-                        out.println("          set_option_helper(hotSpotOptionsClass, name, name_len, option, value[0], Handle(), 0L);");
-                        out.println("        }");
-                    } else if (desc.getType() == String.class) {
-                        out.println("        check_required_value(name, name_len, value, CHECK_(true));");
+                        out.println("      if (!check_only) {");
+                        out.println("        Handle stringValue = java_lang_String::create_from_str(value, CHECK_(true));");
+                        out.println("        set_option_helper(hotSpotOptionsClass, name, name_len, option, 's', stringValue, 0L);");
+                        out.println("      }");
+                    } else {
+                        char spec = getPrimitiveSpecChar(desc);
+                        out.println("      jlong primitiveValue = parse_primitive_option_value('" + spec + "', name, name_len, value, CHECK_(true));");
+                        out.println("      if (!check_only) {");
                         out.printf("        Handle option = get_OptionValue(\"L%s;\", \"%s\", \"L%s;\", CHECK_(true));%n", toInternalName(declaringClass), desc.getFieldName(),
                                         toInternalName(getFieldType(desc)));
-                        out.println("        if (!check_only) {");
-                        out.println("          Handle stringValue = java_lang_String::create_from_str(value, CHECK_(true));");
-                        out.println("          set_option_helper(hotSpotOptionsClass, name, name_len, option, 's', stringValue, 0L);");
-                        out.println("        }");
-                    } else {
-                        char spec = getPrimitiveSpecChar(desc);
-                        out.println("        jlong primitiveValue = parse_primitive_option_value('" + spec + "', name, name_len, value, CHECK_(true));");
-                        out.println("        if (!check_only) {");
-                        out.printf("          Handle option = get_OptionValue(\"L%s;\", \"%s\", \"L%s;\", CHECK_(true));%n", toInternalName(declaringClass), desc.getFieldName(),
-                                        toInternalName(getFieldType(desc)));
-                        out.println("          set_option_helper(hotSpotOptionsClass, name, name_len, option, '" + spec + "', Handle(), primitiveValue);");
-                        out.println("        }");
+                        out.println("        set_option_helper(hotSpotOptionsClass, name, name_len, option, '" + spec + "', Handle(), primitiveValue);");
+                        out.println("      }");
                     }
-                    out.println("        return true;");
-                    out.println("      }");
+                    out.println("      return true;");
+                    out.println("    }");
                 }
             }
         }
-        out.println("    }");
+        out.println("  }");
     }
 
     @SuppressWarnings("unchecked")
--- a/src/share/vm/graal/graalRuntime.cpp	Wed Aug 20 15:17:17 2014 +0200
+++ b/src/share/vm/graal/graalRuntime.cpp	Wed Aug 20 15:35:27 2014 +0200
@@ -790,7 +790,7 @@
   if (first == '+' || first == '-') {
     name = arg + 1;
     name_len = strlen(name);
-    recognized = set_option(hotSpotOptionsClass, name, (int)name_len, arg, CHECK);
+    recognized = set_option_bool(hotSpotOptionsClass, name, (int)name_len, first, CHECK);
   } else {
     char* sep = strchr(arg, '=');
     name = arg;
--- a/src/share/vm/graal/graalRuntime.hpp	Wed Aug 20 15:17:17 2014 +0200
+++ b/src/share/vm/graal/graalRuntime.hpp	Wed Aug 20 15:35:27 2014 +0200
@@ -70,6 +70,21 @@
   static void parse_argument(KlassHandle hotSpotOptionsClass, char* arg, TRAPS);
 
   /**
+   * Searches for a Boolean Graal option denoted by a given name and sets it value.
+   *
+   * The definition of this method is in graalRuntime.inline.hpp
+   * which is generated by com.oracle.graal.hotspot.sourcegen.GenGraalRuntimeInlineHpp.
+   *
+   * @param hotSpotOptionsClass the HotSpotOptions klass or NULL if only checking for valid option
+   * @param name option name
+   * @param name_len length of option name
+   * @param value '+' to set the option, '-' to reset the option
+   * @returns true if the option was found
+   * @throws InternalError if there was a problem setting the option's value
+   */
+  static bool set_option_bool(KlassHandle hotSpotOptionsClass, char* name, int name_len, char value, TRAPS);
+
+  /**
    * Searches for a Graal option denoted by a given name and sets it value.
    *
    * The definition of this method is in graalRuntime.inline.hpp