Integrated: 8282773: Refactor parsing of integer VM options
Ioi Lam
iklam at openjdk.java.net
Thu Mar 17 17:54:37 UTC 2022
On Wed, 9 Mar 2022 19:59:21 GMT, Ioi Lam <iklam at openjdk.org> wrote:
> Background:
>
> When a VM option is given an integer value (e.g., `-XX:ActiveProcessorCount=123`), we call the `set_numeric_flag` function in arguments.cpp. The old implementation always calls `strtoull()` to get an unsigned 64-bit number, and then tries to cast/truncate the result to the target type, performing sign conversion as needed.
>
> This is cumbersome and buggy, because the target type (such as `size_t`) may have different bit-widths on different platforms. Also, manual conversion of unsigned to sign values can run into corner cases.
>
> The fix:
>
> - Use C++ templates and SFINAE to automatically choose the correct `parse_integer_impl()` function for parsing the input string. The VM should avoid doing cast and sign conversion as much as possible.
> - Use a similar technique to write a comprehensive set of test cases to validate the implementation. (This has revealed several failures in the old implementation).
>
> Notes on K/M/G/T suffix handling:
>
> For input like `-XX:ActiveProcessorCount=2048m`, the old implementation checks for overflow with `(n * suffix / suffix) == n`. This works for unsigned types because the [C++ specification](https://en.cppreference.com/w/cpp/language/operator_arithmetic) allows unsigned arithmetic to overflow. However, overflowing a signed integer is UB. Therefore, I wrote a new function `multiply_by_1k()` to check for overflow without potentially undefined behavior. Thanks to @kimbarrett for helping me come up with the solution.
>
> Tests:
>
> tiers 1-2 passed. tiers 3-5 in progress.
This pull request has now been integrated.
Changeset: b004fb05
Author: Ioi Lam <iklam at openjdk.org>
URL: https://git.openjdk.java.net/jdk/commit/b004fb0550d8fc94e3f6412975c23c0a2ac2a42f
Stats: 684 lines in 4 files changed: 479 ins; 155 del; 50 mod
8282773: Refactor parsing of integer VM options
Reviewed-by: dholmes, kbarrett
-------------
PR: https://git.openjdk.java.net/jdk/pull/7763
More information about the hotspot-runtime-dev
mailing list