changeset 16270:d56a09df1a1f

implemented eager checking of Graal options (GRAAL-807)
author Doug Simon <doug.simon@oracle.com>
date Fri, 27 Jun 2014 19:55:54 +0200
parents f5437f2db322
children e589c26c2eb8
files graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotOptions.java src/share/vm/graal/graalRuntime.cpp src/share/vm/graal/graalRuntime.hpp src/share/vm/runtime/thread.cpp
diffstat 5 files changed, 142 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java	Fri Jun 27 17:24:28 2014 +0200
+++ b/graal/com.oracle.graal.hotspot.sourcegen/src/com/oracle/graal/hotspot/sourcegen/GenGraalRuntimeInlineHpp.java	Fri Jun 27 19:55:54 2014 +0200
@@ -140,8 +140,8 @@
         }
         lengths.add("PrintFlags".length());
 
-        out.println("bool GraalRuntime::set_option(KlassHandle hotSpotOptionsClass, const char* name, int name_len, Handle name_handle, const char* value, TRAPS) {");
-        out.println("  if (value[0] == '+' || value[0] == '-') {");
+        out.println("bool GraalRuntime::set_option(KlassHandle hotSpotOptionsClass, char* name, int name_len, const char* value, TRAPS) {");
+        out.println("  if (value != NULL && (value[0] == '+' || value[0] == '-')) {");
         out.println("    // boolean options");
         genMatchers(out, lengths, options, true);
         out.println("  } else {");
@@ -164,7 +164,11 @@
                 out.println("    case " + len + ":");
                 out.printf("      if (strncmp(name, \"PrintFlags\", %d) == 0) {%n", len);
                 out.println("        if (value[0] == '+') {");
-                out.println("          set_option_helper(hotSpotOptionsClass, name_handle, Handle(), '?', Handle(), 0L);");
+                out.println("          if (hotSpotOptionsClass.is_null()) {");
+                out.println("            TempNewSymbol name = SymbolTable::new_symbol(\"Lcom/oracle/graal/hotspot/HotSpotOptions;\", THREAD);");
+                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("        }");
                 out.println("        return true;");
                 out.println("      }");
@@ -178,17 +182,28 @@
                     }
                     out.printf("      if (strncmp(name, \"%s\", %d) == 0) {%n", e.getKey(), len);
                     Class<?> declaringClass = desc.getDeclaringClass();
-                    out.printf("        Handle option = get_OptionValue(\"L%s;\", \"%s\", \"L%s;\", CHECK_(true));%n", toInternalName(declaringClass), desc.getFieldName(),
-                                    toInternalName(getFieldType(desc)));
                     if (isBoolean) {
-                        out.println("        set_option_helper(hotSpotOptionsClass, name_handle, option, value[0], Handle(), 0L);");
+                        out.printf("        Handle option = get_OptionValue(\"L%s;\", \"%s\", \"L%s;\", CHECK_(true));%n", toInternalName(declaringClass), desc.getFieldName(),
+                                        toInternalName(getFieldType(desc)));
+                        out.println("        if (!hotSpotOptionsClass.is_null()) {");
+                        out.println("          set_option_helper(hotSpotOptionsClass, name, name_len, option, value[0], Handle(), 0L);");
+                        out.println("        }");
                     } else if (desc.getType() == String.class) {
-                        out.println("        Handle stringValue = java_lang_String::create_from_str(value, CHECK_(true));");
-                        out.println("        set_option_helper(hotSpotOptionsClass, name_handle, option, 's', stringValue, 0L);");
+                        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 (!hotSpotOptionsClass.is_null()) {");
+                        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_handle, value, CHECK_(true));");
-                        out.println("        set_option_helper(hotSpotOptionsClass, name_handle, option, '" + spec + "', Handle(), primitiveValue);");
+                        out.println("        jlong primitiveValue = parse_primitive_option_value('" + spec + "', name, name_len, value, CHECK_(true));");
+                        out.println("        if (!hotSpotOptionsClass.is_null()) {");
+                        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("        return true;");
                     out.println("      }");
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotOptions.java	Fri Jun 27 17:24:28 2014 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotOptions.java	Fri Jun 27 19:55:54 2014 +0200
@@ -207,7 +207,7 @@
         if (!matches.isEmpty()) {
             System.err.println("Did you mean one of the following?");
             for (OptionDescriptor match : matches) {
-                boolean isBoolean = match.getType() == boolean.class;
+                boolean isBoolean = match.getType() == boolean.class || match.getType() == Boolean.class;
                 System.err.println(String.format("    %s%s%s", isBoolean ? "(+/-)" : "", match.getName(), isBoolean ? "" : "=<value>"));
             }
         }
--- a/src/share/vm/graal/graalRuntime.cpp	Fri Jun 27 17:24:28 2014 +0200
+++ b/src/share/vm/graal/graalRuntime.cpp	Fri Jun 27 19:55:54 2014 +0200
@@ -701,6 +701,31 @@
 JVM_END
 #endif
 
+jint GraalRuntime::check_arguments(TRAPS) {
+  KlassHandle nullHandle;
+  parse_arguments(nullHandle, THREAD);
+  if (HAS_PENDING_EXCEPTION) {
+    // Errors in parsing Graal arguments cause exceptions.
+    // We now load and initialize HotSpotOptions which in turn
+    // causes argument parsing to be redone with better error messages.
+    CLEAR_PENDING_EXCEPTION;
+    TempNewSymbol name = SymbolTable::new_symbol("Lcom/oracle/graal/hotspot/HotSpotOptions;", THREAD);
+    instanceKlassHandle hotSpotOptionsClass = SystemDictionary::resolve_or_fail(name, true, THREAD);
+    GUARANTEE_NO_PENDING_EXCEPTION("Error in check_arguments");
+
+    parse_arguments(hotSpotOptionsClass, THREAD);
+    assert(HAS_PENDING_EXCEPTION, "must be");
+
+    ResourceMark rm;
+    oop message = java_lang_Throwable::message(PENDING_EXCEPTION);
+    assert(message != NULL, "Graal argument parsing exception is expected to hava message");
+    tty->print_cr("Error parsing Graal options: %s", java_lang_String::as_utf8_string(message));
+    CLEAR_PENDING_EXCEPTION;
+    return JNI_ERR;
+  }
+  return JNI_OK;
+}
+
 bool GraalRuntime::parse_arguments(KlassHandle hotSpotOptionsClass, TRAPS) {
   ResourceMark rm(THREAD);
 
@@ -716,43 +741,52 @@
   return CITime || CITimeEach;
 }
 
+void GraalRuntime::check_required_value(const char* name, int name_len, const char* value, TRAPS) {
+  if (value == NULL) {
+    char buf[200];
+    jio_snprintf(buf, sizeof(buf), "Value for option %.*s must use '-G:%.*s=<value>' format", name_len, name, name_len, name);
+    THROW_MSG(vmSymbols::java_lang_InternalError(), buf);
+  }
+}
+
 void GraalRuntime::parse_argument(KlassHandle hotSpotOptionsClass, char* arg, TRAPS) {
   char first = arg[0];
   char* name;
   size_t name_len;
-  Handle name_handle;
-  bool valid = true;
+  bool recognized = true;
   if (first == '+' || first == '-') {
     name = arg + 1;
     name_len = strlen(name);
-    name_handle = java_lang_String::create_from_str(name, CHECK);
-    valid = set_option(hotSpotOptionsClass, name, (int)name_len, name_handle, arg, CHECK);
+    recognized = set_option(hotSpotOptionsClass, name, (int)name_len, arg, CHECK);
   } else {
     char* sep = strchr(arg, '=');
+    name = arg;
+    char* value = NULL;
     if (sep != NULL) {
-      name = arg;
       name_len = sep - name;
-      // Temporarily replace '=' with NULL to create the Java string for the option name
-      *sep = '\0';
-      name_handle = java_lang_String::create_from_str(arg, THREAD);
-      *sep = '=';
-      if (HAS_PENDING_EXCEPTION) {
-        return;
+      value = sep + 1;
+    } else {
+      name_len = strlen(name);
+    }
+    recognized = set_option(hotSpotOptionsClass, name, (int)name_len, value, CHECK);
+  }
+
+  if (!recognized) {
+    bool throw_err = hotSpotOptionsClass.is_null();
+    if (!hotSpotOptionsClass.is_null()) {
+      instanceKlassHandle ikh(hotSpotOptionsClass());
+      if (!ikh->is_reentrant_initialization(THREAD)) {
+        set_option_helper(hotSpotOptionsClass, name, name_len, Handle(), ' ', Handle(), 0L);
+        throw_err = true;
       }
-      valid = set_option(hotSpotOptionsClass, name, (int)name_len, name_handle, sep + 1, CHECK);
-    } else {
+    }
+
+    if (throw_err) {
       char buf[200];
-      jio_snprintf(buf, sizeof(buf), "Value for option %s must use '-G:%s=<value>' format", arg, arg);
+      jio_snprintf(buf, sizeof(buf), "Unrecognized Graal option %.*s", name_len, name);
       THROW_MSG(vmSymbols::java_lang_InternalError(), buf);
     }
   }
-
-  if (!valid) {
-    set_option_helper(hotSpotOptionsClass, name_handle, Handle(), ' ', Handle(), 0L);
-    char buf[200];
-    jio_snprintf(buf, sizeof(buf), "Invalid Graal option %s", arg);
-    THROW_MSG(vmSymbols::java_lang_InternalError(), buf);
-  }
 }
 
 void GraalRuntime::parse_graal_options_file(KlassHandle hotSpotOptionsClass, TRAPS) {
@@ -802,7 +836,8 @@
   }
 }
 
-jlong GraalRuntime::parse_primitive_option_value(char spec, Handle name, const char* value, TRAPS) {
+jlong GraalRuntime::parse_primitive_option_value(char spec, const char* name, int name_len, const char* value, TRAPS) {
+  check_required_value(name, name_len, value, CHECK_(0L));
   union {
     jint i;
     jlong l;
@@ -829,23 +864,39 @@
   }
   ResourceMark rm(THREAD);
   char buf[200];
-  jio_snprintf(buf, sizeof(buf), "Invalid %s value for Graal option %s: %s", (spec == 'i' ? "numeric" : "float/double"), java_lang_String::as_utf8_string(name()), value);
+  jio_snprintf(buf, sizeof(buf), "Invalid %s value for Graal option %.*s: %s", (spec == 'i' ? "numeric" : "float/double"), name, name_len, value);
   THROW_MSG_(vmSymbols::java_lang_InternalError(), buf, 0L);
 }
 
-void GraalRuntime::set_option_helper(KlassHandle hotSpotOptionsClass, Handle name, Handle option, jchar spec, Handle stringValue, jlong primitiveValue) {
+void GraalRuntime::set_option_helper(KlassHandle hotSpotOptionsClass, char* name, int name_len, Handle option, jchar spec, Handle stringValue, jlong primitiveValue) {
   Thread* THREAD = Thread::current();
+  Handle name_handle;
+  if (name != NULL) {
+    if ((int) strlen(name) > name_len) {
+      // Temporarily replace '=' with NULL to create the Java string for the option name
+      char save = name[name_len];
+      name[name_len] = '\0';
+      name_handle = java_lang_String::create_from_str(name, THREAD);
+      name[name_len] = '=';
+      if (HAS_PENDING_EXCEPTION) {
+        return;
+      }
+    } else {
+      assert((int) strlen(name) == name_len, "must be");
+      name_handle = java_lang_String::create_from_str(name, CHECK);
+    }
+  }
+
   TempNewSymbol setOption = SymbolTable::new_symbol("setOption", THREAD);
   TempNewSymbol sig = SymbolTable::new_symbol("(Ljava/lang/String;Lcom/oracle/graal/options/OptionValue;CLjava/lang/String;J)V", THREAD);
   JavaValue result(T_VOID);
   JavaCallArguments args;
-  args.push_oop(name());
+  args.push_oop(name_handle());
   args.push_oop(option());
   args.push_int(spec);
   args.push_oop(stringValue());
   args.push_long(primitiveValue);
-  JavaCalls::call_static(&result, hotSpotOptionsClass, setOption, sig, &args, THREAD);
-  GUARANTEE_NO_PENDING_EXCEPTION("Error while calling set_option_helper");
+  JavaCalls::call_static(&result, hotSpotOptionsClass, setOption, sig, &args, CHECK);
 }
 
 Handle GraalRuntime::get_OptionValue(const char* declaringClass, const char* fieldName, const char* fieldSig, TRAPS) {
@@ -906,6 +957,7 @@
 
   assert(exception->is_a(SystemDictionary::Throwable_klass()), "Throwable instance expected");
   JavaValue result(T_VOID);
+  tty->print_cr(message);
   JavaCalls::call_virtual(&result,
                           exception,
                           KlassHandle(THREAD,
--- a/src/share/vm/graal/graalRuntime.hpp	Fri Jun 27 17:24:28 2014 +0200
+++ b/src/share/vm/graal/graalRuntime.hpp	Fri Jun 27 19:55:54 2014 +0200
@@ -45,11 +45,12 @@
    * Parses the string form of a numeric, float or double option into a jlong (using raw bits for floats/doubles).
    *
    * @param spec 'i', 'f' or 'd' (see HotSpotOptions.setOption())
-   * @param name name option option
+   * @param name option name
+   * @param name_len length of option name
    * @param value string value to parse
    * @throws InternalError if value could not be parsed according to spec
    */
-  static jlong parse_primitive_option_value(char spec, Handle name, const char* value, TRAPS);
+  static jlong parse_primitive_option_value(char spec, const char* name, int name_len, const char* value, TRAPS);
 
   /**
    * Loads default option value overrides from a <jre_home>/lib/graal.options if it exists. Each
@@ -72,15 +73,26 @@
    * 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
    * @returns true if the option was found
    * @throws InternalError if there was a problem setting the option's value
    */
-  static bool set_option(KlassHandle hotSpotOptionsClass, const char* name, int name_len, Handle name_handle, const char* value, TRAPS);
+  static bool set_option(KlassHandle hotSpotOptionsClass, char* name, int name_len, const char* value, TRAPS);
+
+  /**
+   * Raises an InternalError for an option that expects a value but was specified without a "=<value>" prefix.
+   */
+  static void check_required_value(const char* name, int name_len, const char* value, TRAPS);
 
   /**
    * Java call to HotSpotOptions.setOption(String name, OptionValue<?> option, char spec, String stringValue, long primitiveValue)
+   *
+   * @param name option name
+   * @param name_len length of option name
    */
-  static void set_option_helper(KlassHandle hotSpotOptionsClass, Handle name, Handle option, jchar spec, Handle stringValue, jlong primitiveValue);
+  static void set_option_helper(KlassHandle hotSpotOptionsClass, char* name, int name_len, Handle option, jchar spec, Handle stringValue, jlong primitiveValue);
 
   /**
    * Instantiates a service object, calls its default constructor and returns it.
@@ -132,6 +144,18 @@
 
   static BufferBlob* initialize_buffer_blob();
 
+  /**
+   * Checks that all Graal specific VM options presented by the launcher are recognized
+   * and formatted correctly. To set relevant Java fields from the option, parse_arguments()
+   * must be called. This method makes no Java calls apart from creating exception objects
+   * if there is an errors in the Graal options.
+   */
+  static jint check_arguments(TRAPS);
+
+  /**
+   * Parses the Graal specific VM options that were presented by the launcher and sets
+   * the relevants Java fields.
+   */
   static bool parse_arguments(KlassHandle hotSpotOptionsClass, TRAPS);
 
   static BasicType kindToBasicType(jchar ch);
--- a/src/share/vm/runtime/thread.cpp	Fri Jun 27 17:24:28 2014 +0200
+++ b/src/share/vm/runtime/thread.cpp	Fri Jun 27 19:55:54 2014 +0200
@@ -31,6 +31,7 @@
 #include "compiler/compileBroker.hpp"
 #ifdef GRAAL
 #include "graal/graalCompiler.hpp"
+#include "graal/graalRuntime.hpp"
 #endif
 #include "interpreter/interpreter.hpp"
 #include "interpreter/linkResolver.hpp"
@@ -3731,6 +3732,14 @@
     Chunk::start_chunk_pool_cleaner_task();
   }
 
+#ifdef GRAAL
+  status = GraalRuntime::check_arguments(main_thread);
+  if (status != JNI_OK) {
+    *canTryAgain = false; // don't let caller call JNI_CreateJavaVM again
+    return status;
+  }
+#endif
+
   // initialize compiler(s)
 #if defined(COMPILER1) || defined(COMPILER2) || defined(SHARK) || defined(COMPILERGRAAL)
   CompileBroker::compilation_init();