RFR: 8276453: Undefined behavior in C1 LIR_OprDesc causes SEGV in fastdebug build [v5]

[Man Cao]

On Wed, 3 Nov 2021 20:01:37 GMT, Man Cao <manc at openjdk.org> wrote:

>> Hi all,
>> 
>> Could anyone provide some feedback on this bug fix and refactoring change? See https://bugs.openjdk.java.net/browse/JDK-8276453 for more details.
>> If the direction of this change looks good, we can proceed removing the "UGLY HACK" in c1_LIR.hpp and refactor occurrences of "opr->fn()" to "opr.fn()".
>
> Man Cao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix aarch64 and arm builds

I'm running DaCapo benchmarks to check C1 performance, and will post results after it's done.

> Wouldn't adding something like:
> typedef LIR_Opr LIR_OprDesc;
> make most of the renames unnecessary?

That could work. Do you think it is better to split this into two or three RFEs:
(1) Mainly change c1_LIR.hpp/cpp, with the "typedef LIR_Opr LIR_OprDesc;" workaround. This minimizes the patch size and makes it easier to backport to JDK 11 and JDK 17.
(2) Rename all remaining LIR_OprDesc to LIR_Opr.
(3) Replace "->" with "." and remove the hack "LIR_Opr* operator->() { return this; }" in c1_LIR.hpp. (~rasbold has made a patch for x86 Linux in Google internal JDK 11 already.)

> I also wonder if LIR_Opr() is the best choice to replace NULL. In some places LIR_Opr::illegalOpr() is used instead, which seems inconsistent. Maybe we need LIR_Opr::nullOpr()?

Do you think it is feasible to replace all the NULL and 0 with LIR_Opr::illegalOpr()?

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

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