8257800: CompileCommand TypedMethodOptionMatcher::parse_method_pattern() may over consume

Reviewed-by: thartmann, chagedorn, phh

Author: Xin Liu

src/hotspot/share/compiler/compilerOracle.cpp

@@ -428,7 +428,7 @@ bool CompilerOracle::should_blackhole(const methodHandle& method) {
   return true;
 }
 
-static enum CompileCommand parse_option_name(const char* line, int* bytes_read, char* errorbuf, int bufsize) {
+static enum CompileCommand match_option_name(const char* line, int* bytes_read, char* errorbuf, int bufsize) {
   assert(ARRAY_SIZE(option_names) == static_cast<int>(CompileCommand::Count), "option_names size mismatch");
 
   *bytes_read = 0;
@@ -445,6 +445,25 @@ static enum CompileCommand parse_option_name(const char* line, int* bytes_read,
   return CompileCommand::Unknown;
 }
 
+// match exactly and don't mess with errorbuf
+enum CompileCommand CompilerOracle::parse_option_name(const char* line) {
+  for (uint i = 0; i < ARRAY_SIZE(option_names); i++) {
+    if (strcasecmp(line, option_names[i]) == 0) {
+      return static_cast<enum CompileCommand>(i);
+    }
+  }
+  return CompileCommand::Unknown;
+}
+
+enum OptionType CompilerOracle::parse_option_type(const char* type_str) {
+  for (uint i = 0; i < ARRAY_SIZE(optiontype_names); i++) {
+    if (strcasecmp(type_str, optiontype_names[i]) == 0) {
+      return static_cast<enum OptionType>(i);
+    }
+  }
+  return OptionType::Unknown;
+}
+
 void print_tip() { // CMH Update info
   tty->cr();
   tty->print_cr("Usage: '-XX:CompileCommand=<option>,<method pattern>' - to set boolean option to true");
@@ -663,7 +682,7 @@ static void scan_option_and_value(enum OptionType type, char* line, int& total_b
     total_bytes_read += bytes_read;
     int bytes_read2 = 0;
     total_bytes_read += skip_whitespace(line);
-    enum CompileCommand option = parse_option_name(option_buf, &bytes_read2, errorbuf, buf_size);
+    enum CompileCommand option = match_option_name(option_buf, &bytes_read2, errorbuf, buf_size);
     if (option == CompileCommand::Unknown) {
       assert(*errorbuf != '\0', "error must have been set");
       return;
@@ -692,15 +711,6 @@ void CompilerOracle::print_parse_error(char* error_msg, char* original_line) {
   print_tip();
 }
 
-enum OptionType parse_option_type(const char* type_str) {
-  for (uint i = 0; i < ARRAY_SIZE(optiontype_names); i++) {
-    if (strcasecmp(type_str, optiontype_names[i]) == 0) {
-      return static_cast<enum OptionType>(i);
-    }
-  }
-  return OptionType::Unknown;
-}
-
 class LineCopy : StackObj {
   const char* _copy;
 public:
@@ -723,7 +733,7 @@ void CompilerOracle::parse_from_line(char* line) {
   int bytes_read;
   char error_buf[1024] = {0};
 
-  enum CompileCommand option = parse_option_name(line, &bytes_read, error_buf, sizeof(error_buf));
+  enum CompileCommand option = match_option_name(line, &bytes_read, error_buf, sizeof(error_buf));
   line += bytes_read;
   ResourceMark rm;
 
@@ -784,7 +794,7 @@ void CompilerOracle::parse_from_line(char* line) {
       } else {
         // Type (1) option - option_type contains the option name -> bool value = true is implied
         int bytes_read;
-        enum CompileCommand option = parse_option_name(option_type, &bytes_read, error_buf, sizeof(error_buf));
+        enum CompileCommand option = match_option_name(option_type, &bytes_read, error_buf, sizeof(error_buf));
         if (option == CompileCommand::Unknown) {
           print_parse_error(error_buf, original.get());
           return;
@@ -1001,5 +1011,5 @@ void CompilerOracle::parse_compile_only(char* line) {
 enum CompileCommand CompilerOracle::string_to_option(const char* name) {
   int bytes_read = 0;
   char errorbuf[1024] = {0};
-  return parse_option_name(name, &bytes_read, errorbuf, sizeof(errorbuf));
+  return match_option_name(name, &bytes_read, errorbuf, sizeof(errorbuf));
 }

src/hotspot/share/compiler/compilerOracle.hpp

@@ -171,6 +171,14 @@ class CompilerOracle : AllStatic {
   // convert a string to a proper compilecommand option - used from whitebox.
   // returns CompileCommand::Unknown on names not matching an option.
   static enum CompileCommand string_to_option(const char* name);
+
+  // convert a string to a proper compilecommand option
+  // returns CompileCommand::Unknown if name is not an option.
+  static enum CompileCommand parse_option_name(const char* name);
+
+  // convert a string to a proper option type
+  // returns OptionType::Unknown on strings not matching an option type.
+  static enum OptionType parse_option_type(const char* type_str);
 };
 
 #endif // SHARE_COMPILER_COMPILERORACLE_HPP

src/hotspot/share/compiler/methodMatcher.cpp

@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "classfile/symbolTable.hpp"
 #include "classfile/vmSymbols.hpp"
+#include "compiler/compilerOracle.hpp"
 #include "compiler/methodMatcher.hpp"
 #include "memory/oopFactory.hpp"
 #include "memory/resourceArea.hpp"
@@ -105,7 +106,6 @@ bool MethodMatcher::canonicalize(char * line, const char *& error_msg) {
       }
     }
 
-    bool in_signature = false;
     char* pos = line;
     if (pos != NULL) {
       for (char* lp = pos + 1; *lp != '\0'; lp++) {
@@ -269,11 +269,25 @@ void MethodMatcher::parse_method_pattern(char*& line, const char*& error_msg, Me
     c_match = check_mode(class_name, error_msg);
     m_match = check_mode(method_name, error_msg);
 
+    // Over-consumption
+    // method_name points to an option type or option name because the method name is not specified by users.
+    // In very rare case, the method name happens to be same as option type/name, so look ahead to make sure
+    // it doesn't show up again.
+    if ((OptionType::Unknown != CompilerOracle::parse_option_type(method_name) ||
+        CompileCommand::Unknown != CompilerOracle::parse_option_name(method_name)) &&
+        *(line + bytes_read) != '\0' &&
+        strstr(line + bytes_read, method_name) == NULL) {
+      error_msg = "Did not specify any method name";
+      method_name[0] = '\0';
+      return;
+    }
+
     if ((strchr(class_name, JVM_SIGNATURE_SPECIAL) != NULL) ||
         (strchr(class_name, JVM_SIGNATURE_ENDSPECIAL) != NULL)) {
       error_msg = "Chars '<' and '>' not allowed in class name";
       return;
     }
+
     if ((strchr(method_name, JVM_SIGNATURE_SPECIAL) != NULL) ||
         (strchr(method_name, JVM_SIGNATURE_ENDSPECIAL) != NULL)) {
       if (!vmSymbols::object_initializer_name()->equals(method_name) &&

test/hotspot/jtreg/compiler/compilercontrol/commands/OptionTest.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8257800
+ * @summary Tests CompileCommand=option,package/class,ccstrlist,ControlIntrinsic,+_id
+ * @library /test/lib /
+ *
+ * @run driver compiler.compilercontrol.commands.OptionTest
+ */
+
+package compiler.compilercontrol.commands;
+
+import jdk.test.lib.process.ProcessTools;
+
+public class OptionTest {
+    public static void main(String[] args) throws Exception {
+        ProcessTools.executeTestJvm("-XX:CompileCommand=option,package/class,ccstrlist,ControlIntrinsic,+_id", "-version")
+                    .shouldHaveExitValue(0)
+                    .shouldContain("CompileCommand: An error occurred during parsing")
+                    .shouldContain("Error: Did not specify any method name")
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+
+        ProcessTools.executeTestJvm("-XX:CompileCommand=option,*,ccstrlist,ControlIntrinsic,+_id", "-version")
+                    .shouldHaveExitValue(0)
+                    .shouldContain("CompileCommand: An error occurred during parsing")
+                    .shouldContain("Error: Did not specify any method name")
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+
+        // corner case:
+        // ccstrlist could be a valid method name, so it is accepted in the well-formed case.
+        ProcessTools.executeTestJvm("-XX:CompileCommand=option,*.ccstrlist,ccstrlist,ControlIntrinsic,+_id", "-version")
+                    .shouldContain("CompileCommand: ControlIntrinsic *.ccstrlist const char* ControlIntrinsic")
+                    .shouldHaveExitValue(0)
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+
+        ProcessTools.executeTestJvm("-XX:CompileCommand=option,*.*,ccstrlist,ControlIntrinsic,+_id", "-version")
+                    .shouldContain("CompileCommand: ControlIntrinsic *.* const char* ControlIntrinsic")
+                    .shouldHaveExitValue(0)
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+
+        ProcessTools.executeTestJvm("-XX:CompileCommand=option,class,PrintIntrinsics", "-version")
+                    .shouldHaveExitValue(0)
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+
+        // corner case:
+        // PrintIntrinsics could be a valid method name, so it is accepted in the well-formed case.
+        ProcessTools.executeTestJvm("-XX:CompileCommand=option,class.PrintIntrinsics,PrintIntrinsics", "-version")
+                    .shouldContain("CompileCommand: PrintIntrinsics class.PrintIntrinsics bool PrintIntrinsics = true")
+                    .shouldHaveExitValue(0)
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+
+        // corner case:
+        // _dontinline_* is a valid method pattern, so it should be accepted
+        ProcessTools.executeTestJvm("-XX:CompileCommand=dontinline,*::dontinline_*", "-version")
+                    .shouldContain("CompileCommand: dontinline *.dontinline_* bool dontinline = true")
+                    .shouldHaveExitValue(0)
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+
+        ProcessTools.executeTestJvm("-XX:CompileCommand=dontinline,*.dontinline", "-version")
+                    .shouldContain("CompileCommand: dontinline *.dontinline bool dontinline = true")
+                    .shouldHaveExitValue(0)
+                    .shouldNotContain("Error: Did not specify any method name")
+                    .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
+    }
+}