On Fri, 14 Jul 2023 03:08:50 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Please review this fix to correctly read a long value in the runtime >> constant pool. Details are mentioned in the issue [0]. >> As an example, before this fix the long value generated by SA's dumpclass >> for java.lang.String.serialVersionUID was: >> >> >> private static final long serialVersionUID; >> descriptor: J >> flags: (0x001a) ACC_PRIVATE, ACC_STATIC, ACC_FINAL >> ConstantValue: long 2050732866l >> >> >> After this fix value of java.lang.String.serialVersionUID is: >> >> >> private static final long serialVersionUID; >> descriptor: J >> flags: (0x001a) ACC_PRIVATE, ACC_STATIC, ACC_FINAL >> ConstantValue: long -6849794470754667710l >> >> >> Correct value as obtained from original java.lang.String is >> `-6849794470754667710l`. >> >> Testing: tests under `serviceability/sa` and `sun/tools/jhsdb` are passing. >> >> [0] https://bugs.openjdk.org/browse/JDK-8311971 > > This raises a few questions for me. > > First, what is it about constructing the long from two ints that is incorrect? > > Second, the fact we have `VM.buildLongFromIntsPD` suggests this is the > intended way to do things. Why do we also have `Address.getJLongAt()`? Do we > not actually need `VM.buildLongFromIntsPD`? Is its other use in the code in > ./jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/StackValueCollection.java > also incorrect? > > And third, how can it be that we seemingly have no test for this??? @dholmes-ora Could this code precede the 64-bit version of java? It would certainly make sense for 32-bit when a 64-bit value needs to be cobbled together by reading two slots. The function precedes the initial OpenJDK load, so I cannot check. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14855#issuecomment-1636171797
