JDK-8337066 : Repeated call of StringBuffer.reverse with double byte string returns wrong result
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 11,17,21,24
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2024-07-24
  • Updated: 2024-11-06
  • Resolved: 2024-10-08
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 11 JDK 17 JDK 21 JDK 23 JDK 24
11.0.26-oracleFixed 17.0.14-oracleFixed 21.0.6-oracleFixed 23.0.2Fixed 24 b19Fixed
Related Reports
Relates :  
JDK-8242115 - C2 SATB barriers are not safepoint-safe
Relates :  
JDK-8343687 - [17u] TestAntiDependencyForPinnedLoads requires UTF-8
Relates :  
JDK-8341831 - PhaseCFG::insert_anti_dependences asserts with "no loads"
Description
When calling StringBuffer/StringBuilder.reverse with 2byte string repeatedly, the API returns wrong result.
Comments
Fix request [17u,21u] I backport this for parity with 17.0.14-oracle,21.0.6-oracle from 23. Simple change, but typical C2 risk. Related issues require no follow-up backport. Clean backport. Test passes but does not reproduce the bug. SAP nightly testing passed.
02-11-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk17u-dev/pull/3021 Date: 2024-10-31 19:56:27 +0000
31-10-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk21u-dev/pull/1116 Date: 2024-10-31 19:56:16 +0000
31-10-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk23u/pull/187 Date: 2024-10-16 20:02:03 +0000
16-10-2024
[jdk23u-fix-request] Critical for jdk11u (for later JDKs more of a defensive fix), customer reported issue leading to incorrect execution. Fix is medium risk. Applies with some minor tweaking. Tier1-3 passed.
16-10-2024
Changeset: 7eab0a50 Branch: master Author: Igor Veresov <iveresov@openjdk.org> Date: 2024-10-08 23:21:44 +0000 URL: https://git.openjdk.org/jdk/commit/7eab0a506adffac7bed940cc020e37754f0adbdb
08-10-2024
[~iveresov] So you can reproduce the TestAlignVectorFuzzer failure with mainline? Could you please file a separate bug for this?
30-09-2024
I ran TestAlignVectorFuzzer a bunch of times during the weekend and this failure mode is preexisting. No caused by my change. So everything is clean.
29-09-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/21222 Date: 2024-09-27 16:02:29 +0000
27-09-2024
So, the reason this is happening is that insert_anti_dependencies() assumes that the memory input for a load is the root of the memory subgraph that it has to search for the possibly conflicting store. Usually this is true if we run all the memory optimizations but with pinned we don't. As a result we may end up with the graph like the one in the attached graph.pdf. The solution, I think, could be to walk up the memory chain of the load, skipping MergeMems, in order to get to the real root and then run the precedence edge insertion algorithm from there. Suggested fix: diff --git a/src/hotspot/share/opto/gcm.cpp b/src/hotspot/share/opto/gcm.cpp index d940717dd3..b76ea00ba2 100644 --- a/src/hotspot/share/opto/gcm.cpp +++ b/src/hotspot/share/opto/gcm.cpp @@ -669,6 +669,15 @@ Block* PhaseCFG::insert_anti_dependences(Block* LCA, Node* load, bool verify) { // The anti-dependence constraints apply only to the fringe of this tree. Node* initial_mem = load->in(MemNode::Memory); + if (load_alias_idx >= Compile::AliasIdxRaw) { + while (initial_mem->is_MergeMem()) { + MergeMemNode* mm = initial_mem->as_MergeMem(); + Node* p = mm->memory_at(load_alias_idx); + if (p != mm->base_memory()) { + initial_mem = p; + } else break; + } + } worklist_store.push(initial_mem); worklist_visited.push(initial_mem); worklist_mem.push(NULL);
17-09-2024
It seems that a load is sunk after a store in a violation of the correct memory dependency for some reason. GCM/LCM? Will need to debug it further... Good: 0e0 B12: # B22 B13 <- B11 Freq: 4.9954 0e0 movzwl RBX, [R8 + #14 + R11 << #1] # ushort/char 0e6 movzwl R10, [R8 + #16 + RCX << #1] # ushort/char 0ec movw [R8 + #14 + R11 << #1], R10 # char/short 0f2 movw [R8 + #16 + RCX << #1], RBX # char/short Bad: e0 B12: # B22 B13 <- B11 Freq: 4.99539 0e0 movzwl R10, [R8 + #16 + RCX << #1] # ushort/char 0e6 movw [R8 + #14 + R11 << #1], R10 # char/short 0ec movzwl RBX, [R8 + #14 + R11 << #1] # ushort/char 0f2 movw [R8 + #16 + RCX << #1], RBX # char/short
06-08-2024
[~iveresov] could you please have a look?
29-07-2024
C2 regression appears to be triggered by the backport of JDK-8242115.
27-07-2024