Thu, 28 Jun 2012 11:37:28 +0200
7178703: Fix handling of quoted arguments and better error messages in dcmd
Reviewed-by: coleenp, mgronlun, rbackman
1.1 --- a/src/share/vm/prims/whitebox.cpp Wed Jul 04 15:55:45 2012 -0400 1.2 +++ b/src/share/vm/prims/whitebox.cpp Thu Jun 28 11:37:28 2012 +0200 1.3 @@ -113,6 +113,9 @@ 1.4 int offset = offset_for_field(field_name, object, 1.5 vmSymbols::string_signature()); 1.6 oop string = object->obj_field(offset); 1.7 + if (string == NULL) { 1.8 + return NULL; 1.9 + } 1.10 const char* ret = java_lang_String::as_utf8_string(string); 1.11 return ret; 1.12 }
2.1 --- a/src/share/vm/services/diagnosticCommand.hpp Wed Jul 04 15:55:45 2012 -0400 2.2 +++ b/src/share/vm/services/diagnosticCommand.hpp Thu Jun 28 11:37:28 2012 +0200 2.3 @@ -48,7 +48,7 @@ 2.4 "With no argument this will show a list of available commands. " 2.5 "'help all' will show help for all commands."; 2.6 } 2.7 - static const char* impact() { return "Low: "; } 2.8 + static const char* impact() { return "Low"; } 2.9 static int num_arguments(); 2.10 virtual void execute(TRAPS); 2.11 }; 2.12 @@ -60,7 +60,7 @@ 2.13 static const char* description() { 2.14 return "Print JVM version information."; 2.15 } 2.16 - static const char* impact() { return "Low: "; } 2.17 + static const char* impact() { return "Low"; } 2.18 static int num_arguments() { return 0; } 2.19 virtual void execute(TRAPS); 2.20 }; 2.21 @@ -72,7 +72,7 @@ 2.22 static const char* description() { 2.23 return "Print the command line used to start this VM instance."; 2.24 } 2.25 - static const char* impact() { return "Low: "; } 2.26 + static const char* impact() { return "Low"; } 2.27 static int num_arguments() { return 0; } 2.28 virtual void execute(TRAPS) { 2.29 Arguments::print_on(_output); 2.30 @@ -88,7 +88,7 @@ 2.31 return "Print system properties."; 2.32 } 2.33 static const char* impact() { 2.34 - return "Low: "; 2.35 + return "Low"; 2.36 } 2.37 static int num_arguments() { return 0; } 2.38 virtual void execute(TRAPS); 2.39 @@ -105,7 +105,7 @@ 2.40 return "Print VM flag options and their current values."; 2.41 } 2.42 static const char* impact() { 2.43 - return "Low: "; 2.44 + return "Low"; 2.45 } 2.46 static int num_arguments(); 2.47 virtual void execute(TRAPS); 2.48 @@ -121,7 +121,7 @@ 2.49 return "Print VM uptime."; 2.50 } 2.51 static const char* impact() { 2.52 - return "Low: "; 2.53 + return "Low"; 2.54 } 2.55 static int num_arguments(); 2.56 virtual void execute(TRAPS);
3.1 --- a/src/share/vm/services/diagnosticFramework.cpp Wed Jul 04 15:55:45 2012 -0400 3.2 +++ b/src/share/vm/services/diagnosticFramework.cpp Thu Jun 28 11:37:28 2012 +0200 3.3 @@ -75,11 +75,13 @@ 3.4 } 3.5 // extracting first item, argument or option name 3.6 _key_addr = &_buffer[_cursor]; 3.7 + bool arg_had_quotes = false; 3.8 while (_cursor <= _len - 1 && _buffer[_cursor] != '=' && _buffer[_cursor] != _delim) { 3.9 // argument can be surrounded by single or double quotes 3.10 if (_buffer[_cursor] == '\"' || _buffer[_cursor] == '\'') { 3.11 _key_addr++; 3.12 char quote = _buffer[_cursor]; 3.13 + arg_had_quotes = true; 3.14 while (_cursor < _len - 1) { 3.15 _cursor++; 3.16 if (_buffer[_cursor] == quote && _buffer[_cursor - 1] != '\\') { 3.17 @@ -95,16 +97,22 @@ 3.18 _cursor++; 3.19 } 3.20 _key_len = &_buffer[_cursor] - _key_addr; 3.21 + if (arg_had_quotes) { 3.22 + // if the argument was quoted, we need to step past the last quote here 3.23 + _cursor++; 3.24 + } 3.25 // check if the argument has the <key>=<value> format 3.26 if (_cursor <= _len -1 && _buffer[_cursor] == '=') { 3.27 _cursor++; 3.28 _value_addr = &_buffer[_cursor]; 3.29 + bool value_had_quotes = false; 3.30 // extract the value 3.31 while (_cursor <= _len - 1 && _buffer[_cursor] != _delim) { 3.32 // value can be surrounded by simple or double quotes 3.33 if (_buffer[_cursor] == '\"' || _buffer[_cursor] == '\'') { 3.34 _value_addr++; 3.35 char quote = _buffer[_cursor]; 3.36 + value_had_quotes = true; 3.37 while (_cursor < _len - 1) { 3.38 _cursor++; 3.39 if (_buffer[_cursor] == quote && _buffer[_cursor - 1] != '\\') { 3.40 @@ -120,6 +128,10 @@ 3.41 _cursor++; 3.42 } 3.43 _value_len = &_buffer[_cursor] - _value_addr; 3.44 + if (value_had_quotes) { 3.45 + // if the value was quoted, we need to step past the last quote here 3.46 + _cursor++; 3.47 + } 3.48 } else { 3.49 _value_addr = NULL; 3.50 _value_len = 0; 3.51 @@ -185,8 +197,17 @@ 3.52 arg->read_value(iter.key_addr(), iter.key_length(), CHECK); 3.53 next_argument = next_argument->next(); 3.54 } else { 3.55 - THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), 3.56 - "Unknown argument in diagnostic command"); 3.57 + const size_t buflen = 120; 3.58 + const size_t argbuflen = 30; 3.59 + char buf[buflen]; 3.60 + char argbuf[argbuflen]; 3.61 + size_t len = MIN2<size_t>(iter.key_length(), argbuflen - 1); 3.62 + 3.63 + strncpy(argbuf, iter.key_addr(), len); 3.64 + argbuf[len] = '\0'; 3.65 + jio_snprintf(buf, buflen - 1, "Unknown argument '%s' in diagnostic command.", argbuf); 3.66 + 3.67 + THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf); 3.68 } 3.69 } 3.70 cont = iter.next(CHECK); 3.71 @@ -207,19 +228,21 @@ 3.72 } 3.73 3.74 void DCmdParser::check(TRAPS) { 3.75 + const size_t buflen = 256; 3.76 + char buf[buflen]; 3.77 GenDCmdArgument* arg = _arguments_list; 3.78 while (arg != NULL) { 3.79 if (arg->is_mandatory() && !arg->has_value()) { 3.80 - THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), 3.81 - "Missing argument for diagnostic command"); 3.82 + jio_snprintf(buf, buflen - 1, "The argument '%s' is mandatory.", arg->name()); 3.83 + THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf); 3.84 } 3.85 arg = arg->next(); 3.86 } 3.87 arg = _options; 3.88 while (arg != NULL) { 3.89 if (arg->is_mandatory() && !arg->has_value()) { 3.90 - THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), 3.91 - "Missing option for diagnostic command"); 3.92 + jio_snprintf(buf, buflen - 1, "The option '%s' is mandatory.", arg->name()); 3.93 + THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf); 3.94 } 3.95 arg = arg->next(); 3.96 }
4.1 --- a/src/share/vm/services/diagnosticFramework.hpp Wed Jul 04 15:55:45 2012 -0400 4.2 +++ b/src/share/vm/services/diagnosticFramework.hpp Thu Jun 28 11:37:28 2012 +0200 4.3 @@ -238,6 +238,16 @@ 4.4 static const char* name() { return "No Name";} 4.5 static const char* description() { return "No Help";} 4.6 static const char* disabled_message() { return "Diagnostic command currently disabled"; } 4.7 + // The impact() method returns a description of the intrusiveness of the diagnostic 4.8 + // command on the Java Virtual Machine behavior. The rational for this method is that some 4.9 + // diagnostic commands can seriously disrupt the behavior of the Java Virtual Machine 4.10 + // (for instance a Thread Dump for an application with several tens of thousands of threads, 4.11 + // or a Head Dump with a 40GB+ heap size) and other diagnostic commands have no serious 4.12 + // impact on the JVM (for instance, getting the command line arguments or the JVM version). 4.13 + // The recommended format for the description is <impact level>: [longer description], 4.14 + // where the impact level is selected among this list: {Low, Medium, High}. The optional 4.15 + // longer description can provide more specific details like the fact that Thread Dump 4.16 + // impact depends on the heap size. 4.17 static const char* impact() { return "Low: No impact"; } 4.18 static int num_arguments() { return 0; } 4.19 outputStream* output() { return _output; } 4.20 @@ -250,7 +260,7 @@ 4.21 bool has_arg = iter.next(CHECK); 4.22 if (has_arg) { 4.23 THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), 4.24 - "Unknown argument in diagnostic command"); 4.25 + "The argument list of this diagnostic command should be empty."); 4.26 } 4.27 } 4.28 virtual void execute(TRAPS) { }
5.1 --- a/test/serviceability/ParserTest.java Wed Jul 04 15:55:45 2012 -0400 5.2 +++ b/test/serviceability/ParserTest.java Thu Jun 28 11:37:28 2012 +0200 5.3 @@ -20,6 +20,7 @@ 5.4 testNanoTime(); 5.5 testJLong(); 5.6 testBool(); 5.7 + testQuotes(); 5.8 testMemorySize(); 5.9 } 5.10 5.11 @@ -95,6 +96,33 @@ 5.12 parse(name, "false", "", args); 5.13 } 5.14 5.15 + public void testQuotes() throws Exception { 5.16 + String name = "name"; 5.17 + DiagnosticCommand arg1 = new DiagnosticCommand(name, 5.18 + "desc", DiagnosticArgumentType.STRING, 5.19 + false, null); 5.20 + DiagnosticCommand arg2 = new DiagnosticCommand("arg", 5.21 + "desc", DiagnosticArgumentType.STRING, 5.22 + false, null); 5.23 + DiagnosticCommand[] args = {arg1, arg2}; 5.24 + 5.25 + // try with a quoted value 5.26 + parse(name, "Recording 1", name + "=\"Recording 1\"", args); 5.27 + // try with a quoted argument 5.28 + parse(name, "myrec", "\"" + name + "\"" + "=myrec", args); 5.29 + // try with both a quoted value and a quoted argument 5.30 + parse(name, "Recording 1", "\"" + name + "\"" + "=\"Recording 1\"", args); 5.31 + 5.32 + // now the same thing but with other arguments after 5.33 + 5.34 + // try with a quoted value 5.35 + parse(name, "Recording 1", name + "=\"Recording 1\",arg=value", args); 5.36 + // try with a quoted argument 5.37 + parse(name, "myrec", "\"" + name + "\"" + "=myrec,arg=value", args); 5.38 + // try with both a quoted value and a quoted argument 5.39 + parse(name, "Recording 1", "\"" + name + "\"" + "=\"Recording 1\",arg=value", args); 5.40 + } 5.41 + 5.42 public void testMemorySize() throws Exception { 5.43 String name = "name"; 5.44 String defaultValue = "1024";