> 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 ...
Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision: fewer macro ------------- Changes: - all: https://git.openjdk.org/valhalla/pull/1764/files - new: https://git.openjdk.org/valhalla/pull/1764/files/de1271de..1e913868 Webrevs: - full: https://webrevs.openjdk.org/?repo=valhalla&pr=1764&range=03 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=1764&range=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/valhalla/pull/1764.diff Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1764/head:pull/1764 PR: https://git.openjdk.org/valhalla/pull/1764
