From 6c1f55b9bc82d93e83956d7ec228cb98214bea64 Mon Sep 17 00:00:00 2001
From: Marc Chevalier <marc.chevalier@oracle.com>
Date: Fri, 2 May 2025 16:48:25 +0200
Subject: [PATCH 1/7] Make some compile command locked by
 UnlockDiagnosticVMOptions

---
 src/hotspot/share/compiler/compilerOracle.cpp | 52 ++++++++++++++-----
 .../runtime/CommandLine/VMOptionWarning.java  | 18 +++++++
 2 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/src/hotspot/share/compiler/compilerOracle.cpp b/src/hotspot/share/compiler/compilerOracle.cpp
index e7ef97031914b..532f35fff867b 100644
--- a/src/hotspot/share/compiler/compilerOracle.cpp
+++ b/src/hotspot/share/compiler/compilerOracle.cpp
@@ -319,6 +319,8 @@ TypedMethodOptionMatcher* TypedMethodOptionMatcher::match(const methodHandle& me
 template<typename T>
 static void register_command(TypedMethodOptionMatcher* matcher,
                              CompileCommandEnum option,
+                             char* errorbuf,
+                             const int buf_size,
                              T value) {
   assert(matcher != option_list, "No circular lists please");
   if (option == CompileCommandEnum::Log && !LogCompilation) {
@@ -334,6 +336,16 @@ static void register_command(TypedMethodOptionMatcher* matcher,
     return;
   }
 
+  if (!UnlockDiagnosticVMOptions) {
+    const char* name = option2name(option);
+    JVMFlag* flag = JVMFlag::find_declared_flag(name);
+    if (flag != nullptr && flag->is_diagnostic()) {
+      jio_snprintf(errorbuf, buf_size, "VM option '%s' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.", name);
+      delete matcher;
+      return;
+    }
+  }
+
   matcher->init(option, option_list);
   matcher->set_value<T>(value);
   option_list = matcher;
@@ -734,7 +746,7 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
     if (success) {
       total_bytes_read += bytes_read;
       line += bytes_read;
-      register_command(matcher, option, value);
+      register_command(matcher, option, errorbuf, buf_size, value);
       return;
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
@@ -752,7 +764,7 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
     if (success) {
       total_bytes_read += bytes_read;
       line += bytes_read;
-      register_command(matcher, option, value);
+      register_command(matcher, option, errorbuf, buf_size, value);
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
     }
@@ -762,7 +774,7 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
     if (sscanf(line, "%255[_a-zA-Z0-9]%n", value, &bytes_read) == 1) {
       total_bytes_read += bytes_read;
       line += bytes_read;
-      register_command(matcher, option, (ccstr) value);
+      register_command(matcher, option, errorbuf, buf_size, (ccstr) value);
       return;
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
@@ -819,7 +831,7 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
         assert(false, "Ccstrlist type option missing validator");
       }
 
-      register_command(matcher, option, (ccstr) value);
+      register_command(matcher, option, errorbuf, buf_size, (ccstr) value);
       return;
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
@@ -829,19 +841,19 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
     if (*line == '\0') {
       // Short version of a CompileCommand sets a boolean Option to true
       // -XXCompileCommand=<Option>,<method pattern>
-      register_command(matcher, option, true);
+      register_command(matcher, option, errorbuf, buf_size,true);
       return;
     }
     if (sscanf(line, "%255[a-zA-Z]%n", value, &bytes_read) == 1) {
       if (strcasecmp(value, "true") == 0) {
         total_bytes_read += bytes_read;
         line += bytes_read;
-        register_command(matcher, option, true);
+        register_command(matcher, option, errorbuf, buf_size,true);
         return;
       } else if (strcasecmp(value, "false") == 0) {
         total_bytes_read += bytes_read;
         line += bytes_read;
-        register_command(matcher, option, false);
+        register_command(matcher, option, errorbuf, buf_size,false);
         return;
       } else {
         jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
@@ -858,7 +870,7 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
       jio_snprintf(value, sizeof(value), "%s.%s", buffer[0], buffer[1]);
       total_bytes_read += bytes_read;
       line += bytes_read;
-      register_command(matcher, option, atof(value));
+      register_command(matcher, option, errorbuf, buf_size, atof(value));
       return;
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
@@ -1011,7 +1023,11 @@ bool CompilerOracle::parse_from_line(char* line) {
           return false;
         }
         if (option2type(option) == OptionType::Bool) {
-          register_command(typed_matcher, option, true);
+          register_command(typed_matcher, option, error_buf, sizeof(error_buf), true);
+          if (*error_buf != '\0') {
+            print_parse_error(error_buf, original.get());
+            return false;
+          }
         } else {
           jio_snprintf(error_buf, sizeof(error_buf), "  Missing type '%s' before option '%s'",
                        optiontype2name(option2type(option)), option2name(option));
@@ -1041,11 +1057,19 @@ bool CompilerOracle::parse_from_line(char* line) {
     if (*line == '\0') {
       if (option2type(option) == OptionType::Bool) {
         // if this is a bool option this implies true
-        register_command(matcher, option, true);
+        register_command(matcher, option, error_buf, sizeof(error_buf), true);
+        if (*error_buf != '\0') {
+          print_parse_error(error_buf, original.get());
+          return false;
+        }
         return true;
       } else if (option == CompileCommandEnum::MemStat) {
         // MemStat default action is to collect data but to not print
-        register_command(matcher, option, (uintx)MemStatAction::collect);
+        register_command(matcher, option, error_buf, sizeof(error_buf), (uintx)MemStatAction::collect);
+        if (*error_buf != '\0') {
+          print_parse_error(error_buf, original.get());
+          return false;
+        }
         return true;
       } else {
         jio_snprintf(error_buf, sizeof(error_buf), "  Option '%s' is not followed by a value", option2name(option));
@@ -1161,8 +1185,10 @@ bool CompilerOracle::parse_compile_only(char* line) {
     if (method_pattern != nullptr) {
       TypedMethodOptionMatcher* matcher = TypedMethodOptionMatcher::parse_method_pattern(method_pattern, error_buf, sizeof(error_buf));
       if (matcher != nullptr) {
-        register_command(matcher, CompileCommandEnum::CompileOnly, true);
-        continue;
+        register_command(matcher, CompileCommandEnum::CompileOnly, error_buf, sizeof(error_buf), true);
+        if (*error_buf == '\0') {
+          continue;
+        }
       }
     }
     ttyLocker ttyl;
diff --git a/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java b/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
index 90b309069ac4f..3eeac795dc025 100644
--- a/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
+++ b/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
@@ -44,6 +44,17 @@
  * @run driver VMOptionWarning Diagnostic
  */
 
+/* @test VMCompileCommandWarningDiagnostic
+ * @bug 8027314
+ * @summary Warn if compile command that is an alias for a diagnostic vm option is used and -XX:+UnlockDiagnosticVMOptions isn't specified.
+ * @requires vm.flagless
+ * @requires ! vm.debug
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.management
+ * @run driver VMOptionWarning DiagnosticCompileCommand
+ */
+
 /* @test VMOptionWarningDevelop
  * @bug 8027314
  * @summary Warn if develop vm option is used with product version of VM.
@@ -82,6 +93,13 @@ public static void main(String[] args) throws Exception {
                 output.shouldContain("Error: VM option 'PrintInlining' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.");
                 break;
             }
+            case "DiagnosticCompileCommand": {
+                pb = ProcessTools.createLimitedTestJavaProcessBuilder("-XX:CompileCommand=PrintAssembly,*::*", "-version");
+                output = new OutputAnalyzer(pb.start());
+                output.shouldNotHaveExitValue(0);
+                output.shouldContain("Error: VM option 'PrintAssembly' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.");
+                break;
+            }
             case "Develop": {
                 pb = ProcessTools.createLimitedTestJavaProcessBuilder("-XX:+VerifyStack", "-version");
                 output = new OutputAnalyzer(pb.start());

From 8e6c9a98224d06328f689bfe10ec795648e7cd4a Mon Sep 17 00:00:00 2001
From: Marc Chevalier <marc.chevalier@oracle.com>
Date: Mon, 5 May 2025 09:10:40 +0200
Subject: [PATCH 2/7] Add some UnlockDiagnosticVMOptions

---
 .../compilercontrol/commands/OptionTest.java         | 12 ++++++------
 .../intrinsics/bigInteger/TestMultiplyToLen.java     |  1 +
 .../compiler/intrinsics/bigInteger/TestShift.java    |  2 ++
 .../intrinsics/bigInteger/TestSquareToLen.java       |  1 +
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/test/hotspot/jtreg/compiler/compilercontrol/commands/OptionTest.java b/test/hotspot/jtreg/compiler/compilercontrol/commands/OptionTest.java
index d00b0a57b3663..aeeceac4ee220 100644
--- a/test/hotspot/jtreg/compiler/compilercontrol/commands/OptionTest.java
+++ b/test/hotspot/jtreg/compiler/compilercontrol/commands/OptionTest.java
@@ -37,13 +37,13 @@
 
 public class OptionTest {
     public static void main(String[] args) throws Exception {
-        ProcessTools.executeTestJava("-XX:CompileCommand=option,package/class,ccstrlist,ControlIntrinsic,+_getClass", "-version")
+        ProcessTools.executeTestJava("-XX:+UnlockDiagnosticVMOptions", "-XX:CompileCommand=option,package/class,ccstrlist,ControlIntrinsic,+_getClass", "-version")
                     .shouldHaveExitValue(1)
                     .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.executeTestJava("-XX:CompileCommand=option,*,ccstrlist,ControlIntrinsic,+_getClass", "-version")
+        ProcessTools.executeTestJava("-XX:+UnlockDiagnosticVMOptions", "-XX:CompileCommand=option,*,ccstrlist,ControlIntrinsic,+_getClass", "-version")
                     .shouldHaveExitValue(1)
                     .shouldContain("CompileCommand: An error occurred during parsing")
                     .shouldContain("Error: Did not specify any method name")
@@ -51,24 +51,24 @@ public static void main(String[] args) throws Exception {
 
         // corner case:
         // ccstrlist could be a valid method name, so it is accepted in the well-formed case.
-        ProcessTools.executeTestJava("-XX:CompileCommand=option,*.ccstrlist,ccstrlist,ControlIntrinsic,+_getClass", "-version")
+        ProcessTools.executeTestJava("-XX:+UnlockDiagnosticVMOptions", "-XX:CompileCommand=option,*.ccstrlist,ccstrlist,ControlIntrinsic,+_getClass", "-version")
                     .shouldContain("CompileCommand: ControlIntrinsic *.ccstrlist const char* ControlIntrinsic")
                     .shouldHaveExitValue(0)
                     .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
 
 
-        ProcessTools.executeTestJava("-XX:CompileCommand=option,*.*,ccstrlist,ControlIntrinsic,+_getClass", "-version")
+        ProcessTools.executeTestJava("-XX:+UnlockDiagnosticVMOptions", "-XX:CompileCommand=option,*.*,ccstrlist,ControlIntrinsic,+_getClass", "-version")
                     .shouldContain("CompileCommand: ControlIntrinsic *.* const char* ControlIntrinsic")
                     .shouldHaveExitValue(0)
                     .shouldNotContain("# A fatal error has been detected by the Java Runtime Environment");
 
-        ProcessTools.executeTestJava("-XX:CompileCommand=option,class,PrintIntrinsics", "-version")
+        ProcessTools.executeTestJava("-XX:+UnlockDiagnosticVMOptions", "-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.executeTestJava("-XX:CompileCommand=option,class.PrintIntrinsics,PrintIntrinsics", "-version")
+        ProcessTools.executeTestJava("-XX:+UnlockDiagnosticVMOptions", "-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");
diff --git a/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestMultiplyToLen.java b/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestMultiplyToLen.java
index 439fb9bf17a43..25f0941ec683e 100644
--- a/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestMultiplyToLen.java
+++ b/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestMultiplyToLen.java
@@ -29,6 +29,7 @@
  *
  * @library /test/lib
  * @run main/othervm/timeout=600 -XX:-TieredCompilation -Xbatch
+ *      -XX:+UnlockDiagnosticVMOptions
  *      -XX:CompileCommand=exclude,compiler.intrinsics.bigInteger.TestMultiplyToLen::main
  *      -XX:CompileCommand=option,compiler.intrinsics.bigInteger.TestMultiplyToLen::base_multiply,ccstrlist,DisableIntrinsic,_multiplyToLen
  *      -XX:CompileCommand=option,java.math.BigInteger::multiply,ccstrlist,DisableIntrinsic,_multiplyToLen
diff --git a/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestShift.java b/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestShift.java
index fe27cebc8f552..61edf1e583b83 100644
--- a/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestShift.java
+++ b/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestShift.java
@@ -30,6 +30,7 @@
  * @requires vm.compiler2.enabled
  *
  * @run main/othervm/timeout=600 -XX:-TieredCompilation -Xbatch
+ *      -XX:+UnlockDiagnosticVMOptions
  *      -XX:CompileCommand=exclude,compiler.intrinsics.bigInteger.TestShift::main
  *      -XX:CompileCommand=option,compiler.intrinsics.bigInteger.TestShift::base_left_shift,ccstrlist,DisableIntrinsic,_bigIntegerLeftShiftWorker
  *      -XX:CompileCommand=option,compiler.intrinsics.bigInteger.TestShift::base_right_shift,ccstrlist,DisableIntrinsic,_bigIntegerRightShiftWorker
@@ -38,6 +39,7 @@
  *      compiler.intrinsics.bigInteger.TestShift
  *
  * @run main/othervm/timeout=600
+ *      -XX:+UnlockDiagnosticVMOptions
  *      -XX:CompileCommand=exclude,compiler.intrinsics.bigInteger.TestShift::main
  *      -XX:CompileCommand=option,compiler.intrinsics.bigInteger.TestShift::base_left_shift,ccstrlist,DisableIntrinsic,_bigIntegerLeftShiftWorker
  *      -XX:CompileCommand=option,compiler.intrinsics.bigInteger.TestShift::base_right_shift,ccstrlist,DisableIntrinsic,_bigIntegerRightShiftWorker
diff --git a/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestSquareToLen.java b/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestSquareToLen.java
index a6c8ead280101..b3d7518a20485 100644
--- a/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestSquareToLen.java
+++ b/test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestSquareToLen.java
@@ -29,6 +29,7 @@
  * @library /test/lib
  *
  * @run main/othervm/timeout=600 -XX:-TieredCompilation -Xbatch
+ *      -XX:+UnlockDiagnosticVMOptions
  *      -XX:CompileCommand=exclude,compiler.intrinsics.bigInteger.TestSquareToLen::main
  *      -XX:CompileCommand=option,compiler.intrinsics.bigInteger.TestSquareToLen::base_multiply,ccstrlist,DisableIntrinsic,_squareToLen
  *      -XX:CompileCommand=option,java.math.BigInteger::multiply,ccstrlist,DisableIntrinsic,_squareToLen

From f1837f63ae5fdf47862cdf8f9695e4722cca0640 Mon Sep 17 00:00:00 2001
From: Marc Chevalier <marc.chevalier@oracle.com>
Date: Fri, 16 May 2025 14:05:43 +0200
Subject: [PATCH 3/7] Address comments

---
 src/hotspot/share/compiler/compilerOracle.cpp               | 2 +-
 test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/hotspot/share/compiler/compilerOracle.cpp b/src/hotspot/share/compiler/compilerOracle.cpp
index 532f35fff867b..52beeab0bcb43 100644
--- a/src/hotspot/share/compiler/compilerOracle.cpp
+++ b/src/hotspot/share/compiler/compilerOracle.cpp
@@ -330,7 +330,7 @@ static void register_command(TypedMethodOptionMatcher* matcher,
   assert(CompilerOracle::option_matches_type(option, value), "Value must match option type");
 
   if (option == CompileCommandEnum::Blackhole && !UnlockExperimentalVMOptions) {
-    warning("Blackhole compile option is experimental and must be enabled via -XX:+UnlockExperimentalVMOptions");
+    jio_snprintf(errorbuf, buf_size, "Blackhole compile option is experimental and must be enabled via -XX:+UnlockExperimentalVMOptions");
     // Delete matcher as we don't keep it
     delete matcher;
     return;
diff --git a/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java b/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
index 3eeac795dc025..5cedca7d046a2 100644
--- a/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
+++ b/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
@@ -45,10 +45,10 @@
  */
 
 /* @test VMCompileCommandWarningDiagnostic
- * @bug 8027314
+ * @bug 8351958
  * @summary Warn if compile command that is an alias for a diagnostic vm option is used and -XX:+UnlockDiagnosticVMOptions isn't specified.
  * @requires vm.flagless
- * @requires ! vm.debug
+ * @requires !vm.debug
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *          java.management
@@ -94,7 +94,7 @@ public static void main(String[] args) throws Exception {
                 break;
             }
             case "DiagnosticCompileCommand": {
-                pb = ProcessTools.createLimitedTestJavaProcessBuilder("-XX:CompileCommand=PrintAssembly,*::*", "-version");
+                pb = ProcessTools.createLimitedTestJavaProcessBuilder("-XX:CompileCommand=PrintAssembly,MyClass::myMethod", "-version");
                 output = new OutputAnalyzer(pb.start());
                 output.shouldNotHaveExitValue(0);
                 output.shouldContain("Error: VM option 'PrintAssembly' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.");

From 670e401973d7e76d7ea3a5a353261c509cb07425 Mon Sep 17 00:00:00 2001
From: Marc Chevalier <marc.chevalier@oracle.com>
Date: Fri, 16 May 2025 15:08:28 +0200
Subject: [PATCH 4/7] For consistency

---
 test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java b/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
index 5cedca7d046a2..ccc998bb7e044 100644
--- a/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
+++ b/test/hotspot/jtreg/runtime/CommandLine/VMOptionWarning.java
@@ -26,7 +26,7 @@
  * @bug 8027314
  * @summary Warn if experimental vm option is used and -XX:+UnlockExperimentalVMOptions isn't specified.
  * @requires vm.flagless
- * @requires ! vm.opt.final.UnlockExperimentalVMOptions
+ * @requires !vm.opt.final.UnlockExperimentalVMOptions
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *          java.management
@@ -37,7 +37,7 @@
  * @bug 8027314
  * @summary Warn if diagnostic vm option is used and -XX:+UnlockDiagnosticVMOptions isn't specified.
  * @requires vm.flagless
- * @requires ! vm.debug
+ * @requires !vm.debug
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *          java.management
@@ -59,7 +59,7 @@
  * @bug 8027314
  * @summary Warn if develop vm option is used with product version of VM.
  * @requires vm.flagless
- * @requires ! vm.debug
+ * @requires !vm.debug
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *          java.management

From 159fa771c8696e17467dddc40c27ca8d65eaa936 Mon Sep 17 00:00:00 2001
From: Marc Chevalier <marc.chevalier@oracle.com>
Date: Mon, 19 May 2025 11:27:16 +0200
Subject: [PATCH 5/7] Re-use warning for blackhole

---
 src/hotspot/share/compiler/compilerOracle.cpp               | 2 +-
 .../compiler/blackhole/BlackholeExperimentalUnlockTest.java | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/hotspot/share/compiler/compilerOracle.cpp b/src/hotspot/share/compiler/compilerOracle.cpp
index 52beeab0bcb43..532f35fff867b 100644
--- a/src/hotspot/share/compiler/compilerOracle.cpp
+++ b/src/hotspot/share/compiler/compilerOracle.cpp
@@ -330,7 +330,7 @@ static void register_command(TypedMethodOptionMatcher* matcher,
   assert(CompilerOracle::option_matches_type(option, value), "Value must match option type");
 
   if (option == CompileCommandEnum::Blackhole && !UnlockExperimentalVMOptions) {
-    jio_snprintf(errorbuf, buf_size, "Blackhole compile option is experimental and must be enabled via -XX:+UnlockExperimentalVMOptions");
+    warning("Blackhole compile option is experimental and must be enabled via -XX:+UnlockExperimentalVMOptions");
     // Delete matcher as we don't keep it
     delete matcher;
     return;
diff --git a/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java b/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java
index 0a403a784506f..0d5132061a878 100644
--- a/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java
+++ b/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java
@@ -68,7 +68,7 @@ private static List<String> cmdline(String[] args) {
     public static void shouldFail(String... args) throws IOException {
         ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(cmdline(args));
         OutputAnalyzer output = new OutputAnalyzer(pb.start());
-        output.shouldHaveExitValue(0);
+        output.shouldHaveExitValue(1);
         output.shouldContain(MSG);
     }
 
@@ -103,12 +103,12 @@ public static void driver() throws IOException {
         );
 
         // Should be able to shun the warning
-        shouldPass(
+        shouldFail(
             "-XX:-PrintWarnings",
             "-XX:CompileCommand=quiet",
             "-XX:CompileCommand=option,compiler/blackhole/BlackholeTarget.bh_*,Blackhole"
         );
-        shouldPass(
+        shouldFail(
             "-XX:-PrintWarnings",
             "-XX:CompileCommand=quiet",
             "-XX:CompileCommand=blackhole,compiler/blackhole/BlackholeTarget.bh_*"

From ef620280e87b1ccad288f7f78a52228cdf3ae0ec Mon Sep 17 00:00:00 2001
From: Marc Chevalier <marc.chevalier@oracle.com>
Date: Mon, 19 May 2025 12:32:44 +0200
Subject: [PATCH 6/7] Revert blackhole test change

---
 .../compiler/blackhole/BlackholeExperimentalUnlockTest.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java b/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java
index 0d5132061a878..0a403a784506f 100644
--- a/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java
+++ b/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java
@@ -68,7 +68,7 @@ private static List<String> cmdline(String[] args) {
     public static void shouldFail(String... args) throws IOException {
         ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(cmdline(args));
         OutputAnalyzer output = new OutputAnalyzer(pb.start());
-        output.shouldHaveExitValue(1);
+        output.shouldHaveExitValue(0);
         output.shouldContain(MSG);
     }
 
@@ -103,12 +103,12 @@ public static void driver() throws IOException {
         );
 
         // Should be able to shun the warning
-        shouldFail(
+        shouldPass(
             "-XX:-PrintWarnings",
             "-XX:CompileCommand=quiet",
             "-XX:CompileCommand=option,compiler/blackhole/BlackholeTarget.bh_*,Blackhole"
         );
-        shouldFail(
+        shouldPass(
             "-XX:-PrintWarnings",
             "-XX:CompileCommand=quiet",
             "-XX:CompileCommand=blackhole,compiler/blackhole/BlackholeTarget.bh_*"

From f28b2076b4f709d2bcceedefdea7b081aff11cff Mon Sep 17 00:00:00 2001
From: Marc Chevalier <marc.chevalier@oracle.com>
Date: Tue, 20 May 2025 10:45:50 +0200
Subject: [PATCH 7/7] Use return value instead of testing buffer

---
 src/hotspot/share/compiler/compilerOracle.cpp | 79 +++++++++----------
 1 file changed, 36 insertions(+), 43 deletions(-)

diff --git a/src/hotspot/share/compiler/compilerOracle.cpp b/src/hotspot/share/compiler/compilerOracle.cpp
index 532f35fff867b..868ae8bfa41f4 100644
--- a/src/hotspot/share/compiler/compilerOracle.cpp
+++ b/src/hotspot/share/compiler/compilerOracle.cpp
@@ -317,7 +317,7 @@ TypedMethodOptionMatcher* TypedMethodOptionMatcher::match(const methodHandle& me
 }
 
 template<typename T>
-static void register_command(TypedMethodOptionMatcher* matcher,
+static bool register_command(TypedMethodOptionMatcher* matcher,
                              CompileCommandEnum option,
                              char* errorbuf,
                              const int buf_size,
@@ -333,7 +333,7 @@ static void register_command(TypedMethodOptionMatcher* matcher,
     warning("Blackhole compile option is experimental and must be enabled via -XX:+UnlockExperimentalVMOptions");
     // Delete matcher as we don't keep it
     delete matcher;
-    return;
+    return true;
   }
 
   if (!UnlockDiagnosticVMOptions) {
@@ -342,7 +342,7 @@ static void register_command(TypedMethodOptionMatcher* matcher,
     if (flag != nullptr && flag->is_diagnostic()) {
       jio_snprintf(errorbuf, buf_size, "VM option '%s' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.", name);
       delete matcher;
-      return;
+      return false;
     }
   }
 
@@ -358,7 +358,7 @@ static void register_command(TypedMethodOptionMatcher* matcher,
     matcher->print();
   }
 
-  return;
+  return true;
 }
 
 template<typename T>
@@ -726,7 +726,7 @@ static bool parseMemStat(const char* line, uintx& value, int& bytes_read, char*
   return false;
 }
 
-static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
+static bool scan_value(enum OptionType type, char* line, int& total_bytes_read,
         TypedMethodOptionMatcher* matcher, CompileCommandEnum option, char* errorbuf, const int buf_size) {
   int bytes_read = 0;
   const char* ccname = option2name(option);
@@ -745,11 +745,10 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
     }
     if (success) {
       total_bytes_read += bytes_read;
-      line += bytes_read;
-      register_command(matcher, option, errorbuf, buf_size, value);
-      return;
+      return register_command(matcher, option, errorbuf, buf_size, value);
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
+      return false;
     }
   } else if (type == OptionType::Uintx) {
     uintx value;
@@ -763,21 +762,20 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
     }
     if (success) {
       total_bytes_read += bytes_read;
-      line += bytes_read;
-      register_command(matcher, option, errorbuf, buf_size, value);
+      return register_command(matcher, option, errorbuf, buf_size, value);
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
+      return false;
     }
   } else if (type == OptionType::Ccstr) {
     ResourceMark rm;
     char* value = NEW_RESOURCE_ARRAY(char, strlen(line) + 1);
     if (sscanf(line, "%255[_a-zA-Z0-9]%n", value, &bytes_read) == 1) {
       total_bytes_read += bytes_read;
-      line += bytes_read;
-      register_command(matcher, option, errorbuf, buf_size, (ccstr) value);
-      return;
+      return register_command(matcher, option, errorbuf, buf_size, (ccstr) value);
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
+      return false;
     }
   } else if (type == OptionType::Ccstrlist) {
     // Accumulates several strings into one. The internal type is ccstr.
@@ -802,6 +800,7 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
 
         if (!validator.is_valid()) {
           jio_snprintf(errorbuf, buf_size, "Unrecognized intrinsic detected in %s: %s", option2name(option), validator.what());
+          return false;
         }
       }
 #if !defined(PRODUCT) && defined(COMPILER2)
@@ -810,18 +809,21 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
 
         if (!validator.is_valid()) {
           jio_snprintf(errorbuf, buf_size, "Unrecognized tag name in %s: %s", option2name(option), validator.what());
+          return false;
         }
       } else if (option == CompileCommandEnum::TraceMergeStores) {
         TraceMergeStores::TagValidator validator(value, true);
 
         if (!validator.is_valid()) {
           jio_snprintf(errorbuf, buf_size, "Unrecognized tag name in %s: %s", option2name(option), validator.what());
+          return false;
         }
       } else if (option == CompileCommandEnum::PrintIdealPhase) {
         PhaseNameValidator validator(value);
 
         if (!validator.is_valid()) {
           jio_snprintf(errorbuf, buf_size, "Unrecognized phase name in %s: %s", option2name(option), validator.what());
+          return false;
         }
       } else if (option == CompileCommandEnum::TestOptionList) {
         // all values are ok
@@ -831,35 +833,32 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
         assert(false, "Ccstrlist type option missing validator");
       }
 
-      register_command(matcher, option, errorbuf, buf_size, (ccstr) value);
-      return;
+      return register_command(matcher, option, errorbuf, buf_size, (ccstr) value);
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
+      return false;
     }
   } else if (type == OptionType::Bool) {
     char value[256];
     if (*line == '\0') {
       // Short version of a CompileCommand sets a boolean Option to true
       // -XXCompileCommand=<Option>,<method pattern>
-      register_command(matcher, option, errorbuf, buf_size,true);
-      return;
+      return register_command(matcher, option, errorbuf, buf_size,true);
     }
     if (sscanf(line, "%255[a-zA-Z]%n", value, &bytes_read) == 1) {
       if (strcasecmp(value, "true") == 0) {
         total_bytes_read += bytes_read;
-        line += bytes_read;
-        register_command(matcher, option, errorbuf, buf_size,true);
-        return;
+        return register_command(matcher, option, errorbuf, buf_size,true);
       } else if (strcasecmp(value, "false") == 0) {
         total_bytes_read += bytes_read;
-        line += bytes_read;
-        register_command(matcher, option, errorbuf, buf_size,false);
-        return;
+        return register_command(matcher, option, errorbuf, buf_size,false);
       } else {
         jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
+        return false;
       }
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
+      return false;
     }
   } else if (type == OptionType::Double) {
     char buffer[2][256];
@@ -869,21 +868,21 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read,
       char value[512] = "";
       jio_snprintf(value, sizeof(value), "%s.%s", buffer[0], buffer[1]);
       total_bytes_read += bytes_read;
-      line += bytes_read;
-      register_command(matcher, option, errorbuf, buf_size, atof(value));
-      return;
+      return register_command(matcher, option, errorbuf, buf_size, atof(value));
     } else {
       jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str);
+      return false;
     }
   } else {
     jio_snprintf(errorbuf, buf_size, "Type '%s' not supported ", type_str);
+    return false;
   }
 }
 
 // Scan next option and value in line, return MethodMatcher object on success, nullptr on failure.
 // On failure, error_msg contains description for the first error.
 // For future extensions: set error_msg on first error.
-static void scan_option_and_value(enum OptionType type, char* line, int& total_bytes_read,
+static bool scan_option_and_value(enum OptionType type, char* line, int& total_bytes_read,
                                 TypedMethodOptionMatcher* matcher,
                                 char* errorbuf, const int buf_size) {
   total_bytes_read = 0;
@@ -899,21 +898,21 @@ static void scan_option_and_value(enum OptionType type, char* line, int& total_b
     CompileCommandEnum option = match_option_name(option_buf, &bytes_read2, errorbuf, buf_size);
     if (option == CompileCommandEnum::Unknown) {
       assert(*errorbuf != '\0', "error must have been set");
-      return;
+      return false;
     }
     enum OptionType optiontype = option2type(option);
     if (option2type(option) != type) {
       const char* optiontype_name = optiontype2name(optiontype);
       const char* type_name = optiontype2name(type);
       jio_snprintf(errorbuf, buf_size, "Option '%s' with type '%s' doesn't match supplied type '%s'", option_buf, optiontype_name, type_name);
-      return;
+      return false;
     }
-    scan_value(type, line, total_bytes_read, matcher, option, errorbuf, buf_size);
+    return scan_value(type, line, total_bytes_read, matcher, option, errorbuf, buf_size);
   } else {
     const char* type_str = optiontype2name(type);
     jio_snprintf(errorbuf, buf_size, "Option name for type '%s' should be alphanumeric ", type_str);
+    return false;
   }
-  return;
 }
 
 void CompilerOracle::print_parse_error(char* error_msg, char* original_line) {
@@ -1008,8 +1007,7 @@ bool CompilerOracle::parse_from_line(char* line) {
       enum OptionType type = parse_option_type(option_type);
       if (type != OptionType::Unknown) {
         // Type (2) option: parse option name and value.
-        scan_option_and_value(type, line, bytes_read, typed_matcher, error_buf, sizeof(error_buf));
-        if (*error_buf != '\0') {
+        if (!scan_option_and_value(type, line, bytes_read, typed_matcher, error_buf, sizeof(error_buf))) {
           print_parse_error(error_buf, original.get());
           return false;
         }
@@ -1023,8 +1021,7 @@ bool CompilerOracle::parse_from_line(char* line) {
           return false;
         }
         if (option2type(option) == OptionType::Bool) {
-          register_command(typed_matcher, option, error_buf, sizeof(error_buf), true);
-          if (*error_buf != '\0') {
+          if (!register_command(typed_matcher, option, error_buf, sizeof(error_buf), true)) {
             print_parse_error(error_buf, original.get());
             return false;
           }
@@ -1057,16 +1054,14 @@ bool CompilerOracle::parse_from_line(char* line) {
     if (*line == '\0') {
       if (option2type(option) == OptionType::Bool) {
         // if this is a bool option this implies true
-        register_command(matcher, option, error_buf, sizeof(error_buf), true);
-        if (*error_buf != '\0') {
+        if (!register_command(matcher, option, error_buf, sizeof(error_buf), true)) {
           print_parse_error(error_buf, original.get());
           return false;
         }
         return true;
       } else if (option == CompileCommandEnum::MemStat) {
         // MemStat default action is to collect data but to not print
-        register_command(matcher, option, error_buf, sizeof(error_buf), (uintx)MemStatAction::collect);
-        if (*error_buf != '\0') {
+        if (!register_command(matcher, option, error_buf, sizeof(error_buf), (uintx)MemStatAction::collect)) {
           print_parse_error(error_buf, original.get());
           return false;
         }
@@ -1077,8 +1072,7 @@ bool CompilerOracle::parse_from_line(char* line) {
         return false;
       }
     }
-    scan_value(type, line, bytes_read, matcher, option, error_buf, sizeof(error_buf));
-    if (*error_buf != '\0') {
+    if (!scan_value(type, line, bytes_read, matcher, option, error_buf, sizeof(error_buf))) {
       print_parse_error(error_buf, original.get());
       return false;
     }
@@ -1185,8 +1179,7 @@ bool CompilerOracle::parse_compile_only(char* line) {
     if (method_pattern != nullptr) {
       TypedMethodOptionMatcher* matcher = TypedMethodOptionMatcher::parse_method_pattern(method_pattern, error_buf, sizeof(error_buf));
       if (matcher != nullptr) {
-        register_command(matcher, CompileCommandEnum::CompileOnly, error_buf, sizeof(error_buf), true);
-        if (*error_buf == '\0') {
+        if (register_command(matcher, CompileCommandEnum::CompileOnly, error_buf, sizeof(error_buf), true)) {
           continue;
         }
       }