--- old/src/share/vm/prims/whitebox.cpp 2012-06-21 13:24:13.000000000 +0200 +++ new/src/share/vm/prims/whitebox.cpp 2012-06-21 13:24:13.000000000 +0200 @@ -113,6 +113,9 @@ int offset = offset_for_field(field_name, object, vmSymbols::string_signature()); oop string = object->obj_field(offset); + if (oopDesc::is_null(string)) { + return NULL; + } const char* ret = java_lang_String::as_utf8_string(string); return ret; } --- old/src/share/vm/services/diagnosticCommand.hpp 2012-06-21 13:24:14.000000000 +0200 +++ new/src/share/vm/services/diagnosticCommand.hpp 2012-06-21 13:24:14.000000000 +0200 @@ -48,7 +48,7 @@ "With no argument this will show a list of available commands. " "'help all' will show help for all commands."; } - static const char* impact() { return "Low: "; } + static const char* impact() { return "Low"; } static int num_arguments(); virtual void execute(TRAPS); }; @@ -60,7 +60,7 @@ static const char* description() { return "Print JVM version information."; } - static const char* impact() { return "Low: "; } + static const char* impact() { return "Low"; } static int num_arguments() { return 0; } virtual void execute(TRAPS); }; @@ -72,7 +72,7 @@ static const char* description() { return "Print the command line used to start this VM instance."; } - static const char* impact() { return "Low: "; } + static const char* impact() { return "Low"; } static int num_arguments() { return 0; } virtual void execute(TRAPS) { Arguments::print_on(_output); @@ -88,7 +88,7 @@ return "Print system properties."; } static const char* impact() { - return "Low: "; + return "Low"; } static int num_arguments() { return 0; } virtual void execute(TRAPS); @@ -105,7 +105,7 @@ return "Print VM flag options and their current values."; } static const char* impact() { - return "Low: "; + return "Low"; } static int num_arguments(); virtual void execute(TRAPS); @@ -121,7 +121,7 @@ return "Print VM uptime."; } static const char* impact() { - return "Low: "; + return "Low"; } static int num_arguments(); virtual void execute(TRAPS); --- old/src/share/vm/services/diagnosticFramework.cpp 2012-06-21 13:24:15.000000000 +0200 +++ new/src/share/vm/services/diagnosticFramework.cpp 2012-06-21 13:24:15.000000000 +0200 @@ -75,11 +75,13 @@ } // extracting first item, argument or option name _key_addr = &_buffer[_cursor]; + bool arg_had_quotes = false; while (_cursor <= _len - 1 && _buffer[_cursor] != '=' && _buffer[_cursor] != _delim) { // argument can be surrounded by single or double quotes if (_buffer[_cursor] == '\"' || _buffer[_cursor] == '\'') { _key_addr++; char quote = _buffer[_cursor]; + arg_had_quotes = true; while (_cursor < _len - 1) { _cursor++; if (_buffer[_cursor] == quote && _buffer[_cursor - 1] != '\\') { @@ -95,16 +97,22 @@ _cursor++; } _key_len = &_buffer[_cursor] - _key_addr; + if (arg_had_quotes) { + // if the argument was quoted, we need to step past the last quote here + _cursor++; + } // check if the argument has the = format if (_cursor <= _len -1 && _buffer[_cursor] == '=') { _cursor++; _value_addr = &_buffer[_cursor]; + bool value_had_quotes = false; // extract the value while (_cursor <= _len - 1 && _buffer[_cursor] != _delim) { // value can be surrounded by simple or double quotes if (_buffer[_cursor] == '\"' || _buffer[_cursor] == '\'') { _value_addr++; char quote = _buffer[_cursor]; + value_had_quotes = true; while (_cursor < _len - 1) { _cursor++; if (_buffer[_cursor] == quote && _buffer[_cursor - 1] != '\\') { @@ -120,6 +128,10 @@ _cursor++; } _value_len = &_buffer[_cursor] - _value_addr; + if (value_had_quotes) { + // if the value was quoted, we need to step past the last quote here + _cursor++; + } } else { _value_addr = NULL; _value_len = 0; @@ -185,8 +197,16 @@ arg->read_value(iter.key_addr(), iter.key_length(), CHECK); next_argument = next_argument->next(); } else { - THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), - "Unknown argument in diagnostic command"); + size_t buflen = 120; + char buf[buflen]; + size_t len = iter.key_length(); + char argname[len + 1]; + + strncpy(argname, iter.key_addr(), len); + argname[len] = '\0'; + jio_snprintf(buf, buflen - 1, "Unknown argument '%s' in diagnostic command.", argname); + + THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf); } } cont = iter.next(CHECK); @@ -207,19 +227,21 @@ } void DCmdParser::check(TRAPS) { + size_t buflen = 256; + char buf[buflen]; GenDCmdArgument* arg = _arguments_list; while (arg != NULL) { if (arg->is_mandatory() && !arg->has_value()) { - THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), - "Missing argument for diagnostic command"); + snprintf(buf, buflen - 1, "The argument '%s' is mandatory.", arg->name()); + THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf); } arg = arg->next(); } arg = _options; while (arg != NULL) { if (arg->is_mandatory() && !arg->has_value()) { - THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), - "Missing option for diagnostic command"); + snprintf(buf, buflen - 1, "The option '%s' is mandatory.", arg->name()); + THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf); } arg = arg->next(); } --- old/src/share/vm/services/diagnosticFramework.hpp 2012-06-21 13:24:16.000000000 +0200 +++ new/src/share/vm/services/diagnosticFramework.hpp 2012-06-21 13:24:16.000000000 +0200 @@ -238,6 +238,16 @@ static const char* name() { return "No Name";} static const char* description() { return "No Help";} static const char* disabled_message() { return "Diagnostic command currently disabled"; } + // The impact() method returns a description of the intrusiveness of the diagnostic + // command on the Java Virtual Machine behavior. The rational for this method is that some + // diagnostic commands can seriously disrupt the behavior of the Java Virtual Machine + // (for instance a Thread Dump for an application with several tens of thousands of threads, + // or a Head Dump with a 40GB+ heap size) and other diagnostic commands have no serious + // impact on the JVM (for instance, getting the command line arguments or the JVM version). + // The recommended format for the description is : [longer description], + // where the impact level is selected among this list: {Low, Medium, High}. The optional + // longer description can provide more specific details like the fact that Thread Dump + // impact depends on the heap size. static const char* impact() { return "Low: No impact"; } static int num_arguments() { return 0; } outputStream* output() { return _output; } @@ -250,7 +260,7 @@ bool has_arg = iter.next(CHECK); if (has_arg) { THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), - "Unknown argument in diagnostic command"); + "Argument list of diagnostic command should be empty."); } } virtual void execute(TRAPS) { } --- old/test/serviceability/ParserTest.java 2012-06-21 13:24:17.000000000 +0200 +++ new/test/serviceability/ParserTest.java 2012-06-21 13:24:17.000000000 +0200 @@ -20,6 +20,7 @@ testNanoTime(); testJLong(); testBool(); + testQuotes(); testMemorySize(); } @@ -95,6 +96,33 @@ parse(name, "false", "", args); } + public void testQuotes() throws Exception { + String name = "name"; + DiagnosticCommand arg1 = new DiagnosticCommand(name, + "desc", DiagnosticArgumentType.STRING, + false, null); + DiagnosticCommand arg2 = new DiagnosticCommand("arg", + "desc", DiagnosticArgumentType.STRING, + false, null); + DiagnosticCommand[] args = {arg1, arg2}; + + // try with a quoted value + parse(name, "Recording 1", name + "=\"Recording 1\"", args); + // try with a quoted argument + parse(name, "myrec", "\"" + name + "\"" + "=myrec", args); + // try with both a quoted value and a quoted argument + parse(name, "Recording 1", "\"" + name + "\"" + "=\"Recording 1\"", args); + + // now the same thing but with other arguments after + + // try with a quoted value + parse(name, "Recording 1", name + "=\"Recording 1\",arg=value", args); + // try with a quoted argument + parse(name, "myrec", "\"" + name + "\"" + "=myrec,arg=value", args); + // try with both a quoted value and a quoted argument + parse(name, "Recording 1", "\"" + name + "\"" + "=\"Recording 1\",arg=value", args); + } + public void testMemorySize() throws Exception { String name = "name"; String defaultValue = "1024";