RFR: 8280067: Incorrect code generated for unary - on char operand

[Vicente Romero]

On Thu, 20 Jan 2022 12:31:57 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Consider the following snippet of code:
> 
> Character var = (char) -(false ? (char) param : (char) 2);
> 
> 
> The `javac` frontend will not put a constant type on `false ? (char) param : (char) 2` (i.e. it will not constant-fold `(char) 2` to the whole conditional expression). This seems correct, as this is not (I think) a constant expression. But then, in the code generator:
> -`Gen.visitUnary` is called, which requests to evaluate the argument, specifying the type should be `int`.
> -when the argument (i.e. `false ? (char) param : (char) 2`) is evaluted, it produces an `ImmediateItem` with the constant, of type `char`, skipping the "then" expression, as that is never reached
> -then the type of the `Item` is converted using `coerce` to the desired type (`int`). But in `ImmediateItem.coerce`, `char` is considered to be "equivalent" to `int`, and no conversion happens, and the same (`char`) `ImmediateItem` is returned
> -`Gen.visitUnary` will push the constant  the stack and will produce the `ineg` instruction, but will (effectively) use the type of the parameter's `Item` for its result
> -so the outermost `char` cast is a no-op, because the stack's top is considered to be `char` - but that is not correct, because `char` is unsigned, and the actual value at the top of the stack is `-2`.
> 
> This can then lead to various wrong behavior, including an exception from `Character.valueOf(char)`.
> 
> The proposal is to produce an `Item` with the correct type (i.e. `int`) in `ImmediateItem.coerce` for `char`-typed `ImmediateItem`s. `Item`s of type `byte`/`short` are kept as they are, even though it is not clear to me if simply producing an `int`-typed `Item` would not work equally well. But returning an `Item` with type `char` when `int` is requested seems wrong due to the unsigned property of `char`.

I'm seeing that Lower is reducing the conditional to `(char)(char)2` at `Lower::visitConditional` could it be that the type is wrongly set to `int` somewhere before reaching to `Gen`?

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

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