8065783: DCMD parser fails to recognize one character argument when it's positioned last

Reviewed-by: sla, egahlin, fparain

hotspot/src/share/vm/prims/wbtestmethods/parserTests.cpp

@@ -70,38 +70,63 @@ static void fill_in_parser(DCmdParser* parser, oop argument)
   const char* desc = WhiteBox::lookup_jstring("desc", argument);
   const char* default_value = WhiteBox::lookup_jstring("defaultValue", argument);
   bool mandatory = WhiteBox::lookup_bool("mandatory", argument);
+  bool isarg = WhiteBox::lookup_bool("argument", argument);
   const char*  type = lookup_diagnosticArgumentEnum("type", argument);
 
    if (strcmp(type, "STRING") == 0) {
      DCmdArgument<char*>* argument = new DCmdArgument<char*>(
      name, desc,
      "STRING", mandatory, default_value);
-     parser->add_dcmd_option(argument);
+     if (isarg) {
+      parser->add_dcmd_argument(argument);
+     } else {
+      parser->add_dcmd_option(argument);
+     }
    } else if (strcmp(type, "NANOTIME") == 0) {
      DCmdArgument<NanoTimeArgument>* argument = new DCmdArgument<NanoTimeArgument>(
      name, desc,
      "NANOTIME", mandatory, default_value);
-     parser->add_dcmd_option(argument);
+     if (isarg) {
+      parser->add_dcmd_argument(argument);
+     } else {
+      parser->add_dcmd_option(argument);
+     }
    } else if (strcmp(type, "JLONG") == 0) {
      DCmdArgument<jlong>* argument = new DCmdArgument<jlong>(
      name, desc,
      "JLONG", mandatory, default_value);
-     parser->add_dcmd_option(argument);
+     if (isarg) {
+      parser->add_dcmd_argument(argument);
+     } else {
+      parser->add_dcmd_option(argument);
+     }
    } else if (strcmp(type, "BOOLEAN") == 0) {
      DCmdArgument<bool>* argument = new DCmdArgument<bool>(
      name, desc,
      "BOOLEAN", mandatory, default_value);
-     parser->add_dcmd_option(argument);
+     if (isarg) {
+      parser->add_dcmd_argument(argument);
+     } else {
+      parser->add_dcmd_option(argument);
+     }
    } else if (strcmp(type, "MEMORYSIZE") == 0) {
      DCmdArgument<MemorySizeArgument>* argument = new DCmdArgument<MemorySizeArgument>(
      name, desc,
      "MEMORY SIZE", mandatory, default_value);
-     parser->add_dcmd_option(argument);
+     if (isarg) {
+      parser->add_dcmd_argument(argument);
+     } else {
+      parser->add_dcmd_option(argument);
+     }
    } else if (strcmp(type, "STRINGARRAY") == 0) {
      DCmdArgument<StringArrayArgument*>* argument = new DCmdArgument<StringArrayArgument*>(
      name, desc,
      "STRING SET", mandatory);
-     parser->add_dcmd_option(argument);
+     if (isarg) {
+      parser->add_dcmd_argument(argument);
+     } else {
+      parser->add_dcmd_option(argument);
+     }
    }
 }
 
@@ -111,11 +136,12 @@ static void fill_in_parser(DCmdParser* parser, oop argument)
  * { name, value, name, value ... }
  * This can then be checked from java.
  */
-WB_ENTRY(jobjectArray, WB_ParseCommandLine(JNIEnv* env, jobject o, jstring j_cmdline, jobjectArray arguments))
+WB_ENTRY(jobjectArray, WB_ParseCommandLine(JNIEnv* env, jobject o, jstring j_cmdline, jchar j_delim, jobjectArray arguments))
   ResourceMark rm;
   DCmdParser parser;
 
   const char* c_cmdline = java_lang_String::as_utf8_string(JNIHandles::resolve(j_cmdline));
+  const char c_delim = j_delim & 0xff;
   objArrayOop argumentArray = objArrayOop(JNIHandles::resolve_non_null(arguments));
   objArrayHandle argumentArray_ah(THREAD, argumentArray);
 
@@ -127,20 +153,29 @@ WB_ENTRY(jobjectArray, WB_ParseCommandLine(JNIEnv* env, jobject o, jstring j_cmd
   }
 
   CmdLine cmdline(c_cmdline, strlen(c_cmdline), true);
-  parser.parse(&cmdline,',',CHECK_NULL);
+  parser.parse(&cmdline,c_delim,CHECK_NULL);
 
   Klass* k = SystemDictionary::Object_klass();
   objArrayOop returnvalue_array = oopFactory::new_objArray(k, parser.num_arguments() * 2, CHECK_NULL);
   objArrayHandle returnvalue_array_ah(THREAD, returnvalue_array);
 
   GrowableArray<const char *>*parsedArgNames = parser.argument_name_array();
+  GenDCmdArgument* arglist = parser.arguments_list();
 
   for (int i = 0; i < parser.num_arguments(); i++) {
     oop parsedName = java_lang_String::create_oop_from_str(parsedArgNames->at(i), CHECK_NULL);
     returnvalue_array_ah->obj_at_put(i*2, parsedName);
     GenDCmdArgument* arg = parser.lookup_dcmd_option(parsedArgNames->at(i), strlen(parsedArgNames->at(i)));
+    if (!arg) {
+      arg = arglist;
+      arglist = arglist->next();
+    }
     char buf[VALUE_MAXLEN];
-    arg->value_as_str(buf, sizeof(buf));
+    if (arg) {
+      arg->value_as_str(buf, sizeof(buf));
+    } else {
+      sprintf(buf, "<null>");
+    }
     oop parsedValue = java_lang_String::create_oop_from_str(buf, CHECK_NULL);
     returnvalue_array_ah->obj_at_put(i*2+1, parsedValue);
   }

hotspot/src/share/vm/prims/wbtestmethods/parserTests.hpp

@@ -27,6 +27,6 @@
 #include "prims/jni.h"
 #include "prims/whitebox.hpp"
 
-WB_METHOD_DECLARE(jobjectArray) WB_ParseCommandLine(JNIEnv* env, jobject o, jstring args, jobjectArray arguments);
+WB_METHOD_DECLARE(jobjectArray) WB_ParseCommandLine(JNIEnv* env, jobject o, jstring args, jchar delim, jobjectArray arguments);
 
 #endif //SHARE_VM_PRIMS_WBTESTMETHODS_PARSERTESTS_H

hotspot/src/share/vm/prims/whitebox.cpp

@@ -1127,7 +1127,7 @@ static JNINativeMethod methods[] = {
   {CC"getVMPageSize",      CC"()I",                   (void*)&WB_GetVMPageSize     },
   {CC"isClassAlive0",      CC"(Ljava/lang/String;)Z", (void*)&WB_IsClassAlive      },
   {CC"parseCommandLine",
-      CC"(Ljava/lang/String;[Lsun/hotspot/parser/DiagnosticCommand;)[Ljava/lang/Object;",
+      CC"(Ljava/lang/String;C[Lsun/hotspot/parser/DiagnosticCommand;)[Ljava/lang/Object;",
       (void*) &WB_ParseCommandLine
   },
   {CC"addToBootstrapClassLoaderSearch", CC"(Ljava/lang/String;)V",

hotspot/src/share/vm/services/diagnosticFramework.cpp

@@ -60,16 +60,15 @@ CmdLine::CmdLine(const char* line, size_t len, bool no_command_name) {
 
 bool DCmdArgIter::next(TRAPS) {
   if (_len == 0) return false;
-  // skipping spaces
+  // skipping delimiters
   while (_cursor < _len - 1 && _buffer[_cursor] == _delim) {
     _cursor++;
   }
   // handling end of command line
-  if (_cursor >= _len - 1) {
-    _cursor = _len - 1;
-    _key_addr = &_buffer[_len - 1];
+  if (_cursor == _len - 1 && _buffer[_cursor] == _delim) {
+    _key_addr = &_buffer[_cursor];
     _key_len = 0;
-    _value_addr = &_buffer[_len - 1];
+    _value_addr = &_buffer[_cursor];
     _value_len = 0;
     return false;
   }

hotspot/test/serviceability/ParserTest.java

@@ -48,6 +48,7 @@ public class ParserTest {
         testBool();
         testQuotes();
         testMemorySize();
+        testSingleLetterArg();
     }
 
     public static void main(String... args) throws Exception  {
@@ -99,7 +100,7 @@ public class ParserTest {
                 false, "0");
         DiagnosticCommand[] args = {arg};
 
-        wb.parseCommandLine(name + "=10", args);
+        wb.parseCommandLine(name + "=10", ',', args);
         parse(name, "10", name + "=10", args);
         parse(name, "-5", name + "=-5", args);
 
@@ -149,6 +150,15 @@ public class ParserTest {
         parse(name, "Recording 1", "\"" + name + "\"" + "=\"Recording 1\",arg=value", args);
     }
 
+    public void testSingleLetterArg() throws Exception {
+        DiagnosticCommand[] args = new DiagnosticCommand[]{
+            new DiagnosticCommand("flag", "desc", DiagnosticArgumentType.STRING, true, false, null),
+            new DiagnosticCommand("value", "desc", DiagnosticArgumentType.STRING, true, false, null)
+        };
+        parse("flag", "flag", "flag v", ' ', args);
+        parse("value", "v", "flag v", ' ', args);
+    }
+
     public void testMemorySize() throws Exception {
         String name = "name";
         String defaultValue = "1024";
@@ -176,9 +186,13 @@ public class ParserTest {
 
     public void parse(String searchName, String expectedValue,
             String cmdLine, DiagnosticCommand[] argumentTypes) throws Exception {
+        parse(searchName, expectedValue, cmdLine, ',', argumentTypes);
+    }
+    public void parse(String searchName, String expectedValue,
+            String cmdLine, char delim, DiagnosticCommand[] argumentTypes) throws Exception {
         //parseCommandLine will return an object array that looks like
         //{<name of parsed object>, <of parsed object> ... }
-        Object[] res = wb.parseCommandLine(cmdLine, argumentTypes);
+        Object[] res = wb.parseCommandLine(cmdLine, delim, argumentTypes);
         for (int i = 0; i < res.length-1; i+=2) {
             String parsedName = (String) res[i];
             if (searchName.equals(parsedName)) {
@@ -196,8 +210,11 @@ public class ParserTest {
     }
 
     private void shouldFail(String argument, DiagnosticCommand[] argumentTypes) throws Exception {
+        shouldFail(argument, ',', argumentTypes);
+    }
+    private void shouldFail(String argument, char delim, DiagnosticCommand[] argumentTypes) throws Exception {
         try {
-            wb.parseCommandLine(argument, argumentTypes);
+            wb.parseCommandLine(argument, delim, argumentTypes);
             throw new Exception("Parser accepted argument: " + argument);
         } catch (IllegalArgumentException e) {
             //expected

hotspot/test/testlibrary/whitebox/sun/hotspot/WhiteBox.java

@@ -94,7 +94,7 @@ public class WhiteBox {
   public native boolean g1IsHumongous(Object o);
   public native long    g1NumFreeRegions();
   public native int     g1RegionSize();
-  public native Object[]    parseCommandLine(String commandline, DiagnosticCommand[] args);
+  public native Object[]    parseCommandLine(String commandline, char delim, DiagnosticCommand[] args);
 
   // NMT
   public native long NMTMalloc(long size);

hotspot/test/testlibrary/whitebox/sun/hotspot/parser/DiagnosticCommand.java

@@ -34,14 +34,21 @@ public class DiagnosticCommand {
     private DiagnosticArgumentType type;
     private boolean mandatory;
     private String defaultValue;
+    private boolean argument;
 
     public DiagnosticCommand(String name, String desc, DiagnosticArgumentType type,
             boolean mandatory, String defaultValue) {
+        this(name, desc, type, false, mandatory, defaultValue);
+    }
+
+    public DiagnosticCommand(String name, String desc, DiagnosticArgumentType type,
+            boolean argument, boolean mandatory, String defaultValue) {
         this.name = name;
         this.desc = desc;
         this.type = type;
         this.mandatory = mandatory;
         this.defaultValue = defaultValue;
+        this.argument = argument;
     }
 
     public String getName() {
@@ -60,6 +67,10 @@ public class DiagnosticCommand {
         return mandatory;
     }
 
+    public boolean isArgument() {
+        return argument;
+    }
+
     public String getDefaultValue() {
         return defaultValue;
     }