Skip to content

8351958: Some compile commands should be made diagnostic #25150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
marc-chevalier wants to merge 7 commits into from
Closed

8351958: Some compile commands should be made diagnostic #25150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6c1f55b
Make some compile command locked by UnlockDiagnosticVMOptions
marc-chevalier May 2, 2025
8e6c9a9
Add some UnlockDiagnosticVMOptions
marc-chevalier May 5, 2025
f1837f6
Address comments
marc-chevalier May 16, 2025
670e401
For consistency
marc-chevalier May 16, 2025
159fa77
Re-use warning for blackhole
marc-chevalier May 19, 2025
ef62028
Revert blackhole test change
marc-chevalier May 19, 2025
f28b207
Use return value instead of testing buffer
marc-chevalier May 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 58 additions & 39 deletions src/hotspot/share/compiler/compilerOracle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,10 @@ 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,
T value) {
assert(matcher != option_list, "No circular lists please");
if (option == CompileCommandEnum::Log && !LogCompilation) {
Expand All @@ -331,7 +333,17 @@ 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) {
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 false;
}
}

matcher->init(option, option_list);
Expand All @@ -346,7 +358,7 @@ static void register_command(TypedMethodOptionMatcher* matcher,
matcher->print();
}

return;
return true;
}

template<typename T>
Expand Down Expand Up @@ -714,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);
Expand All @@ -733,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, 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;
Expand All @@ -751,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, 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, (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.
Expand All @@ -790,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)
Expand All @@ -798,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
Expand All @@ -819,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, (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, 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, 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, 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];
Expand All @@ -857,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, 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;
Expand All @@ -887,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) {
Expand Down Expand Up @@ -996,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;
}
Expand All @@ -1011,7 +1021,10 @@ bool CompilerOracle::parse_from_line(char* line) {
return false;
}
if (option2type(option) == OptionType::Bool) {
register_command(typed_matcher, option, true);
if (!register_command(typed_matcher, option, error_buf, sizeof(error_buf), true)) {
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));
Expand Down Expand Up @@ -1041,20 +1054,25 @@ 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);
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, (uintx)MemStatAction::collect);
if (!register_command(matcher, option, error_buf, sizeof(error_buf), (uintx)MemStatAction::collect)) {
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));
print_parse_error(error_buf, original.get());
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;
}
Expand Down Expand Up @@ -1161,8 +1179,9 @@ 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;
if (register_command(matcher, CompileCommandEnum::CompileOnly, error_buf, sizeof(error_buf), true)) {
continue;
}
}
}
ttyLocker ttyl;
Expand Down
12 changes: 6 additions & 6 deletions test/hotspot/jtreg/compiler/compilercontrol/commands/OptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,38 +37,38 @@

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")
.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.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");
Expand Down
1 change: 1 addition & 0 deletions test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestMultiplyToLen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestShift.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/hotspot/jtreg/compiler/intrinsics/bigInteger/TestSquareToLen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading