On Fri, 28 Nov 2025 15:49:25 GMT, Marc Chevalier <[email protected]> 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 ... > > Marc Chevalier has updated the pull request incrementally with one additional > commit since the last revision: > > fewer macro Happy as can be. ------------- Marked as reviewed by mhaessig (no project role). PR Review: https://git.openjdk.org/valhalla/pull/1764#pullrequestreview-3519457171
