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

[Ioi Lam]

On 2/13/18 4:30 PM, coleen.phillimore at oracle.com wrote:
>
>
> On 2/13/18 6:37 PM, Ioi Lam wrote:
>>
>>
>> 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?
>
> I don't think so.  I never used it.  It might be for debugging short 
> and ints?  I think it's useless if you want to remove it, I'll review 
> it quickly.
>>
>>> 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.
>
> I saw this line afterwards, and can't guess why it's there.
>
> Your patch is fine if you want to push it.
>

Thanks Coleen, I'll push it as is.

- Ioi
> Coleen
>>
>> 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) {
>>>>
>>>
>>
>