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.christ...@oracle.com>
> À: "hotspot-dev" <hotspot-...@openjdk.java.net>, "core-libs-dev"
> <core-libs-dev@openjdk.java.net>
> Cc: "Fabio Tudone (fa...@paralleluniverse.co)" <fa...@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
