Avoid some GCC 10.X warnings in HotSpot

Koichi Sakata sakatakui at oss.nttdata.com
Thu Jul 9 00:55:35 UTC 2020


Thank you for all your help.

Koichi

On 2020/07/09 5:13, Kim Barrett wrote:
>> On Jul 8, 2020, at 2:42 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>
>>
>>
>> On 7/8/20 11:32 AM, Ioi Lam wrote:
>>>
>>>
>>> On 7/8/20 11:04 AM, Kim Barrett wrote:
>>>>> On Jul 8, 2020, at 8:35 AM, Koichi Sakata <sakatakui at oss.nttdata.com> wrote:
>>>>>
>>>>> I fixed my patch because it had unnecessary code that was pointed before.
>>>>> I would appreciate if anyone could sponsor this patch.
>>>
>>> I can sponsor the patch. I un-edited the line that assigns _body[0] to minimize the delta.
>>>
>>> diff -r c29c9012c0ed src/hotspot/share/oops/symbol.cpp
>>> --- a/src/hotspot/share/oops/symbol.cpp    Tue Jul 07 23:11:13 2020 -0700
>>> +++ b/src/hotspot/share/oops/symbol.cpp    Wed Jul 08 11:25:48 2020 -0700
>>> @@ -52,9 +52,7 @@
>>>     _hash_and_refcount = pack_hash_and_refcount((short)os::random(), refcount);
>>>     _length = length;
>>>     _body[0] = 0;  // in case length == 0
>>> -  for (int i = 0; i < length; i++) {
>>> -    byte_at_put(i, name[i]);
>>> -  }
>>> +  memcpy(_body, name, length);
>>>   }
>>>
>>
>> Here's the webrev. Is everyone OK with it?
>>
>> 8247818: GCC 10 warning stringop-overflow with symbol code
>> Reviewed-by: kbarrett, iklam
>> Contributed-by: sakatakui at oss.nttdata.com
>>
>> http://cr.openjdk.java.net/~iklam/jdk16/8247818-symbol-gcc-warning.v01/
> 
> Looks good.
> 
>>>
>>>> What about this unaddressed comment?
>>>>
>>>>> On Jul 7, 2020, at 10:36 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>>>> However, the first two elements of _body are used by identity_hash().
>>>>> That seems like a possible reason to force initialization of both
>>>>> elements, which currently isn't done for length == 1.  But maybe it
>>>>> doesn't matter that identity_hash isn't consistent between processes,
>>>>> in which case forcing the initialization of _body[0] also shouldn't
>>>>> be needed.
>>>> I think it’s a waste to initialize _body[0] or a bug to not initialize _body[1].
>>>> I’ve no idea which.
>>>>
>>> I think this should be done in a separate RFE. I filed
>>> https://bugs.openjdk.java.net/browse/JDK-8249087
>>> Symbol constructor unnecessarily initializes _body[0]
> 
> OK.
> 


More information about the hotspot-runtime-dev mailing list