[lworld] RFR: 8371993: [lworld] Aarch64: save bad values instead of rfp and lr above the extension space

[Manuel Hässig]

On Thu, 27 Nov 2025 21:04:28 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

> Related lore: https://github.com/openjdk/valhalla/pull/1540 & https://github.com/openjdk/valhalla/pull/1751. Please, go check those up if you miss the context.
> 
> As we established in [JDK-8367151](https://bugs.openjdk.org/browse/JDK-8367151)/https://github.com/openjdk/valhalla/pull/1751, LR2 and FP2 are not reliable (resp. not patched for deopt and not known by deopt code, not updated by GC). Since reading them is probably fine, but maybe not, it is risky to leave reasonable value there. In debug, I suggest we store a magic but recognizable value to make more obvious one read the wrong copy, actually, we don't really need LR2 and FP2 to contain lr and rfp, we mostly need it to make space between the stack extension and the proper frame to pretend it is like a scalarized call.
> 
> What I propose here is similar to zapping unused space freed by the GC: when `ZapUnusedHeapArea`, that is `trueInDebug`, we zap the heap not to read something good-looking when we have a wrong pointer.
> 
> https://github.com/openjdk/valhalla/blob/1144cb4c5183c69a74aa0211f7ead5ac388ee41d/src/hotspot/share/runtime/globals.hpp#L482-L483
> 
> https://github.com/openjdk/valhalla/blob/1144cb4c5183c69a74aa0211f7ead5ac388ee41d/src/hotspot/share/gc/serial/serialFullGC.cpp#L371-L373
> 
> What I'm not sure about:
> - should I make the `save_fake_rfp_lr` an argument also in product build, just unused, to avoid the slightly ugly `NOT_PRODUCT(COMMA save_fake_rfp_lr)`?
> - how should I name `save_fake_rfp_lr`? I think it is clear, but not great.
> - I've introduced a new value to zap registers, that looks special, but that is not what `badHeapWord` to avoid confusion. Any opinion on the variable name and the magic value? I intend to reuse it to zap other registers (the caller-saved ones).
> - is there an easier way to write a 64-bit immediate in a register in Aarch64?! I found movptr, but it asserts the immediate is an address and so, that it is actually only 48-bits. I've wrote my own, because I couldn't find another example pointing me to an existing implementation of that, but I've probably missed something.
> 
> I've also elected not to make a flag but just to make mandatory to write these magic value in debug mode. I don't think it's worth a flag, as I see little benefit in not doing it: the performance cost is surely very marginal. Also, adding a flag, even develop, also implies some commitment (might end up in some tests or scripts), make sure it works to turn it on and off... Not terrible complications, but still ...

That is a very useful continuation of this story arch.

> how should I name `save_fake_rfp_lr`?

`mangle_unused_rfp_lr`, `zap_unextended_rfp_lr` and permutations of those come to mind.

> is there an easier way to write a 64-bit immediate in a register in Aarch64?

You could load a constant from the constant pool (`ldr xTmp, =imm64`) if you want to trade off code size vs. having a load from memory. Otherwise, I think that your version is the best you can do.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5906:

> 5904:   if (framesize < ((1 << 9) + 2 * wordSize)) {
> 5905:     sub(sp, sp, framesize);
> 5906: #ifndef PRODUCT

Are you sure you want this in the `optimized` build? Since that is used for profiling performance, I would tend towards using `ASSERT`.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5911:

> 5909:       movk(rscratch1, (badRegWordVal >> 16) & 0xffff, 16);
> 5910:       movk(rscratch1, badRegWordVal & 0xffff, 32);
> 5911:       movk(rscratch1, (badRegWordVal >> 16) & 0xffff, 48);

This should go in its own function in the macro assembler.

src/hotspot/share/utilities/globalDefinitions.hpp line 1047:

> 1045: const intptr_t badDispHeaderDeopt = 0xDE0BD000;             // value to fill unused displaced header during deoptimization
> 1046: const intptr_t badDispHeaderOSR   = 0xDEAD05A0;             // value to fill unused displaced header during OSR
> 1047: const juint    badRegWordVal      = 0xCAFEFADE;             // value used to zap registers

Since you asked: keeping the `0xDEAD` prefix would make sense. Otherwise, I have no opinion, because there is no good way to spell "R" in hex.

-------------

Changes requested by mhaessig (no project role).

PR Review: https://git.openjdk.org/valhalla/pull/1764#pullrequestreview-3517919380
PR Review Comment: https://git.openjdk.org/valhalla/pull/1764#discussion_r2570810116
PR Review Comment: https://git.openjdk.org/valhalla/pull/1764#discussion_r2570816195
PR Review Comment: https://git.openjdk.org/valhalla/pull/1764#discussion_r2570805892