RFR: 8311971: SA's ConstantPool.java uses incorrect computation to read long value in the constant pool

[Thomas Stuefe]

On Fri, 14 Jul 2023 03:08:50 GMT, David Holmes <dholmes at 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