JDK-8331133 : C2: wrong result - broken memory graph - arrays with bottom element type
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 11,17,21,25,26
  • Priority: P3
  • Status: In Progress
  • Resolution: Unresolved
  • Submitted: 2024-04-25
  • Updated: 2025-10-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 26
26Unresolved
Related Reports
Relates :  
JDK-8349930 - C2: verify memory liveness invariants after scheduling
Relates :  
JDK-8331054 - C2 MergeStores: assert failed: unexpected basic type after JDK-8318446 and JDK-8329555
Description
I was inspired by a recent bug found by [~jbhateja]: JDK-8329555

There, we have a handle to an Object, which we know to be an array, but there are multiple possible element type, say a "byte[]" or "int[]".

Also JDK-8331054 dealt with such an example.

Now I found a more general bug. And this with or without UseSuperWord.

On JDK23, we can run:
./java --add-modules java.base --add-exports java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.util=ALL-UNNAMED -Xbatch -XX:CompileCommand=compileonly,Test::test* -XX:-UseSuperWord Test.java
CompileCommand: compileonly Test.test* bool compileonly = true
Exception in thread "main" java.lang.RuntimeException: wrong value
	at Test.main(Test.java:26)

Also older versions are affected:
/oracle-work/jdk-11.0.9.0.4/fastdebug/bin/java --add-modules java.base --add-exports java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.util=ALL-UNNAMED -Xbatch -XX:CompileCommand=compileonly,Test::test* -XX:-UseSuperWord Test.java


This example may seem quite contrived, with Unsafe. But this is a perfectly reasonable access pattern for MemorySegments. Such an example is what [~jbhateja] encountered in JDK-8329555.
Comments
A naive conversion of the attached Test.java to use MemorySegements (TestMemSeg.java) does not fail with a wrong execution, but still exhibits the same missing relation in the graph, so seems susceptible to the same kind of wrong execution.
14-10-2025
Intermittent and not a new regression. Deferring to JDK 26 for now.
05-05-2025
Deferring to JDK 25 for now because this is an old issue. Feel free to re-target to JDK 24 if the fix is ready in time.
17-10-2024
Unassigning, because I will not work on this during the next few months. Feel free to reassign it to me if it has higher priority.
08-08-2024
Deferring from JDK23 -> JDK24, since this is an old issue (at least JDK11).
16-05-2024
[~epeter] Good point. I guess the programmer could effectively do that by putting the loop in a separate function/lambda, then calling it inside the if and else clauses, letting inlining optimize it.
15-05-2024
[~dlong] We could possibly split here. But not in general. Actually, not even in my example: The if is before the loop. So we would not just have to split the store, but the whole loop. We certainly do not do that.
15-05-2024
It might be nice if the full memory slice "int[]" "OR" "byte[]" alias information from the Phi could be retained, instead of losing information when going to a single bottom[] alias. I thought that's what MergeMemNode nodes were for, but I'm not very familiar with that part of C2. When we decide if two memory operations conflict, do we only checks slice aliases, or do we compare MergeMemNodes?
08-05-2024
I'm curious why the line byte v = UNSAFE.getByte(a, adr); isn't transformed into something like this: byte v = (i % 2 == 0) ? UNSAFE.getByte(aB, adr) : UNSAFE.getByte(aI, adr); Isn't that what "split through phi" optimizations are supposed to do?
08-05-2024
Such calls are memory barriers.
08-05-2024
Isn't a bottom[] load/store similar to the memory effects of a non-inlined call? After the call, any array that could have escaped could have been read or written by the call. How do we handle that without a recompilation?
08-05-2024
ILW = Incorrect result of C2 compiled code, old issue; edge case; reproducible with Unsafe, no workaround but disable compilation of affected method = HLM = P3
06-05-2024
Discussed it with [~thartmann]: We see 2 options: - add barriers before/after every "bottom[]" load/store. Probably expensive, i.e. prevents other optimizations. - if we ever find a "bottom[]" load/store, we must make sure that all array accesses go to "bottom[]" slice. This probably requires a re-compiltion, i.e. we set a flag and recompile. This may cost more compile-time - though we only have to go through parsing with the first compilation and not optimize at all. If "bottom[]" is relatively rare, this should be ok.
06-05-2024
The "flatten_alias_type" method is called from "Compile::find_alias_type", where we either find an existing alias type with existing alias_idx, or we allocate a new alias_idx. And this is called from "C->alias_type".
29-04-2024
I think I tracked it down to: const TypePtr *Compile::flatten_alias_type( const TypePtr *tj ) const Not every type has its own slice. For example, for instance klass, we make sure that we take the most super class, see: ciInstanceKlass* ciInstanceKlass::get_canonical_holder(int offset) This makes sure if we have some base-class A, and a derived class B, these do not end up on separate slices, because an A could turn out to be a B. But for arrays, such logic seems to be missing, at least we don't seem to care that a "byte[]" could also be a "bottom[]". If the alias type is not the same for these, then they will be assigned separate alias_idx, and that puts the loads/stores onto separate slices, i.e. we lose the memory-edges. That can lead to wrong scheduling, and in the end to wrong results. In "Compile::flatten_alias_type", there is some special case logic, like for example we make sure that if "ta->elem() == TypeInt::BOOL", i.e. if we have a "boolean[]", we instead use the "byte[]" type, which puts those two types on the same slice. The tricky thing: it is nice to have "int[]" and "byte[]" on separate slices. But as soon as there is a "bottom[]", we cannot really separate these any more. I'm not sure how to deal with this.
29-04-2024
Quick analysis: image_0.png ./java --add-modules java.base --add-exports java.base/jdk.internal.misc=ALL-UNNAMED --add-exports java.base/jdk.internal.util=ALL-UNNAMED -Xbatch -XX:CompileCommand=compileonly,Test::test* -XX:-UseSuperWord -XX:+TraceLoopOpts -XX:LoopMaxUnroll=2 Test.java We see that the LoadB have no dependency on the StoreB. But since we "store forward", there should be such a dependency! And this happens as soon as we have at least LoopMaxUnroll=2.
26-04-2024
Note: I think this is a general bug, as well as a SuperWord bug. And while this may be a very old bug, it will increase in relevance, as MemorySegments are probably going to be used more and more, and such mixed array-element-type use-cases could come up, they would be rather likely. On old JDK versions, one had to explicitly use Unsafe on an object that could be proven to be an array, but not its element-type. That is a rather unlikely code shape.
25-04-2024