RFR: 8346836: C2: Introduce a way to verify the correctness of ConstraintCastNodes at runtime

[Quan Anh Mai]

On Wed, 8 Jan 2025 07:24:18 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Hi,
>> 
>> This patch adds a develop flag `VerifyConstraintCasts`, which will verify the correctness of `CastIINode`s and `CastLLNode`s at runtime and crash the VM if the dynamic value lies outside the type value range.
>> 
>> Please take a look, thanks a lot.
>
> Ah, I actually see that you have some examples. So you plan on introducing this flag first, and only then fixing the issues? But does it fail with a simple `java --version`? Or an empty `main` method, maybe with `-Xcomp`?

@eme64 Thanks for looking at this

The context is that while reviewing #22666 I came to the conclusion that our handling of `depends_only_on_test` is broken. I have added a comment explaining my understanding and concerns there. In principle, before the execution, a `DivINode` is the same as a `CastIINode` which limits the value range of the divisor to `!= 0`. As a result, there should not be any difference in the way we handle the movements of these nodes. This leads me to the conclusion that `CastIINode`s may also be wired to the wrong control input, the reason we have not caught them is that unlike a division complaining loudly, a `CastIINode` will silently accept incorrect input values. This motivates me to make this patch.

> If I remember correctly, we relax/widen the Cast ranges somewhere later in optimizations, so that different CastII etc can common. Probably happens after loop-opts. So the ranges usually go from `[1..10]` -> `[0, max]` or `[-1 .. 1]` -> `int`.

You are right, it is in `ConstraintCastNode::widen_type` for which I will disable that widening in the presence of `VerifyConstraintCasts`.

> So you plan on introducing this flag first, and only then fixing the issues?

There are several failures in tier 1 alone, and this flag is not enabled by default or in the pipeline, so I think incorporating it first would be preferable, then after fixing all the issues we can add it to the stress options.

> But does it fail with a simple `java --version`? Or an empty `main` method, maybe with `-Xcomp`?

No it does not fail with `--version` or with an empty `main` method with and without `-Xcomp`.

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

PR Comment: https://git.openjdk.org/jdk/pull/22880#issuecomment-2576960560