On Fri, 14 Jul 2023 03:08:50 GMT, David Holmes <[email protected]> wrote:
> First, what is it about constructing the long from two ints that is incorrect?
The incorrect part is fetching the ints from index and index+1. This doesn't
work for 64-bit platforms because the single entry in runtime constant pool is
large enough to store the long value, so the entry at index+1 is not used and
is invalid.
In ClassFileParser::parse_constant_pool_entries():
case JVM_CONSTANT_Long: {
// some code
const u8 bytes = cfs->get_u8_fast();
cp->long_at_put(index, bytes);
index++; // Skip entry following eigth-byte constant, see JVM book p.
98
The existing code that uses VM.buildLongFromIntsPD() would have worked if each
constant pool entry is of 4 bytes. So 32-bit systems it wouldn't be a problem.
>From my understanding getJLongAt() should work for both 32 and 64 bit systems.
> 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?
It seems both would produce the same result i.e. getJLongAt(0x1000) ==
VM.buildLongFromIntsPD(getIntAt(0x1004), getIntAt(0x1000));
> Is its other use in the code in
> ./jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/StackValueCollection.java
> also incorrect?
Not sure about that. Looking at the code for StackValueCollection.longAt() it
would depend on whether any value is stored in slot+1. If the slots in the
stack behave the same as the runtime constant pool, then it might be incorrect.
Also there are no users of StackValueCollection.longAt() method, so its a dead
code as of now.
> how can it be that we seemingly have no test for this???
As per my knowledge we have very basic tests that just verify that the
classfile generated by SA is parseable by javap. There is not test that checks
for equality of each individual component of the classfile.
I have a test [0] that can be a good proxy for ensuring the generated
classfiles are in a use-able shape because this test has helped me in
uncovering quite a few issues including this one and another one in handling of
invokedynamic bytecodes which is being fixed here
https://github.com/openjdk/jdk/pull/14852 (although it was reported through
another channel).
[0] https://bugs.openjdk.org/browse/JDK-8311101
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14855#issuecomment-1636011869
