Invalid code generated by C2 compiler in OpenJDK 21

[Antoine DESSAIGNE]

Hi David,

Here's a simplified version of the code
private void resolveArrhythmicUpdate() {
    // extracting a value non-null value
    ReadableInterval storedValueVT = storedValue.getValidTime();
    if (conditionA && !conditionB) {
        // then call storedValueVT.getStartMillis() and it's working
    }
    // some code not altering storedValueVT
    // same if as previouly but with one more condition
    if (conditionA && !conditionB && !conditionC) {
        // then call storedValueVT.getStartMillis() and it throws an NPE
    }
}

In my code storedValueVT always has a value, it's tested in the first
if, but in the second if somehow it throws an NPE. The variable is not
set to null and it's not reassigned, it only happens with C2 compiled
code in the second "if" statement, the first one always works.

I cannot publicly share the code on the internet but if you mail me
directly we can arrange a video call and I can show you the code.

As for the branch, here's the bisect history I did, started at
jdk-19-ga, so skipping some history (but I have it complete if you
need)
good: [0ba473489151d74c8a15b75ff4964ac480fecb28]
bad: [0eeaeb8e7ba40be5e93eb87c7e3dc94230062746]
bad: [3696711efa566fb776d6923da86e17b0e1e22964]
good: [f771c56e16a39724712ca0d8c2dd55b9ce260f4d]
first bad commit: [3696711efa566fb776d6923da86e17b0e1e22964]

The commit marked as "first bad commit" is a merge that has 2 parents
f4caaca100d334b671eed56287dfe7a1009c47d7 and
f771c56e16a39724712ca0d8c2dd55b9ce260f4d. We know from the first
bisect that the commit f771 is valid so I redid a bisect on the other
side, ending with f4ca...
bad: [3696711efa566fb776d6923da86e17b0e1e22964]
good: [10bc86cc260fac48bf10f67dd56aa73c6954f026]
bad: [e41686b4050d6b32fb451de8af39a78ec8bed0fd]
good: [0ef353925e645dd519e17aeb7a83e927271f8b95]
bad: [3cdbd878e68dc1131093137a7357710ad303ae8c]
good: [4b313b51b1787113961c289a41708e31fa19cacc]
bad: [10737e168c967a08e257927251861bf2c14795ab]
first bad commit: [10737e168c967a08e257927251861bf2c14795ab]

As I thought many times that I've messed up finding the commit, I
tested the parent commit the commit
10737e168c967a08e257927251861bf2c14795ab and its parent many many
times and it's always the same: 10737e1 is failing whereas its parent
is always working.

Thank you for your help with this bug.

Le lun. 18 déc. 2023 à 01:36, David Holmes <david.holmes at oracle.com> a écrit :
>
> Also from the original:
>
>  > I bisected the repository to find where the regression comes from. I
>  > found this commit 3696711efa5 [1] but it's a merge so I bisected the
>  > branch and found 10737e168c9 [2].
>
> but commit [2] is not part of the merge commit [1].
>
> David
>
> On 18/12/2023 10:02 am, David Holmes wrote:
> > On 16/12/2023 3:44 am, Volker Simonis wrote:
> >> On Fri, Dec 15, 2023 at 1:11 PM Antoine DESSAIGNE
> >> <antoine.dessaigne at gmail.com> wrote:
> >>>
> >>> Thank you Volker and Andrew for your replies,
> >>>
> >>> Unfortunately, there's no hs_error generated, the code throws a
> >>> NullPointerException when it shouldn't.
> >>
> >> Sorry, overlooked the "without" before "crashing due to assertions" :)
> >>
> >>>
> >>> I can even reproduce it with JDK 23, I took the master of the jdk
> >>> repository this morning (commit b31454e3623 to be precise) and it
> >>> still fails.
> >>>
> >>> I cannot exclude this method from the JIT as it's used a lot in our
> >>> application.
> >>>
> >>> I checked many many times the commit to be sure that I have the right
> >>> one and I do. It fails almost every time with 10737e168c9 [1] but it
> >>> never fails with its parent commit, even after 20 tests.
> >>>
> >>
> >> The change looks innocent, but CCing hotstpo-runtime-dev and Coleen
> >> (who's the author of that change [1]). Maybe she has an idea?
> >
> > Coleen is away for a while. I reviewed the change in [1] and I also
> > cannot see how it could affect anything to do with the JIT.
> >
> >  From the original mail on compiler-dev:
> >
> >  >  A valued local variable (effectively final) has its value removed
> >
> > What does this mean???
> >
> > David
> > -----
> >
> >> Is your code doing a lot of dynamic class loading and/or bytecode
> >> instrumentation/rewriting?
> >>
> >>> [1]
> >>> https://github.com/openjdk/jdk/commit/10737e168c967a08e257927251861bf2c14795ab
> >>>
> >>> Le ven. 15 déc. 2023 à 12:37, Andrew Haley
> >>> <aph-open at littlepinkcloud.com> a écrit :
> >>>>
> >>>> On 12/15/23 10:20, Antoine DESSAIGNE wrote:
> >>>>> Do you have an idea of why this is happening? Do you know what test I
> >>>>> can run?
> >>>>
> >>>> First, try to reproduce it with JDK 22 preview.
> >>>>
> >>>> If you can't provide a reproducer, it's likely that no one will be
> >>>> able to fix it now, and you'll have to wait until it gets fixed.
> >>>>
> >>>> Try: '-XX:CompileCommand=exclude,foo.bar.baz.Classname::badMethodName'
> >>>>
> >>>> --
> >>>> Andrew Haley  (he/him)
> >>>> Java Platform Lead Engineer
> >>>> Red Hat UK Ltd. <https://www.redhat.com>
> >>>> https://keybase.io/andrewhaley
> >>>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
> >>>>