Skip to content

fix(java): avoid JDK 8 ARM Unsafe address miscompilation - #4049

Merged
chaokunyang merged 2 commits into
apache:mainfrom
chaokunyang:jdk8-arm-unsafe-access
Sep 15, 2026
Merged

chaokunyang merged 2 commits into
apache:mainfrom
chaokunyang:jdk8-arm-unsafe-access

Conversation

@chaokunyang

@chaokunyang chaokunyang commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Why?

On Oracle JRE 8u503 ARM64, C2 can miscompile an indexed Unsafe access when the address scale differs from the access width. For example, this reads eight bytes starting at a char index, so the byte offset must use charIndex * 2:

UNSAFE.getLong(chars, CHAR_ARRAY_OFFSET + ((long) charIndex << 1));

The captured machine code instead scales the index by eight (x1 is the array object and w2 is charIndex; the array base offset is 16 in this capture):

add x8, x1, #16
ldr x0, [x8, w2, sxtw #3]  // array + 16 + charIndex * 8

For charIndex = 4, this reads chars[16..19] instead of chars[4..7], which can cause the metadata-string failure in #4017. The same class of failure was reproduced for narrower reads and primitive stores. A single getInt is therefore not inherently a safe replacement.

This PR works around the JDK compiler bug in Fory. The corresponding compiler-side mechanism is documented by JDK-8267689: its OpenJDK 8 patch checks that the address scale matches the memory access width before selecting a scaled addressing mode. This link identifies the matching failure mechanism; it does not establish which patches Oracle 8u503 contains.

What does this PR do?

  • Add the static-final JDK8_ARM gate and route indexed primitive reads/writes in buffers, string helpers, and generated codecs through the split access methods on JDK 8 ARM.
  • Split long accesses into two int accesses, int accesses into two short accesses, and char/short accesses into two byte accesses. Floating-point stores preserve their raw bits. Each pair shares a byte offset and preserves native byte order; existing wire-order conversion remains at the caller.
  • Expand the gate directly in MemoryBuffer, LittleEndian, and PlatformStringUtils so hot callers reach the split methods without another forwarding layer. Other JVMs retain direct Unsafe accesses; generated codecs choose their path during code generation.

How sharing the address avoids the bad instruction

For example, the long read uses the existing pair of native-order int loads:

long first = UNSAFE.getInt(base, offset) & 0xffffffffL;
long second = UNSAFE.getInt(base, offset + 4) & 0xffffffffL;
return NativeByteOrder.IS_LITTLE_ENDIAN
    ? first | (second << 32)
    : (first << 32) | second;

Both loads consume the same indexed address expression. In the verified C2 output, their common address prefix is calculated once into a register, then the loads use fixed displacements. The captured string accessor becomes:

add   x10, x1, w2, sxtw #1  // common address = array + charIndex * 2
ldrsw x11, [x10, #20]       // second four bytes: array base offset + 4
ldr   w10, [x10, #16]       // first four bytes: array base offset
orr   x0, x10, x11, lsl #32 // combine the two halves

The * 2 now executes in the address calculation, while the memory instructions use constant offsets. This avoids the faulty indexed-load form that changed the scale to * 8. OpenJDK's shared-node matching and AArch64's clone_shift_expressions = false explain this code shape.

The split helper and its forwarding wrapper were both inlined in this capture. The workaround relies on the paired address uses, not on retaining a method-call boundary: forwarding the original single wide access still failed after inlining. Manually expanding the platform gate reduces inlining depth; the paired accesses are what avoid the observed miscompilation.

Validation

  • Added scaled read/write regression coverage, a 100,000-call test through the actual string accessor, and assertions that generated codecs select the ARM access methods.
  • On current head, focused tests passed on JDK 21 (114 passed, 3 skipped) and Oracle 8u503 ARM64 (114 passed, 2 skipped); the full Java install, Spotless, and Checkstyle also passed.
  • The original issue reproducer passed in three fresh Oracle 8u503 JVMs on current head. During the initial fix validation, disabling only JDK8_ARM made the same reproducer fail in all three runs. The inlined machine-code capture recorded zero mismatches in 20,000 reads.
  • Runtime evidence covers macOS ARM64. Both endian arithmetic branches were checked, but execution on a big-endian JVM and ARM32 remains unverified. This is a tested workaround for the observed compiler behavior, not a language-level guarantee about every JDK optimizer.

Related issues

Closes #4017

AI Contribution Checklist

  • Substantial AI assistance was used in this PR: yes / no
  • If yes, I included a completed AI Contribution Checklist in this PR description and the required AI Usage Disclosure.
  • If yes, my PR description includes the required ai_review summary and screenshot evidence or equivalent persisted links of the final clean AI review results from both fresh reviewers described in AI_POLICY.md, the Fory-guided reviewer and the independent general reviewer, on the current PR diff or current HEAD after the latest code changes.

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

Apple M4 Pro, JMH 1.37, UserTypeSerializeSuite.fory_serialize and UserTypeDeserializeSuite.fory_deserialize, objectType=STRUCT, bufferType=array, references=false, REGISTER_CLASS=true. The existing STRUCT factory produces 104 primitive fields. Both revisions round-trip it and produce identical 679-byte output.

Compare baseline 677049dee9dcaebcfd528d4fb5db4fb879faf426 with current 75d9cc6bcf1c413ea62e206988e85249f0f4925e, using frozen core JARs and one common benchmark JAR. Each runtime/case has two adjacent baseline/current pairs, one process at a time: -f 1 -wi 3 -i 5 -t 1 -w 2s -r 2s, -Xms512m -Xmx512m, default JIT settings. Values are mean throughput in M ops/s.

Runtime Case Baseline Current Change
Oracle 8u503 ARM64 Serialize 21.022 14.752 -29.82%
Oracle 8u503 ARM64 Deserialize 13.501 8.940 -33.79%
OpenJDK 11.0.31 ARM64 Serialize 20.074 20.105 +0.16%
OpenJDK 11.0.31 ARM64 Deserialize 14.518 14.514 -0.02%

The active JDK 8 ARM workaround still has a material throughput cost. On JDK 11, JDK8_ARM=false and these two cases show no consistent regression: serialization pairs changed -1.09%/+1.52%, and deserialization pairs +4.73%/-4.50%. The two-fork means should not be interpreted as proof of exact zero overhead.

@chaokunyang
chaokunyang merged commit 7720dbb into apache:main Sep 15, 2026
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java] Multiple Fory instances cannot be constructed on Java 8 (Non-ASCII characters in meta string)

2 participants