RFR(XXS) 8197857 fieldDescriptor prints incorrect 32-bit representation of compressed oops

[Ioi Lam]

On 2/13/18 12:30 PM, coleen.phillimore at oracle.com wrote:
>
> This looks good but this is very odd output.  I don't know why we 
> print this.  I wouldn't object if it were removed.
>
I guess it's useful for someone debugging issues related to compressed oops?

> Otherwise, I hate to do this to a trivial change but would this also 
> print this better?
>
>   // Print a hint as to the underlying integer representation. This 
> can be wrong for
>   // pointers on an LP64 machine
>   if (ft == T_LONG || ft == T_DOUBLE LP64_ONLY(|| (!UseCompressedOops 
> && !is_java_primitive(ft))) ) {
>     st->print(" (%x %x)", obj->int_field(offset()), 
> obj->int_field(offset()+sizeof(jint)));
>   } else if (as_int < 0 || as_int > 9) {
>     st->print(" (%x)", as_int);
>   }
>

This would make the code even harder to read than it already is.

Also, the (as_int < 0 || as_int > 9) is useful only for 32-bit pointers 
and numerical values. For CompressedOops, I guess it's possible to have 
a value 8. This is probably not a big deal, but I don't want to have 
code that's theoretically incorrect.

Thanks
- Ioi



>
> Thanks,
> Coleen
>
> On 2/13/18 12:28 PM, Ioi Lam wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8197857
>>
>>
>> When UseCompressedOops is enabled for 64-bit VMs, 
>> fieldDescriptor::print_on_for
>> prints two 32-bit integers for each object field. E.g.
>>
>>  - 'asTypeCache' 'Ljava/lang/invoke/MethodHandle;' @24 NULL (0 8221b591)
>>  - final 'argL0' 'Ljava/lang/Object;' @28 a 
>> 'LambHello$$Lambda$1'{0x00000004110dac88} (8221b591 1)
>>
>> However, compressed oops occupy the space of only a single 32-bit
>> integer, so the superfluous output is confusing.
>>
>> The above should be printed as
>>
>>  - 'asTypeCache' 'Ljava/lang/invoke/MethodHandle;' @24 NULL (0)
>>  - final 'argL0' 'Ljava/lang/Object;' @28 a 
>> 'LambHello$$Lambda$1'{0x00000004110dac88} (8221b591)
>>
>> Patch:
>> =======================================
>>
>> --- a/src/hotspot/share/runtime/fieldDescriptor.cpp    Mon Feb 12 
>> 09:12:59 2018 -0800
>> +++ b/src/hotspot/share/runtime/fieldDescriptor.cpp    Tue Feb 13 
>> 09:24:26 2018 -0800
>> @@ -201,6 +201,13 @@
>>    }
>>    // Print a hint as to the underlying integer representation. This 
>> can be wrong for
>>    // pointers on an LP64 machine
>> +
>> +#ifdef _LP64
>> +  if ((ft == T_OBJECT || ft == T_ARRAY) && UseCompressedOops) {
>> +    st->print(" (%x)", obj->int_field(offset()));
>> +  }
>> +  else // <- intended
>> +#endif
>>    if (ft == T_LONG || ft == T_DOUBLE LP64_ONLY(|| 
>> !is_java_primitive(ft)) ) {
>>      st->print(" (%x %x)", obj->int_field(offset()), 
>> obj->int_field(offset()+sizeof(jint)));
>>    } else if (as_int < 0 || as_int > 9) {
>>
>