JDK-8343206 : Final graph reshaping should not compress abstract or interface class pointers
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 24
  • Priority: P5
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2024-10-29
  • Updated: 2025-07-04
  • Resolved: 2024-11-04
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 24
24 b23Fixed
Related Reports
Relates :  
JDK-8361211 - C2 generates unencodeable klass constants
Relates :  
JDK-8338526 - Don't store abstract and interface Klasses in class metaspace
Sub Tasks
JDK-8343218 :  
Disable allocating interface and abstract classes in non-class metaspace - Closed
Description
After JDK-8338526 it is no longer legal to encode compressed class pointers for interfaces or abstract classes as they are not stored in the compressed class space any longer.

However, in C2 final graph reshaping, it seems like we perform the following transformation (in pseudo code):

decode(dynamic_narrow_klass) == static_klass
=>
dynamic_narrow_klass == encode(static_klass)

This transformation is no longer valid if static_klass is an interface or abstract class. I can't see that there are any guards for that. I guess the result of the expression should typically be false. Encoding a not encodable klass pointer will result in garbage bits. Most of the time comparing a real compressed class to garbage bits will result in false. But I suppose sometimes you win the garbage lottery and get the wrong result.
Comments
I think I caught a CTW failure related to the same code now: JDK-8361211
01-07-2025
Changeset: 2432c4f8 Branch: master Author: Tobias Hartmann <thartmann@openjdk.org> Date: 2024-11-04 06:27:33 +0000 URL: https://git.openjdk.org/jdk/commit/2432c4f862e66e91c60e75ccc43b376020d80a1f
04-11-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/21784 Date: 2024-10-30 11:38:53 +0000
30-10-2024
ILW = Invalid encoding of compressed class pointers (not an issue in current code), cmp with constant class of abstract or interface type, no workaround = LLH = P5
30-10-2024
I think we should add an assert + bailout in product to be on the safe side. I sent a PR for review.
30-10-2024
This is the problematic optimization: https://github.com/openjdk/jdk/blob/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e/src/hotspot/share/opto/compile.cpp#L3493 I had a look at this and I think it's not an issue in current code. The only way we can get a dynamic narrow class is when loading from an object at oopDesc::klass_offset_in_bytes(): https://github.com/openjdk/jdk/blob/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e/src/hotspot/share/opto/type.cpp#L3522 Comparisons of such loads with a constant class pointer of interface or abstract class type are always folded during GVN: https://github.com/openjdk/jdk/blob/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e/src/hotspot/share/opto/subnode.cpp#L1165 And therefore, the code in Compile::final_graph_reshaping_main_switch will never trigger. To verify, I executed some testing with this patch and we never hit the guarantees: diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index a452d439a54..aa063b4701d 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -3490,6 +3490,8 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f } else if (t->isa_oopptr()) { new_in2 = ConNode::make(t->make_narrowoop()); } else if (t->isa_klassptr()) { + guarantee(!t->is_klassptr()->exact_klass()->is_interface(), "using compressed class for interface"); + guarantee(!t->is_klassptr()->exact_klass()->is_abstract(), "using compressed class for abstract class"); new_in2 = ConNode::make(t->make_narrowklass()); } }
30-10-2024
As a general note, I feel a bit uncomfortable with JDK-8338526. This is the second C2 bug we have discovered completely randomly by just stumbling on this in the code. It makes me wonder how many more of these scary (yet at the time sane) assumptions there are, that any statically known Klass can be encoded. Such assumptions may now yield subtle incorrect results.
29-10-2024