JDK-8283013 : Simplify Arguments::parse_argument()
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 19
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2022-03-11
  • Updated: 2022-04-01
  • Resolved: 2022-03-31
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 19
19 masterFixed
Related Reports
Blocks :  
Blocks :  
Relates :  
Relates :  
Description
The current implementation of  Arguments::parse_argument() seems too complicated:

https://github.com/openjdk/jdk/blob/f8878cb0cc436993ef1222bc13b00b923d91aad1/src/hotspot/share/runtime/arguments.cpp#L1032

It makes a series of sscanf calls. The worst one is here: 

#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);
      return set_fp_numeric_flag(flag, value, origin);
    }
  }

However, the syntax of -XX is very simple

    -XX:+<name>
    -XX:-<name>
    -XX:<name>=<value>
    -XX:<name>:=<value>

It should be much easier to parse the string once, find out the <name>, <value> and the operator (+, -, =, or :=), and then parse the <value> according the type of <name>

This will make the code faster and easier to understand. It also avoid the following bug:

If you specify a double argument that has no decimal points. e.g., 

  java -XX:SomeDoubleArgument=1234567890

The VM first parses the input into a 64-bit integer, and then convert that to double. However, this means the user cannot specify an integer that's larger than the largest number representable by a 64-bit integer:

$ java -XX:InlineFrequencyRatio=1000000000000000000000000 -version
Improperly specified VM option 'InlineFrequencyRatio=1000000000000000000000000'
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

$ java -XX:InlineFrequencyRatio=1000000000000000000000000.0 -version
java version "19-internal" 2022-09-20
Java(TM) SE Runtime Environment (slowdebug build 19-internal-adhoc.iklam.zoo)
Java HotSpot(TM) 64-Bit Server VM (slowdebug build 19-internal-adhoc.iklam.zoo, mixed mode, sharing)

To fix this, we should always use strtod() to parse VM arguments of the double type.
Comments
Changeset: 49fcc7a5 Author: Ioi Lam <iklam@openjdk.org> Date: 2022-03-31 15:46:34 +0000 URL: https://git.openjdk.java.net/jdk/commit/49fcc7a5c3619b0580b263bbbb9431ad46c0f154
31-03-2022

The original double support was added as a by-product of JDK-6430224. There is no discussion about why such crude support was added. Initially you couldn't even use an integer value for a double-flag but that was later added. So doing this the right way is long overdue.
28-03-2022

A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk/pull/7916 Date: 2022-03-23 06:19:38 +0000
25-03-2022

ILW = MLM = P4
15-03-2022