RFR: 8282773: Refactor parsing of integer VM options [v2]

[Ioi Lam]

On Thu, 10 Mar 2022 01:43:22 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision:
>> 
>>  - Merge branch 'master' into new-interger-argument-parser
>>  - fixed handling of errno
>>  - add #include <limits> for std::numeric_limits<T>
>>  - avoid UB in multiply_by_1k(); more clean-up
>>  - avoid UB operations in k/m/g/t muplication
>>  - refactored test cases
>>  - step3
>>  - step2
>>  - tmp1
>
> src/hotspot/share/runtime/arguments.cpp line 751:
> 
>> 749: static bool parse_integer_impl(const char *s, char **endptr, int base, T* result) {
>> 750:   // Can't use strtol because it doesn't detect (at least on Linux/gcc) overflowing
>> 751:   // input such as "0x123456789" or "-0x800000001"
> 
> This doesn't seem to be true in general. The Linux manpage indicates overflow and underflow are detected. And a simple test program confirms:
> 
> 
>  > ./conversion
> Overflow detected okay for 0x123456789 (val = 2147483647)
> Overflow detected okay for -0x800000001 (val = -2147483648)

I tried changing the code to this. It doesn't work. "0x123456789" is incorrectly parsed to 591751049 for the `int` flag `AVX3Threshold` without setting `errno`:


template <typename T, ENABLE_IF(std::is_signed<T>::value), ENABLE_IF(sizeof(T) == 4)> // signed 32-bit
static bool parse_integer_impl(const char *s, char **endptr, int base, T* result) {
  errno = 0; // errno is thread safe
  *result = strtol(s, endptr, base);
  return errno == 0;
}

$ java -XX:AVX3Threshold=0 --version
java 19-internal 2022-09-20
Java(TM) SE Runtime Environment (slowdebug build 19-internal-adhoc.iklam.ken)
Java HotSpot(TM) 64-Bit Server VM (slowdebug build 19-internal-adhoc.iklam.ken, mixed mode, sharing)
$ java -XX:AVX3Threshold=0x123456789 --version
AVX3Threshold ( 591751049 ) must be 0 or a power of two value between 0 and MAX_INT
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

$ gcc --version
gcc (GCC) 10.3.0
$ uname -a
Linux host 5.13.0-28-generic #31-Ubuntu SMP Thu Jan 13 17:41:06 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux


My stand-alone test case:


#include <stdio.h>
#include <stdlib.h>
#include <errno.h>

int main(int argc, char** argv) {
  if (argc >= 2) {
    char* endptr;

    errno = 0;    
    int n = strtol(argv[1], &endptr, 16);
    printf("errno = %d, strtol() returns %d == 0x%x\n", errno, n, n);
  }
}

$ gcc -g strtol.cpp -o strtol
$ ./strtol 0x12345678
errno = 0, strtol() returns 305419896 == 0x12345678
$ ./strtol 0x123456789
errno = 0, strtol() returns 591751049 == 0x23456789

> src/hotspot/share/runtime/arguments.cpp line 945:
> 
>> 943:   } else if (flag->is_double()) {
>> 944:     jlong v;
>> 945:     if (parse_integer(value, &v)) {
> 
> I don't see how this can parse floating-point values ???

Floating point strings with a decimal point character are parsed elsewhere in `set_fp_numeric_flag()`. I'll add a comment to clarify this.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7763