7178703: Fix handling of quoted arguments and better error messages in dcmd
Reviewed-by: coleenp, mgronlun, rbackman
--- a/hotspot/src/share/vm/prims/whitebox.cpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/prims/whitebox.cpp Thu Jun 28 11:37:28 2012 +0200
@@ -113,6 +113,9 @@
int offset = offset_for_field(field_name, object,
vmSymbols::string_signature());
oop string = object->obj_field(offset);
+ if (string == NULL) {
+ return NULL;
+ }
const char* ret = java_lang_String::as_utf8_string(string);
return ret;
}
--- a/hotspot/src/share/vm/services/diagnosticCommand.hpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/diagnosticCommand.hpp Thu Jun 28 11:37:28 2012 +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);
--- a/hotspot/src/share/vm/services/diagnosticFramework.cpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/diagnosticFramework.cpp Thu Jun 28 11:37:28 2012 +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 <key>=<value> 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,17 @@
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");
+ const size_t buflen = 120;
+ const size_t argbuflen = 30;
+ char buf[buflen];
+ char argbuf[argbuflen];
+ size_t len = MIN2<size_t>(iter.key_length(), argbuflen - 1);
+
+ strncpy(argbuf, iter.key_addr(), len);
+ argbuf[len] = '\0';
+ jio_snprintf(buf, buflen - 1, "Unknown argument '%s' in diagnostic command.", argbuf);
+
+ THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf);
}
}
cont = iter.next(CHECK);
@@ -207,19 +228,21 @@
}
void DCmdParser::check(TRAPS) {
+ const 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");
+ jio_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");
+ jio_snprintf(buf, buflen - 1, "The option '%s' is mandatory.", arg->name());
+ THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), buf);
}
arg = arg->next();
}
--- a/hotspot/src/share/vm/services/diagnosticFramework.hpp Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/src/share/vm/services/diagnosticFramework.hpp Thu Jun 28 11:37:28 2012 +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 <impact level>: [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");
+ "The argument list of this diagnostic command should be empty.");
}
}
virtual void execute(TRAPS) { }
--- a/hotspot/test/serviceability/ParserTest.java Wed Jul 04 15:55:45 2012 -0400
+++ b/hotspot/test/serviceability/ParserTest.java Thu Jun 28 11:37:28 2012 +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";