8247732: validate user-input intrinsic_ids in ControlIntrinsic

renew webrev to the latest jdk. fixed a typo and a bug.  Add constraints for both DisableIntrinsic and ControlIntrinsics.  Add tests to cover different use cases of them.

Reviewed-by: neliasso, thartmann

src/hotspot/share/compiler/compilerDirectives.cpp

@@ -280,7 +280,7 @@ DirectiveSet::~DirectiveSet() {
 // 2) cloned() returns a pointer that points to the cloned DirectiveSet.
 // Users should only use cloned() when they need to update DirectiveSet.
 //
-// In the end, users need invoke commit() to finalize the pending changes.
+// In the end, users need to invoke commit() to finalize the pending changes.
 // If cloning happens, the smart pointer will return the new pointer after releasing the original
 // one on DirectivesStack. If cloning doesn't happen, it returns the original intact pointer.
 class DirectiveSetPtr {

src/hotspot/share/compiler/compilerDirectives.hpp

@@ -166,9 +166,11 @@ void print(outputStream* st) {
   }
 };
 
-// Iterator of ControlIntrinsic
+// Iterator of ControlIntrinsic=+_id1,-_id2,+_id3,...
-// if disable_all is true, it accepts DisableIntrinsic(deprecated) and all intrinsics
+//
-// appear in the list are to disable
+// If disable_all is set, it accepts DisableIntrinsic and all intrinsic Ids
+// appear in the list are disabled. Arguments don't have +/- prefix. eg.
+// DisableIntrinsic=_id1,_id2,_id3,...
 class ControlIntrinsicIter {
  private:
   bool _enabled;
@@ -188,6 +190,39 @@ class ControlIntrinsicIter {
   ControlIntrinsicIter& operator++();
 };
 
+class ControlIntrinsicValidator {
+ private:
+  bool _valid;
+  char* _bad;
+
+ public:
+  ControlIntrinsicValidator(ccstrlist option, bool disable_all) : _valid(true), _bad(nullptr) {
+    for (ControlIntrinsicIter iter(option, disable_all); *iter != NULL && _valid; ++iter) {
+      if (vmIntrinsics::_none == vmIntrinsics::find_id(*iter)) {
+        const size_t len = MIN2<size_t>(strlen(*iter), 63) + 1;  // cap len to a value we know is enough for all intrinsic names
+        _bad = NEW_C_HEAP_ARRAY(char, len, mtCompiler);
+        // strncpy always writes len characters. If the source string is shorter, the function fills the remaining bytes with NULLs.
+        strncpy(_bad, *iter, len);
+        _valid = false;
+      }
+    }
+  }
+
+  ~ControlIntrinsicValidator() {
+    if (_bad != NULL) {
+      FREE_C_HEAP_ARRAY(char, _bad);
+    }
+  }
+
+  bool is_valid() const {
+    return _valid;
+  }
+
+  const char* what() const {
+    return _bad;
+  }
+};
+
 class CompilerDirectives : public CHeapObj<mtCompiler> {
 private:
   CompilerDirectives* _next;

src/hotspot/share/compiler/compilerOracle.cpp

@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "jvm.h"
 #include "classfile/symbolTable.hpp"
+#include "compiler/compilerDirectives.hpp"
 #include "compiler/compilerOracle.hpp"
 #include "compiler/methodMatcher.hpp"
 #include "memory/allocation.inline.hpp"
@@ -300,7 +301,7 @@ static void register_command(TypedMethodOptionMatcher* matcher,
     any_set = true;
   }
   if (!CompilerOracle::be_quiet()) {
-    // Print out the succesful registration of a comile command
+    // Print out the successful registration of a compile command
     ttyLocker ttyl;
     tty->print("CompileCommand: %s ", option2name(option));
     matcher->print();
@@ -588,6 +589,15 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
         next_value += bytes_read;
         end_value = next_value-1;
       }
+
+      if (option == CompileCommand::ControlIntrinsic || option == CompileCommand::DisableIntrinsic) {
+        ControlIntrinsicValidator validator(value, (option == CompileCommand::DisableIntrinsic));
+
+        if (!validator.is_valid()) {
+          jio_snprintf(errorbuf, buf_size, "Unrecognized intrinsic detected in %s: %s", option2name(option), validator.what());
+        }
+      }
+
       register_command(matcher, option, (ccstr) value);
       return;
     } else {

src/hotspot/share/compiler/directivesParser.cpp

@@ -319,6 +319,22 @@ bool DirectivesParser::set_option_flag(JSON_TYPE t, JSON_VAL* v, const key* opti
         strncpy(s, v->str.start, v->str.length + 1);
         s[v->str.length] = '\0';
         (set->*test)((void *)&s);
+
+        if (strncmp(option_key->name, "ControlIntrinsic", 16) == 0) {
+          ControlIntrinsicValidator validator(s, false/*disabled_all*/);
+
+          if (!validator.is_valid()) {
+            error(VALUE_ERROR, "Unrecognized intrinsic detected in ControlIntrinsic: %s", validator.what());
+            return false;
+          }
+        } else if (strncmp(option_key->name, "DisableIntrinsic", 16) == 0) {
+          ControlIntrinsicValidator validator(s, true/*disabled_all*/);
+
+          if (!validator.is_valid()) {
+            error(VALUE_ERROR, "Unrecognized intrinsic detected in DisableIntrinsic: %s", validator.what());
+            return false;
+          }
+        }
       }
       break;
 

src/hotspot/share/compiler/methodMatcher.cpp

@@ -359,7 +359,7 @@ void MethodMatcher::print_base(outputStream* st) {
 
 BasicMatcher* BasicMatcher::parse_method_pattern(char* line, const char*& error_msg, bool expect_trailing_chars) {
   assert(error_msg == NULL, "Don't call here with error_msg already set");
-  BasicMatcher *bm = new BasicMatcher();
+  BasicMatcher* bm = new BasicMatcher();
   MethodMatcher::parse_method_pattern(line, error_msg, bm);
   if (error_msg != NULL) {
     delete bm;

src/hotspot/share/runtime/flags/jvmFlagAccess.cpp

@@ -357,6 +357,12 @@ JVMFlag::Error JVMFlagAccess::check_range(const JVMFlag* flag, bool verbose) {
 }
 
 JVMFlag::Error JVMFlagAccess::check_constraint(const JVMFlag* flag, void * func, bool verbose) {
+  const int type_enum = flag->type();
+  if (type_enum == JVMFlag::TYPE_ccstr || type_enum == JVMFlag::TYPE_ccstrlist) {
+    // ccstr and ccstrlist are the same type.
+    return ((JVMFlagConstraintFunc_ccstr)func)(flag->get_ccstr(), verbose);
+  }
+
   return access_impl(flag)->check_constraint(flag, func, verbose);
 }
 

src/hotspot/share/runtime/flags/jvmFlagConstraintsCompiler.cpp

@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "code/relocInfo.hpp"
 #include "compiler/compilerDefinitions.hpp"
+#include "compiler/compilerDirectives.hpp"
 #include "oops/metadata.hpp"
 #include "runtime/os.hpp"
 #include "interpreter/invocationCounter.hpp"
@@ -413,3 +414,27 @@ JVMFlag::Error LoopStripMiningIterConstraintFunc(uintx value, bool verbose) {
   return JVMFlag::SUCCESS;
 }
 #endif // COMPILER2
+
+JVMFlag::Error DisableIntrinsicConstraintFunc(ccstrlist value, bool verbose) {
+  ControlIntrinsicValidator validator(value, true/*disabled_all*/);
+  if (!validator.is_valid()) {
+    JVMFlag::printError(verbose,
+                        "Unrecognized intrinsic detected in DisableIntrinsic: %s\n",
+                        validator.what());
+    return JVMFlag::VIOLATES_CONSTRAINT;
+  }
+
+  return JVMFlag::SUCCESS;
+}
+
+JVMFlag::Error ControlIntrinsicConstraintFunc(ccstrlist value, bool verbose) {
+  ControlIntrinsicValidator validator(value, false/*disabled_all*/);
+  if (!validator.is_valid()) {
+    JVMFlag::printError(verbose,
+                        "Unrecognized intrinsic detected in ControlIntrinsic: %s\n",
+                        validator.what());
+    return JVMFlag::VIOLATES_CONSTRAINT;
+  }
+
+  return JVMFlag::SUCCESS;
+}

src/hotspot/share/runtime/flags/jvmFlagConstraintsCompiler.hpp

@@ -51,6 +51,8 @@
   f(uintx, TypeProfileLevelConstraintFunc)              \
   f(intx,  InitArrayShortSizeConstraintFunc)            \
   f(int ,  RTMTotalCountIncrRateConstraintFunc)         \
+  f(ccstrlist, DisableIntrinsicConstraintFunc)          \
+  f(ccstrlist, ControlIntrinsicConstraintFunc)          \
 COMPILER2_PRESENT(                                      \
   f(intx,  InteriorEntryAlignmentConstraintFunc)        \
   f(intx,  NodeLimitFudgeFactorConstraintFunc)          \

src/hotspot/share/runtime/flags/jvmFlagLimit.hpp

@@ -48,6 +48,7 @@ typedef JVMFlag::Error (*JVMFlagConstraintFunc_uintx)(uintx value, bool verbose)
 typedef JVMFlag::Error (*JVMFlagConstraintFunc_uint64_t)(uint64_t value, bool verbose);
 typedef JVMFlag::Error (*JVMFlagConstraintFunc_size_t)(size_t value, bool verbose);
 typedef JVMFlag::Error (*JVMFlagConstraintFunc_double)(double value, bool verbose);
+typedef JVMFlag::Error (*JVMFlagConstraintFunc_ccstr)(ccstr value, bool verbose);
 
 // A JVMFlagLimit is created for each JVMFlag that has a range() and/or constraint() in its declaration in
 // the globals_xxx.hpp file.

src/hotspot/share/runtime/globals.hpp

@@ -365,10 +365,12 @@ const intx ObjectAlignmentInBytes = 8;
                                                                             \
   product(ccstrlist, DisableIntrinsic, "", DIAGNOSTIC,                      \
          "do not expand intrinsics whose (internal) names appear here")     \
+         constraint(DisableIntrinsicConstraintFunc,AfterErgo)               \
                                                                             \
   product(ccstrlist, ControlIntrinsic, "", DIAGNOSTIC,                      \
          "Control intrinsics using a list of +/- (internal) names, "        \
          "separated by commas")                                             \
+         constraint(ControlIntrinsicConstraintFunc,AfterErgo)               \
                                                                             \
   develop(bool, TraceCallFixup, false,                                      \
           "Trace all call fixups")                                          \

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

@@ -0,0 +1,56 @@
+/*
+ * 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 8247732
+ * @summary Tests CompileCommand=ControlIntrinsic,*.*,+_id
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ *
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * @run driver compiler.compilercontrol.commands.ControlIntrinsicTest
+ */
+
+package compiler.compilercontrol.commands;
+
+import compiler.compilercontrol.share.IntrinsicCommand;
+import compiler.compilercontrol.share.IntrinsicCommand.IntrinsicId;
+import compiler.compilercontrol.share.scenario.Scenario;
+
+public class ControlIntrinsicTest {
+    public static void main(String[] args) {
+        IntrinsicId ids[] = new IntrinsicId[3];
+
+        ids[0] = new IntrinsicId("_newArray", true);
+        ids[1] = new IntrinsicId("_minF", false);
+        ids[2] = new IntrinsicId("_copyOf", true);
+        new IntrinsicCommand(Scenario.Type.OPTION, ids).test();
+
+        // even though intrinsic ids are invalid, hotspot returns 0
+        ids[0] = new IntrinsicId("brokenIntrinsic", true);
+        ids[1] = new IntrinsicId("invalidIntrinsic", false);
+        new IntrinsicCommand(Scenario.Type.OPTION, ids).test();
+    }
+}

test/hotspot/jtreg/compiler/compilercontrol/control_off.txt

@@ -2,6 +2,5 @@
   {    
     match: "*.helper",
     PrintAssembly: false,
-    DisableIntrinsic:"x"
   }
 ]

test/hotspot/jtreg/compiler/compilercontrol/directives/ControlIntrinsicTest.java

@@ -0,0 +1,56 @@
+/*
+ * 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 8247732
+ * @summary Tests  -XX:CompilerDirectivesFile=directives.json
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ *
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * @run driver compiler.compilercontrol.directives.ControlIntrinsicTest
+ */
+
+package compiler.compilercontrol.directives;
+
+import compiler.compilercontrol.share.IntrinsicCommand;
+import compiler.compilercontrol.share.IntrinsicCommand.IntrinsicId;
+import compiler.compilercontrol.share.scenario.Scenario;
+
+public class ControlIntrinsicTest {
+    public static void main(String[] args) {
+        IntrinsicId ids[] = new IntrinsicId[3];
+
+        ids[0] = new IntrinsicId("_newArray", true);
+        ids[1] = new IntrinsicId("_minF", false);
+        ids[2] = new IntrinsicId("_copyOf", true);
+        new IntrinsicCommand(Scenario.Type.DIRECTIVE, ids).test();
+
+        // invalid compileCommands, hotspot exits with non-zero retval
+        ids[0] = new IntrinsicId("brokenIntrinsic", true);
+        ids[1] = new IntrinsicId("invalidIntrinsic", false);
+        new IntrinsicCommand(Scenario.Type.DIRECTIVE, ids).test();
+    }
+}

test/hotspot/jtreg/compiler/compilercontrol/jcmd/ControlIntrinsicTest.java

@@ -0,0 +1,56 @@
+/*
+ * 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 8247732
+ * @summary Test ControlIntrinsic via jcmd
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib /
+ *
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * @run driver compiler.compilercontrol.jcmd.ControlIntrinsicTest
+ */
+
+package compiler.compilercontrol.jcmd;
+
+import compiler.compilercontrol.share.IntrinsicCommand;
+import compiler.compilercontrol.share.IntrinsicCommand.IntrinsicId;
+import compiler.compilercontrol.share.scenario.Scenario;
+
+public class ControlIntrinsicTest {
+    public static void main(String[] args) {
+        IntrinsicId ids[] = new IntrinsicId[3];
+
+        ids[0] = new IntrinsicId("_newArray", true);
+        ids[1] = new IntrinsicId("_minF", false);
+        ids[2] = new IntrinsicId("_copyOf", true);
+        new IntrinsicCommand(Scenario.Type.JCMD, ids).test();
+
+        // will get error message but jcmd process still return 0
+        ids[0] = new IntrinsicId("brokenIntrinsic", true);
+        ids[1] = new IntrinsicId("invalidIntrinsic", false);
+        new IntrinsicCommand(Scenario.Type.JCMD, ids).test();
+    }
+}

test/hotspot/jtreg/compiler/compilercontrol/jcmd/PrintDirectivesTest.java

@@ -48,6 +48,8 @@ import jdk.test.lib.Utils;
 
 import java.lang.reflect.Executable;
 
+import static compiler.compilercontrol.share.IntrinsicCommand.VALID_INTRINSIC_SAMPLES;
+
 public class PrintDirectivesTest extends AbstractTestBase {
     private static final int AMOUNT = Utils.getRandomInstance().nextInt(
             Integer.getInteger("compiler.compilercontrol.jcmd."
@@ -63,6 +65,8 @@ public class PrintDirectivesTest extends AbstractTestBase {
         Scenario.Builder builder = Scenario.getBuilder();
         // Add some commands with directives file
         for (int i = 0; i < AMOUNT; i++) {
+            String argument = null;
+
             Executable exec = Utils.getRandomElement(METHODS).first;
             MethodDescriptor methodDescriptor = getValidMethodDescriptor(exec);
             Command command = cmdGen.generateCommand();
@@ -70,9 +74,12 @@ public class PrintDirectivesTest extends AbstractTestBase {
                 // skip invalid command
                 command = Command.COMPILEONLY;
             }
+            if (command == Command.INTRINSIC) {
+                argument = Utils.getRandomElement(VALID_INTRINSIC_SAMPLES);
+            }
             CompileCommand compileCommand = new CompileCommand(command,
                     methodDescriptor, cmdGen.generateCompiler(),
-                    Scenario.Type.DIRECTIVE);
+                    Scenario.Type.DIRECTIVE, argument);
             builder.add(compileCommand);
         }
         // print all directives

test/hotspot/jtreg/compiler/compilercontrol/parser/HugeDirectiveUtil.java

@@ -36,6 +36,8 @@ import java.util.List;
 import java.util.Random;
 import java.util.stream.Collectors;
 
+import static compiler.compilercontrol.share.IntrinsicCommand.VALID_INTRINSIC_SAMPLES;
+
 /**
  * Creates a huge directive file
  */
@@ -82,8 +84,11 @@ public final class HugeDirectiveUtil {
             file.emitCompiler(Utils.getRandomElement(
                     Scenario.Compiler.values()));
             // add option inside the compiler block
-            file.option(Utils.getRandomElement(DirectiveWriter.Option.values()),
+            DirectiveWriter.Option option = Utils.getRandomElement(DirectiveWriter.Option.values());
-                    random.nextBoolean());
+            file.option(option,
+                    option != DirectiveWriter.Option.INTRINSIC
+                    ? random.nextBoolean()
+                    : "\"" + Utils.getRandomElement(VALID_INTRINSIC_SAMPLES) + "\"");
             file.end(); // ends compiler block
 
             // add standalone option, enable can't be used standalone

test/hotspot/jtreg/compiler/compilercontrol/share/IntrinsicCommand.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2015, 2020, Oracle and/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.
+ */
+
+package compiler.compilercontrol.share;
+
+import compiler.compilercontrol.share.method.MethodDescriptor;
+import compiler.compilercontrol.share.scenario.Command;
+import compiler.compilercontrol.share.scenario.CommandGenerator;
+import compiler.compilercontrol.share.scenario.CompileCommand;
+import compiler.compilercontrol.share.scenario.Scenario;
+import jdk.test.lib.Utils;
+
+import java.lang.reflect.Executable;
+import java.util.Arrays;
+import java.util.stream.Collectors;
+
+public class IntrinsicCommand extends AbstractTestBase {
+    public static String[] VALID_INTRINSIC_SAMPLES = {"+_fabs", "-_maxF", "+_newArray", "-_isDigit", "+_putInt"};
+    public static String[] INVALID_INTRINSIC_SAMPLES = {"+fabs", "-maxF"};
+
+    public static class IntrinsicId {
+        private String id;
+        private boolean enable;
+
+        public IntrinsicId(String id, boolean enable) {
+            this.id = id;
+            this.enable = enable;
+        }
+
+        @Override
+        public String toString() {
+            return (enable ? "+" : "-")  + id;
+        }
+    }
+
+    private final Command command;
+    private final Scenario.Type type;
+    private String intrinsic_ids;
+
+    public IntrinsicCommand(Scenario.Type type, IntrinsicId[] intrinsic_ids) {
+        this.command = Command.INTRINSIC;
+        this.type = type;
+        this.intrinsic_ids = Arrays.stream(intrinsic_ids).map(id -> id.toString())
+                                                         .collect(Collectors.joining(","));
+    }
+
+    @Override
+    public void test() {
+        Scenario.Builder builder = Scenario.getBuilder();
+        Executable exec = Utils.getRandomElement(METHODS).first;
+        MethodDescriptor md = getValidMethodDescriptor(exec);
+        CommandGenerator cmdGen = new CommandGenerator();
+
+        CompileCommand compileCommand = cmdGen.generateCompileCommand(command,
+                md, type, intrinsic_ids);
+        builder.add(compileCommand);
+        Scenario scenario = builder.build();
+        scenario.execute();
+    }
+}

test/hotspot/jtreg/compiler/compilercontrol/share/MultiCommand.java

@@ -34,6 +34,9 @@ import java.lang.reflect.Executable;
 import java.util.ArrayList;
 import java.util.List;
 
+import static compiler.compilercontrol.share.IntrinsicCommand.VALID_INTRINSIC_SAMPLES;
+import static compiler.compilercontrol.share.IntrinsicCommand.INVALID_INTRINSIC_SAMPLES;
+
 public class MultiCommand extends AbstractTestBase {
     private final List<CompileCommand> testCases;
 
@@ -51,11 +54,22 @@ public class MultiCommand extends AbstractTestBase {
         CommandGenerator cmdGen = new CommandGenerator();
         List<Command> commands = cmdGen.generateCommands();
         List<CompileCommand> testCases = new ArrayList<>();
+
         for (Command cmd : commands) {
+            String argument = null;
+
             if (validOnly && cmd == Command.NONEXISTENT) {
                 // replace with a valid command
                 cmd = Command.EXCLUDE;
             }
+            if (cmd == Command.INTRINSIC) {
+                if (validOnly) {
+                    argument = Utils.getRandomElement(VALID_INTRINSIC_SAMPLES);
+                } else {
+                    argument = Utils.getRandomElement(INVALID_INTRINSIC_SAMPLES);
+                }
+            }
+
             Executable exec = Utils.getRandomElement(METHODS).first;
             MethodDescriptor md;
             if (validOnly) {
@@ -63,7 +77,12 @@ public class MultiCommand extends AbstractTestBase {
             } else {
                 md = AbstractTestBase.METHOD_GEN.generateRandomDescriptor(exec);
             }
-            CompileCommand cc = cmdGen.generateCompileCommand(cmd, md, null);
+            CompileCommand cc;
+            if (cmd == Command.INTRINSIC) {
+                cc = cmdGen.generateCompileCommand(cmd, md, null, argument);
+            } else {
+                cc = cmdGen.generateCompileCommand(cmd, md, null);
+            }
             testCases.add(cc);
         }
         return new MultiCommand(testCases);

test/hotspot/jtreg/compiler/compilercontrol/share/processors/CommandProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, Oracle and/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
@@ -23,6 +23,7 @@
 
 package compiler.compilercontrol.share.processors;
 
+import compiler.compilercontrol.share.scenario.Command;
 import compiler.compilercontrol.share.scenario.CompileCommand;
 import jdk.test.lib.Asserts;
 import jdk.test.lib.process.OutputAnalyzer;
@@ -38,6 +39,8 @@ public class CommandProcessor implements Consumer<OutputAnalyzer> {
     private static final String INVALID_COMMAND_MSG = "CompileCommand: "
             + "\\b(unrecognized command|Bad pattern|"
             + "An error occurred during parsing)\\b";
+    private static final String WARNING_COMMAND_MSG = "CompileCommand: An error occurred during parsing";
+
     private final Iterator<CompileCommand> nonQuietedIterator;
     private final Iterator<CompileCommand> quietedIterator;
 
@@ -60,11 +63,16 @@ public class CommandProcessor implements Consumer<OutputAnalyzer> {
     }
 
     private void check(String input) {
+        // -XX:CompileCommand(File) ignores invalid items
+        if (input.equals(WARNING_COMMAND_MSG)) {
+            return;
+        }
+
         if (nonQuietedIterator.hasNext()) {
             CompileCommand command = nonQuietedIterator.next();
             if (command.isValid()) {
                 Asserts.assertTrue(input.contains(getOutputString(command)),
-                        getOutputString(command) + "missing in output");
+                        getOutputString(command) + " missing in output");
             } else {
                 Asserts.assertTrue(input.matches(INVALID_COMMAND_MSG),
                         "Error message missing for: " + getOutputString(
@@ -82,9 +90,35 @@ public class CommandProcessor implements Consumer<OutputAnalyzer> {
         }
     }
 
-    private String getOutputString(CompileCommand command) {
+    // the output here must match hotspot compilerOracle.cpp::register_command
-        return "CompileCommand: "
+    //    tty->print("CompileCommand: %s ", option2name(option));
-                + command.command.name + " "
+    //    matcher->print();
-                + command.methodDescriptor.getCanonicalString();
+    private String getOutputString(CompileCommand cc) {
+        StringBuilder sb = new StringBuilder("CompileCommand: ");
+        // ControlIntrinsic output example:
+        // CompileCommand: ControlIntrinsic *Klass.-()V const char* ControlIntrinsic = '+_newArray -_minF +_copyOf'
+        sb.append(cc.command.name);
+        sb.append(" ");
+        sb.append(cc.methodDescriptor.getCanonicalString());
+        if (cc.command == Command.INTRINSIC) {
+            sb.append(" const char* ");
+            sb.append("ControlIntrinsic = '");
+
+            if (cc.argument != null) {
+                boolean initial = true;
+                for (String id: cc.argument.split(",")) {
+                    if(!initial) {
+                        sb.append(" ");
+                    }
+                    else {
+                        initial = false;
+                    }
+                    sb.append(id);
+                }
+            }
+            sb.append("'");
+        }
+
+        return sb.toString();
     }
 }

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/AbstractCommandBuilder.java

@@ -63,8 +63,11 @@ public abstract class AbstractCommandBuilder
 
     @Override
     public boolean isValid() {
-        // -XX:CompileCommand(File) ignores invalid items
+        boolean isValid = true;
-        return true;
+        for (CompileCommand cmd : compileCommands) {
+            isValid &= cmd.isValid();
+        }
+        return isValid;
     }
 
     /*

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/Command.java

@@ -39,6 +39,7 @@ public enum Command {
             "-XX:+LogCompilation", "-XX:LogFile=" + LogProcessor.LOG_FILE),
     PRINT("print", ""),
     QUIET("quiet", ""),
+    INTRINSIC("ControlIntrinsic", ""),
     NONEXISTENT("nonexistent", ""); // wrong command for a negative case
 
     /**

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/CommandFileBuilder.java

@@ -27,6 +27,7 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.Function;
 
 /**
  * Creates CompileCommandFile from the given array of commands
@@ -40,11 +41,21 @@ public class CommandFileBuilder extends AbstractCommandBuilder {
 
     @Override
     public List<String> getOptions() {
+        Function<CompileCommand, String> mapper = cc -> {
+            StringBuilder sb = new StringBuilder(cc.command.name);
+            sb.append(" ");
+            sb.append(cc.methodDescriptor.getString());
+            if (cc.argument != null) {
+                sb.append(" ");
+                sb.append(cc.argument);
+            }
+            return sb.toString();
+        };
+
         // Create CommandFile
         try (PrintWriter pw = new PrintWriter(fileName)) {
             compileCommands.stream()
-                    .map(cc -> cc.command.name + " "
+                    .map(mapper)
-                            + cc.methodDescriptor.getString())
                     .forEach(pw::println);
             if (pw.checkError()) {
                 throw new Error("TESTBUG: write error");

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/CommandGenerator.java

@@ -24,6 +24,7 @@
 package compiler.compilercontrol.share.scenario;
 
 import compiler.compilercontrol.share.method.MethodDescriptor;
+import jdk.test.lib.Asserts;
 import jdk.test.lib.Utils;
 
 import java.util.List;
@@ -88,6 +89,15 @@ public class CommandGenerator {
         return type.createCompileCommand(command, md, generateCompiler());
     }
 
+    public CompileCommand generateCompileCommand(Command command,
+            MethodDescriptor md, Scenario.Type type, String argument) {
+        if (type == null) {
+            type = Utils.getRandomElement(Scenario.Type.values());
+        }
+        return type.createCompileCommand(command, md, generateCompiler(), argument);
+    }
+
+
     /**
      * Generates type of compiler that should be used for the command, or null
      * if any or all compilers should be used

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/CommandOptionsBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, Oracle and/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
@@ -23,22 +23,32 @@
 
 package compiler.compilercontrol.share.scenario;
 
-import compiler.compilercontrol.share.method.MethodDescriptor;
+import java.util.function.Function;
-
 import java.util.List;
 import java.util.stream.Collectors;
 
+import static compiler.compilercontrol.share.method.MethodDescriptor.Separator.COMMA;
 /**
  * Creates VM options by adding CompileCommand prefix to commands
  */
 public class CommandOptionsBuilder extends AbstractCommandBuilder {
     @Override
     public List<String> getOptions() {
-        return compileCommands.stream()
+        Function<CompileCommand, String> mapper = cc -> {
-                .map(cc -> "-XX:CompileCommand="
+            StringBuilder sb = new StringBuilder("-XX:CompileCommand=");
-                        + cc.command.name
+            sb.append(cc.command.name);
-                        + MethodDescriptor.Separator.COMMA.symbol
+            sb.append(COMMA.symbol);
-                        + cc.methodDescriptor.getString())
+            sb.append(cc.methodDescriptor.getString());
-                .collect(Collectors.toList());
+            if (cc.argument != null) {
+                sb.append(COMMA.symbol);
+                sb.append(cc.argument);
+            }
+            return sb.toString();
+        };
+
+        List<String> options = compileCommands.stream()
+                .map(mapper).collect(Collectors.toList());
+
+        return options;
     }
 }

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/CompileCommand.java

@@ -33,6 +33,7 @@ public class CompileCommand {
     public final MethodDescriptor methodDescriptor;
     public final Scenario.Compiler compiler;
     public final Scenario.Type type;
+    public final String argument;
 
     public CompileCommand(Command command,
                           MethodDescriptor methodDescriptor,
@@ -42,8 +43,22 @@ public class CompileCommand {
         this.methodDescriptor = methodDescriptor;
         this.compiler = compiler;
         this.type = type;
+        this.argument = null;
     }
 
+    public CompileCommand(Command command,
+                          MethodDescriptor methodDescriptor,
+                          Scenario.Compiler compiler,
+                          Scenario.Type type,
+                          String argument) {
+        this.command = command;
+        this.methodDescriptor = methodDescriptor;
+        this.compiler = compiler;
+        this.type = type;
+        this.argument = argument;
+    }
+
+
     /**
      * Shows that this compile command is valid
      *
@@ -53,6 +68,24 @@ public class CompileCommand {
         if (command == Command.NONEXISTENT) {
             return false;
         }
+        // -XX:CompileCommand(File) ignores invalid items
+        // Invalid intrinsic ids in CompilerDirectivesFile will force hotspot to exit with non-zero value.
+        if (command == Command.INTRINSIC && type == Scenario.Type.DIRECTIVE) {
+            if (argument != null) {
+                String[] ids = argument.split(",");
+                for (String id : ids) {
+                    char ch = id.charAt(0);
+
+                    // Not a strict check.
+                    // a valid ControlIntrinsic argument is separated by ",", each one starts with '+' or '-'.
+                    // intrinsicId starts with '_'
+                    if ((ch != '+' && ch != '-') || id.charAt(1) != '_') {
+                      return false;
+                    }
+                }
+            }
+        }
+
         return methodDescriptor.isValid();
     }
 

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/DirectiveBuilder.java

@@ -212,6 +212,9 @@ public class DirectiveBuilder implements StateBuilder<CompileCommand> {
             case PRINT:
                 dirFile.option(DirectiveWriter.Option.PRINT_ASSEMBLY, true);
                 break;
+            case INTRINSIC:
+                dirFile.option(DirectiveWriter.Option.INTRINSIC, "\"" + cmd.argument + "\"");
+                break;
             case NONEXISTENT:
                 dirFile.write(JSONFile.Element.PAIR, command.name);
                 dirFile.write(JSONFile.Element.OBJECT);

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/DirectiveWriter.java

@@ -192,7 +192,8 @@ public class DirectiveWriter implements AutoCloseable {
         PRINT_ASSEMBLY("PrintAssembly"),
         LOG("Log"),
         EXCLUDE("Exclude"),
-        ENABLE("Enable");
+        ENABLE("Enable"),
+        INTRINSIC("ControlIntrinsic");
 
         public final String string;
 

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/Executor.java

@@ -85,7 +85,7 @@ public class Executor {
         vmOptions.add(execClass);
         OutputAnalyzer output;
         try (ServerSocket serverSocket = new ServerSocket(0)) {
-            if (isValid) {
+            {
                 // Get port test VM will connect to
                 int port = serverSocket.getLocalPort();
                 if (port == -1) {
@@ -150,9 +150,13 @@ public class Executor {
                 pw.println();
             }
         } catch (IOException e) {
+            // hotspot process may exit because of invalid directives,
+            // suppress the IOException of closed socket.
+            if (!e.getMessage().equals("Socket closed")) {
               throw new Error("Failed to write data: " + e.getMessage(), e);
             }
         }
+    }
 
     // Executes all diagnostic commands
     protected OutputAnalyzer[] executeJCMD(int pid) {

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/JcmdCommand.java

@@ -37,6 +37,17 @@ public class JcmdCommand extends CompileCommand {
         this.jcmdType = jcmdType;
     }
 
+    public JcmdCommand(Command command,
+                       MethodDescriptor methodDescriptor,
+                       Scenario.Compiler compiler,
+                       Scenario.Type type,
+                       Scenario.JcmdType jcmdType,
+                       String argument) {
+        super(command, methodDescriptor, compiler, type, argument);
+        this.jcmdType = jcmdType;
+    }
+
+
     /**
      * Enchances parent's class method with the the JCMDtype printing:
      * {@code ... JCMDType: <jcmd_type>}

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/Scenario.java

@@ -110,18 +110,23 @@ public final class Scenario {
     public void execute() {
         List<OutputAnalyzer> outputList = executor.execute();
         // The first one contains output from the test VM
-        OutputAnalyzer mainOuput = outputList.get(0);
+        OutputAnalyzer mainOutput = outputList.get(0);
         if (isValid) {
-            mainOuput.shouldHaveExitValue(0);
+            mainOutput.shouldHaveExitValue(0);
-            processors.forEach(processor -> processor.accept(mainOuput));
+            processors.forEach(processor -> processor.accept(mainOutput));
             // only the last output contains directives got from print command
             List<OutputAnalyzer> last = new ArrayList<>();
             last.add(outputList.get(outputList.size() - 1));
             jcmdProcessor.accept(last);
         } else {
-            Asserts.assertNE(mainOuput.getExitValue(), 0, "VM should exit with "
+            // two cases for invalid inputs.
+            if (mainOutput.getExitValue() == 0) {
+                mainOutput.shouldContain("CompileCommand: An error occurred during parsing");
+            } else {
+                Asserts.assertNE(mainOutput.getExitValue(), 0, "VM should exit with "
                         + "error for incorrect directives");
-            mainOuput.shouldContain("Parsing of compiler directives failed");
+                mainOutput.shouldContain("Parsing of compiler directives failed");
+            }
         }
     }
 
@@ -164,8 +169,8 @@ public final class Scenario {
      * Type of the compile command
      */
     public static enum Type {
-        OPTION(""),
+        OPTION(""),               // CompilerOracle: -XX:CompileCommand=
-        FILE("command_file"),
+        FILE("command_file"),     // CompilerOracle: -XX:CompileCommandFile=
         DIRECTIVE("directives.json"),
         JCMD("jcmd_directives.json") {
             @Override
@@ -174,6 +179,13 @@ public final class Scenario {
                 return new JcmdCommand(command, md, compiler, this,
                         JcmdType.ADD);
             }
+
+            @Override
+            public CompileCommand createCompileCommand(Command command,
+                    MethodDescriptor md, Compiler compiler, String argument) {
+                return new JcmdCommand(command, md, compiler, this,
+                        JcmdType.ADD, argument);
+            }
         };
 
         public final String fileName;
@@ -183,6 +195,11 @@ public final class Scenario {
             return new CompileCommand(command, md, compiler, this);
         }
 
+        public CompileCommand createCompileCommand(Command command,
+                MethodDescriptor md, Compiler compiler, String argument) {
+            return new CompileCommand(command, md, compiler, this, argument);
+        }
+
         private Type(String fileName) {
             this.fileName = fileName;
         }

test/hotspot/jtreg/compiler/compilercontrol/share/scenario/State.java

@@ -42,6 +42,7 @@ public class State {
     private Optional<Boolean> printAssembly = Optional.empty();
     private Optional<Boolean> printInline = Optional.empty();
     private Optional<Boolean> log = Optional.empty();
+    private Optional<String> controlIntrinsic = Optional.empty();
 
     public State() {
         Arrays.fill(compile, Optional.empty());
@@ -275,6 +276,12 @@ public class State {
         printInline = Optional.of(value);
     }
 
+    public void setControlIntrinsic(String argument) {
+        if (argument != null) {
+            controlIntrinsic = Optional.of(argument);
+        }
+    }
+
     public boolean isLog() {
         return log.orElse(false);
     }
@@ -314,6 +321,9 @@ public class State {
             case PRINT:
                 setPrintAssembly(true);
                 break;
+            case INTRINSIC:
+                setControlIntrinsic(compileCommand.argument);
+                break;
             case QUIET:
             case NONEXISTENT:
                 // doesn't apply the state
@@ -368,6 +378,9 @@ public class State {
         result.printInline = mergeOptional(high.printInline, low.printInline);
         // set LogCompilation
         result.log = mergeOptional(high.log, low.log);
+        // set controlIntrinsic
+        result.controlIntrinsic = mergeOptional(high.controlIntrinsic, low.controlIntrinsic);
+
         return result;
     }
 

test/hotspot/jtreg/compiler/intrinsics/IntrinsicDisabledTest.java

@@ -44,19 +44,19 @@
  *                   -XX:+WhiteBoxAPI
  *                   -XX:ControlIntrinsic=-_putCharVolatile,-_putInt
  *                   -XX:ControlIntrinsic=-_putIntVolatile
- *                   -XX:CompileCommand=option,jdk.internal.misc.Unsafe::putChar,ccstrlist,ControlIntrinsic,-_getCharVolatile,-_getInt
+ *                   -XX:CompileCommand=ControlIntrinsic,jdk.internal.misc.Unsafe::putChar,-_getCharVolatile,-_getInt
- *                   -XX:CompileCommand=option,jdk.internal.misc.Unsafe::putCharVolatile,ccstrlist,ControlIntrinsic,-_getIntVolatile
+ *                   -XX:CompileCommand=ControlIntrinsic,jdk.internal.misc.Unsafe::putCharVolatile,-_getIntVolatile
  *                   compiler.intrinsics.IntrinsicDisabledTest
  * @run main/othervm -Xbootclasspath/a:.
  *                   -XX:+UnlockDiagnosticVMOptions
  *                   -XX:+WhiteBoxAPI
- *                   -XX:ControlIntrinsic=+putIntVolatile,+_putCharVolatile,+_putInt
+ *                   -XX:ControlIntrinsic=+_putIntVolatile,+_putCharVolatile,+_putInt
  *                   -XX:DisableIntrinsic=_putCharVolatile,_putInt
  *                   -XX:DisableIntrinsic=_putIntVolatile
- *                   -XX:CompileCommand=option,jdk.internal.misc.Unsafe::putChar,ccstrlist,ControlIntrinsic,+_getCharVolatile,+_getInt
+ *                   -XX:CompileCommand=ControlIntrinsic,jdk.internal.misc.Unsafe::putChar,+_getCharVolatile,+_getInt
- *                   -XX:CompileCommand=option,jdk.internal.misc.Unsafe::putCharVolatile,ccstrlist,ControlIntrinsic,+_getIntVolatile
+ *                   -XX:CompileCommand=ControlIntrinsic,jdk.internal.misc.Unsafe::putCharVolatile,+_getIntVolatile
- *                   -XX:CompileCommand=option,jdk.internal.misc.Unsafe::putChar,ccstrlist,DisableIntrinsic,_getCharVolatile,_getInt
+ *                   -XX:CompileCommand=DisableIntrinsic,jdk.internal.misc.Unsafe::putChar,_getCharVolatile,_getInt
- *                   -XX:CompileCommand=option,jdk.internal.misc.Unsafe::putCharVolatile,ccstrlist,DisableIntrinsic,_getIntVolatile
+ *                   -XX:CompileCommand=DisableIntrinsic,jdk.internal.misc.Unsafe::putCharVolatile,_getIntVolatile
  *                   compiler.intrinsics.IntrinsicDisabledTest
 */