RFR 8156073 : 2-slot LiveStackFrame locals (long and double) are incorrect (updated)

[Remi Forax]

Hi Brent,
in LiveStackFrame, PrimitiveSlot.size() should be public.
The other solution is to see PrimitiveValue as a technical factorisation of code, not something users should know, so it should be a public class but a package-private abstract class and PrimitiveSlot32/PrimitiveSlot64 should be public (so you can try to make them value types in 10).

in LiveStackFrameInfo, the two flags MODE_* are not declared final.

cheers,
Rémi

----- Mail original -----
> De: "Brent Christian" <brent.christian at oracle.com>
> À: "hotspot-dev" <hotspot-dev at openjdk.java.net>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Cc: "Fabio Tudone (fabio at paralleluniverse.co)" <fabio at paralleluniverse.co>
> Envoyé: Vendredi 27 Janvier 2017 02:07:09
> Objet: RFR 8156073 : 2-slot LiveStackFrame locals (long and double) are	incorrect (updated)

> Hi,
> 
> Please review my updated approach for fixing 8156073 ("2-slot
> LiveStackFrame locals (long and double) are incorrect") in the
> experimental portion of the StackWalker API, added in JDK 9.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8156073
> Webrev: http://cr.openjdk.java.net/~bchristi/8156073/webrev.03/
> 
> (For reference, the earlier RFR thread is here:[1].)
> 
> We've concluded that we can't get enough primitive type info from the VM
> to use the current fine-grain *Primitive classes in LiveStackFrameInfo,
> so those classes have been removed for now.  I've simplified down to
> just a PrimitiveSlot32 for 32-bit VMs, and PrimitiveSlot64 for 64-bit VMs.
> 
> We also do not attempt to interpret a primitive's type based on the
> slot's contents, and instead return the slot's full contents for every
> primitive local.  This more accurately reflects the underlying layout of
> locals in the VM (versus the previous proposed
> LiveStackFrame.getLocals() behavior of having 64-bit behave like 32-bit
> by splitting long/double values into two slots).  On the 64-bit VM, this
> returns 64-bits even for primitive local variables smaller than 64-bits
> (int, etc).
> 
> The upshot is that we now return the entire value for longs/doubles on
> 64-bit VMs.  (The current JDK 9 only reads the first 32-bits.)
> 
> I also now indicate in LiveStackFrameInfo.toString() if the frame is
> interpreted or compiled.
> 
> On the testing front:
> In the interest of simplifying LiveStackFrame testing down into a single
> test file under jdk/test/, I chose not to add the additional test case
> from Fabio Tudone [2,3] (thanks, Fabio!), but instead incorporated some
> of those testing ideas into the existing test.  Testing is a bit relaxed
> versus the previously proposed test case; in particular it does not
> enforce endianness.
> 
> I also removed the more simplistic CountLocalSlots.java test, given that
> the updated LocalsAndOperands.java will now better cover testing -Xcomp.
>  On the hotspot side, I had to update the 8163014 regtest.
> 
> Automated test runs have looked fine so far.
> 
> Thanks,
> -Brent
> 
> 1.
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/042979.html
> 2.
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-June/041666.html
> 3. https://bugs.openjdk.java.net/browse/JDK-8158879