Incorrect use of checked_cast in Arguments::process_settings_file

XMLWordPrintable

    • Type: Bug
    • Resolution: Unresolved
    • Priority: P4
    • 26
    • Affects Version/s: 22, 25, 26
    • Component/s: hotspot

      JDK-8313882 introduced a checked_cast when to silence the Wconversion warnings when we store an `int` in a `char`. The problem is that the assumption that the value in the `int` we get from `getc` is compatible with `char` is incorrect.

      The specification for `getc` is that it returns either `EOF` or the next read `unsigned char` converted to an `int`. This means we have the possible values of `{EOF, 0-255}`, we always check for `EOF` first, so we end up only having `{0-255}` as the possible values. (While a char has the possible values of `{-128-127}`.)

      So first the problem with checked_cast (which ensures that the type roundtrip is lossless) will end up converting any value in `{128-255}` to `{-128, -1}` which when converted back to an `int` is `{-128, -1}` and not `{128-255}`.

      The thing is that this is not a problem except for the assert as we do not care if the conversion roundtrip is lossless. We only want to reinterpret the value as a `char`, and never try to regain the unsigned representation converted to an `int` that `getc` uses.

      Another issue with keeping the value inside an `int` which could lead to bugs in the future is that when we use it in equality comparison agains a `char` we will encounter an integer promotion of the `char` which will lead to surprising incorrect results if the character is non-ascii.

      Currently we have no such comparisons of this `int` vs a non-ascii character. So the only issue is the assert inside checked_cast which will trigger if the settings file contains a non-ascii character.

      I suggest we convert the return value from `getc` to a `char` after we have check for `EOF` and before we use it as a character.

            Assignee:
            Axel Boldt-Christmas
            Reporter:
            Axel Boldt-Christmas
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: