# HG changeset patch # User Doug Simon # Date 1403891754 -7200 # Node ID d56a09df1a1fb0f04a84732d4c6a60e275c19d52 # Parent f5437f2db322c6109299109319a08c6f79a43841 implemented eager checking of Graal options (GRAAL-807) diff -r f5437f2db322 -r d56a09df1a1f 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 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(" }"); diff -r f5437f2db322 -r d56a09df1a1f 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 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 ? "" : "=")); } } diff -r f5437f2db322 -r d56a09df1a1f src/share/vm/graal/graalRuntime.cpp --- 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=' 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=' 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, diff -r f5437f2db322 -r d56a09df1a1f src/share/vm/graal/graalRuntime.hpp --- 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 /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 "=" 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); diff -r f5437f2db322 -r d56a09df1a1f src/share/vm/runtime/thread.cpp --- 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();