On Fri, 14 Jul 2023 17:33:59 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> 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. @tstuefe I don't think this pre-dates 64-bit as the change was made in August 2002. The irony here is that the new code in the PR is what we originally had in August 2002 until we had to fix a "jlong unaligned access" issue! At that point I think someone simply got confused about how classfile parsing needs to interpret the `CONSTANT_Long_info` when filling out the constant pool entry, versus reading back that existing CP entry. As noted above this will only fail on little-endian when the constant is larger than 32-bits, and if the unused slot had a non-zero value. Anyway still extremely surprising that this error has never been noticed. I have to wonder when/where this code actually gets used in practice? I think we need a follow-up RFE to get rid of `buildLongFromIntsPD` and any other dead code. Thanks. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14855#issuecomment-1637299720
