8283013: Simplify Arguments::parse_argument()

Reviewed-by: dholmes, ccheung
This commit is contained in:
Ioi Lam 2022-03-31 15:46:34 +00:00
parent 73cb922bfc
commit 49fcc7a5c3
4 changed files with 148 additions and 120 deletions

View File

@ -891,13 +891,21 @@ static bool set_bool_flag(JVMFlag* flag, bool value, JVMFlagOrigin origin) {
}
}
static bool set_fp_numeric_flag(JVMFlag* flag, char* value, JVMFlagOrigin origin) {
static bool set_fp_numeric_flag(JVMFlag* flag, const char* value, JVMFlagOrigin origin) {
// strtod allows leading whitespace, but our flag format does not.
if (*value == '\0' || isspace(*value)) {
return false;
}
char* end;
errno = 0;
double v = strtod(value, &end);
if ((errno != 0) || (*end != 0)) {
return false;
}
if (g_isnan(v) || !g_isfinite(v)) {
// Currently we cannot handle these special values.
return false;
}
if (JVMFlagAccess::set_double(flag, &v, origin) == JVMFlag::SUCCESS) {
return true;
@ -905,60 +913,48 @@ static bool set_fp_numeric_flag(JVMFlag* flag, char* value, JVMFlagOrigin origin
return false;
}
static JVMFlag::Error set_numeric_flag(JVMFlag* flag, char* value, JVMFlagOrigin origin) {
if (flag == NULL) {
return JVMFlag::INVALID_FLAG;
}
static bool set_numeric_flag(JVMFlag* flag, const char* value, JVMFlagOrigin origin) {
JVMFlag::Error result = JVMFlag::WRONG_FORMAT;
if (flag->is_int()) {
int v;
if (parse_integer(value, &v)) {
return JVMFlagAccess::set_int(flag, &v, origin);
result = JVMFlagAccess::set_int(flag, &v, origin);
}
} else if (flag->is_uint()) {
uint v;
if (parse_integer(value, &v)) {
return JVMFlagAccess::set_uint(flag, &v, origin);
result = JVMFlagAccess::set_uint(flag, &v, origin);
}
} else if (flag->is_intx()) {
intx v;
if (parse_integer(value, &v)) {
return JVMFlagAccess::set_intx(flag, &v, origin);
result = JVMFlagAccess::set_intx(flag, &v, origin);
}
} else if (flag->is_uintx()) {
uintx v;
if (parse_integer(value, &v)) {
return JVMFlagAccess::set_uintx(flag, &v, origin);
result = JVMFlagAccess::set_uintx(flag, &v, origin);
}
} else if (flag->is_uint64_t()) {
uint64_t v;
if (parse_integer(value, &v)) {
return JVMFlagAccess::set_uint64_t(flag, &v, origin);
result = JVMFlagAccess::set_uint64_t(flag, &v, origin);
}
} else if (flag->is_size_t()) {
size_t v;
if (parse_integer(value, &v)) {
return JVMFlagAccess::set_size_t(flag, &v, origin);
}
} else if (flag->is_double()) {
// This function parses only input strings without a decimal
// point character (.)
// If a string looks like a FP number, it would be parsed by
// set_fp_numeric_flag(). See Arguments::parse_argument().
jlong v;
if (parse_integer(value, &v)) {
double double_v = (double) v;
if (value[0] == '-' && v == 0) { // special case: 0.0 is different than -0.0.
double_v = -0.0;
}
return JVMFlagAccess::set_double(flag, &double_v, origin);
result = JVMFlagAccess::set_size_t(flag, &v, origin);
}
}
return JVMFlag::WRONG_FORMAT;
return result == JVMFlag::SUCCESS;
}
static bool set_string_flag(JVMFlag* flag, const char* value, JVMFlagOrigin origin) {
if (value[0] == '\0') {
value = NULL;
}
if (JVMFlagAccess::set_ccstr(flag, &value, origin) != JVMFlag::SUCCESS) return false;
// Contract: JVMFlag always returns a pointer that needs freeing.
FREE_C_HEAP_ARRAY(char, value);
@ -992,7 +988,7 @@ static bool append_to_string_flag(JVMFlag* flag, const char* new_value, JVMFlagO
return true;
}
const char* Arguments::handle_aliases_and_deprecation(const char* arg, bool warn) {
const char* Arguments::handle_aliases_and_deprecation(const char* arg) {
const char* real_name = real_flag_name(arg);
JDK_Version since = JDK_Version();
switch (is_deprecated_flag(arg, &since)) {
@ -1010,16 +1006,14 @@ const char* Arguments::handle_aliases_and_deprecation(const char* arg, bool warn
case 0:
return real_name;
case 1: {
if (warn) {
char version[256];
since.to_string(version, sizeof(version));
if (real_name != arg) {
warning("Option %s was deprecated in version %s and will likely be removed in a future release. Use option %s instead.",
arg, version, real_name);
} else {
warning("Option %s was deprecated in version %s and will likely be removed in a future release.",
arg, version);
}
char version[256];
since.to_string(version, sizeof(version));
if (real_name != arg) {
warning("Option %s was deprecated in version %s and will likely be removed in a future release. Use option %s instead.",
arg, version, real_name);
} else {
warning("Option %s was deprecated in version %s and will likely be removed in a future release.",
arg, version);
}
return real_name;
}
@ -1028,96 +1022,85 @@ const char* Arguments::handle_aliases_and_deprecation(const char* arg, bool warn
return NULL;
}
bool Arguments::parse_argument(const char* arg, JVMFlagOrigin origin) {
// range of acceptable characters spelled out for portability reasons
#define NAME_RANGE "[abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_]"
#define BUFLEN 255
char name[BUFLEN+1];
char dummy;
const char* real_name;
bool warn_if_deprecated = true;
if (sscanf(arg, "-%" XSTR(BUFLEN) NAME_RANGE "%c", name, &dummy) == 1) {
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
JVMFlag* Arguments::find_jvm_flag(const char* name, size_t name_length) {
char name_copied[BUFLEN+1];
if (name[name_length] != 0) {
if (name_length > BUFLEN) {
return NULL;
} else {
strncpy(name_copied, name, name_length);
name_copied[name_length] = '\0';
name = name_copied;
}
JVMFlag* flag = JVMFlag::find_flag(real_name);
return set_bool_flag(flag, false, origin);
}
if (sscanf(arg, "+%" XSTR(BUFLEN) NAME_RANGE "%c", name, &dummy) == 1) {
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
}
JVMFlag* flag = JVMFlag::find_flag(real_name);
return set_bool_flag(flag, true, origin);
}
char punct;
if (sscanf(arg, "%" XSTR(BUFLEN) NAME_RANGE "%c", name, &punct) == 2 && punct == '=') {
const char* value = strchr(arg, '=') + 1;
const char* real_name = Arguments::handle_aliases_and_deprecation(name);
if (real_name == NULL) {
return NULL;
}
JVMFlag* flag = JVMFlag::find_flag(real_name);
return flag;
}
// this scanf pattern matches both strings (handled here) and numbers (handled later))
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
bool Arguments::parse_argument(const char* arg, JVMFlagOrigin origin) {
bool is_bool = false;
bool bool_val = false;
char c = *arg;
if (c == '+' || c == '-') {
is_bool = true;
bool_val = (c == '+');
arg++;
}
const char* name = arg;
while (true) {
c = *arg;
if (isalnum(c) || (c == '_')) {
++arg;
} else {
break;
}
}
size_t name_len = size_t(arg - name);
if (name_len == 0) {
return false;
}
JVMFlag* flag = find_jvm_flag(name, name_len);
if (flag == NULL) {
return false;
}
if (is_bool) {
if (*arg != 0) {
// Error -- extra characters such as -XX:+BoolFlag=123
return false;
}
JVMFlag* flag = JVMFlag::find_flag(real_name);
if (flag != NULL && flag->is_ccstr()) {
return set_bool_flag(flag, bool_val, origin);
}
if (arg[0] == '=') {
const char* value = arg + 1;
if (flag->is_ccstr()) {
if (flag->ccstr_accumulates()) {
return append_to_string_flag(flag, value, origin);
} else {
if (value[0] == '\0') {
value = NULL;
}
return set_string_flag(flag, value, origin);
}
} else {
warn_if_deprecated = false; // if arg is deprecated, we've already done warning...
}
}
if (sscanf(arg, "%" XSTR(BUFLEN) NAME_RANGE ":%c", name, &punct) == 2 && punct == '=') {
const char* value = strchr(arg, '=') + 1;
// -XX:Foo:=xxx will reset the string flag to the given value.
if (value[0] == '\0') {
value = NULL;
}
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
}
JVMFlag* flag = JVMFlag::find_flag(real_name);
return set_string_flag(flag, value, origin);
}
#define SIGNED_FP_NUMBER_RANGE "[-0123456789.eE+]"
#define SIGNED_NUMBER_RANGE "[-0123456789]"
#define NUMBER_RANGE "[0123456789eE+-]"
char value[BUFLEN + 1];
char value2[BUFLEN + 1];
if (sscanf(arg, "%" XSTR(BUFLEN) NAME_RANGE "=" "%" XSTR(BUFLEN) SIGNED_NUMBER_RANGE "." "%" XSTR(BUFLEN) NUMBER_RANGE "%c", name, value, value2, &dummy) == 3) {
// Looks like a floating-point number -- try again with more lenient format string
if (sscanf(arg, "%" XSTR(BUFLEN) NAME_RANGE "=" "%" XSTR(BUFLEN) SIGNED_FP_NUMBER_RANGE "%c", name, value, &dummy) == 2) {
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
}
JVMFlag* flag = JVMFlag::find_flag(real_name);
} else if (flag->is_double()) {
return set_fp_numeric_flag(flag, value, origin);
} else {
return set_numeric_flag(flag, value, origin);
}
}
#define VALUE_RANGE "[-kmgtxKMGTX0123456789abcdefABCDEF]"
if (sscanf(arg, "%" XSTR(BUFLEN) NAME_RANGE "=" "%" XSTR(BUFLEN) VALUE_RANGE "%c", name, value, &dummy) == 2) {
real_name = handle_aliases_and_deprecation(name, warn_if_deprecated);
if (real_name == NULL) {
return false;
}
JVMFlag* flag = JVMFlag::find_flag(real_name);
return set_numeric_flag(flag, value, origin) == JVMFlag::SUCCESS;
if (arg[0] == ':' && arg[1] == '=') {
// -XX:Foo:=xxx will reset the string flag to the given value.
const char* value = arg + 2;
return set_string_flag(flag, value, origin);
}
return false;

View File

@ -37,6 +37,8 @@
// Arguments parses the command line and recognizes options
class JVMFlag;
// Invocation API hook typedefs (these should really be defined in jni.h)
extern "C" {
typedef void (JNICALL *abort_hook_t)(void);
@ -464,10 +466,11 @@ class Arguments : AllStatic {
// Return the real name for the flag passed on the command line (either an alias name or "flag_name").
static const char* real_flag_name(const char *flag_name);
static JVMFlag* find_jvm_flag(const char* name, size_t name_length);
// Return the "real" name for option arg if arg is an alias, and print a warning if arg is deprecated.
// Return NULL if the arg has expired.
static const char* handle_aliases_and_deprecation(const char* arg, bool warn);
static const char* handle_aliases_and_deprecation(const char* arg);
static char* SharedArchivePath;
static char* SharedDynamicArchivePath;

View File

@ -28,6 +28,7 @@
#include "runtime/flags/jvmFlag.hpp"
#include "utilities/align.hpp"
#include "utilities/globalDefinitions.hpp"
#include <errno.h>
class ArgumentsTest : public ::testing::Test {
public:
@ -261,16 +262,12 @@ void check_numeric_flag(JVMFlag* flag, T getvalue(JVMFlag* flag),
}
{
// Invalid strings for *any* type of integer VM arguments
// Invalid strings for *any* numeric type of VM arguments
const char* invalid_strings[] = {
"", " 1", "2 ", "3 2",
"0x", "0x0x1" "e"
"K", "M", "G", "1MB", "1KM", "AA", "0B",
"18446744073709551615K", "17179869184G",
"999999999999999999999999999999",
"0x10000000000000000", "18446744073709551616",
"-0x10000000000000000", "-18446744073709551616",
"-0x8000000000000001", "-9223372036854775809",
"0x8000000t", "0x800000000g",
"0x800000000000m", "0x800000000000000k",
"-0x8000000t", "-0x800000000g",
@ -280,9 +277,21 @@ void check_numeric_flag(JVMFlag* flag, T getvalue(JVMFlag* flag),
check_invalid_numeric_string(flag, invalid_strings);
}
if (!is_double) {
if (is_double) {
const char* invalid_strings_for_double[] = {
"INF", "Inf", "Infinity", "INFINITY",
"-INF", "-Inf", "-Infinity", "-INFINITY",
"nan", "NAN", "NaN",
NULL,
};
check_invalid_numeric_string(flag, invalid_strings_for_double);
} else {
const char* invalid_strings_for_integers[] = {
"1.0", "0x4.5", "0.001", "4e10",
"999999999999999999999999999999",
"0x10000000000000000", "18446744073709551616",
"-0x10000000000000000", "-18446744073709551616",
"-0x8000000000000001", "-9223372036854775809",
NULL,
};
check_invalid_numeric_string(flag, invalid_strings_for_integers);
@ -549,8 +558,6 @@ TEST_VM_F(ArgumentsTest, set_numeric_flag_double) {
return;
}
// TODO -- JDK-8282774
// Need to add more test input that have a fractional part like "4.2".
NumericArgument<double> valid_strings[] = {
NumericArgument<double>("0", 0.0),
NumericArgument<double>("1", 1.0),
@ -564,4 +571,36 @@ TEST_VM_F(ArgumentsTest, set_numeric_flag_double) {
check_numeric_flag<double>(flag, getvalue, valid_strings,
ARRAY_SIZE(valid_strings), /*is_double=*/true);
const char* more_valid_strings[] = {
// These examples are from https://en.cppreference.com/w/cpp/language/floating_literal
// (but with the L and F suffix removed).
"1e10", "1e-5",
"1.e-2", "3.14",
".1", "0.1e-1",
"0x1ffp10", "0X0p-1",
"0x1.p0", "0xf.p-1",
"0x0.123p-1", "0xa.bp10",
"0x1.4p3",
// More test cases
"1.5", "6.02e23", "-6.02e+23",
"1.7976931348623157E+308", // max double
"-0", "0",
"0x1.91eb85p+1",
"999999999999999999999999999999",
};
for (uint i = 0; i < ARRAY_SIZE(more_valid_strings); i++) {
const char* str = more_valid_strings[i];
char* dummy;
double expected = strtod(str, &dummy);
ASSERT_TRUE(errno == 0);
ASSERT_TRUE(ArgumentsTest::parse_argument(flag->name(), str))
<< "Valid string '" <<
str << "' did not parse for type " << flag->type_string() << ".";
double d = flag->get_double();
ASSERT_TRUE(d == expected)
<< "Parsed number " << d << " is not the same as expected " << expected;
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2022, 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
@ -127,7 +127,10 @@ public class DoubleJVMOption extends JVMOption {
validValues.add(formatValue(min));
}
if (testMaxRange) {
validValues.add(formatValue(max));
if (!name.equals("CompileThresholdScaling")) {
// See JDK-8283807: Max range for -XX:CompileThresholdScaling is too large
validValues.add(formatValue(max));
}
}
if (testMinRange) {