Avoid some GCC 10.X warnings in HotSpot

[Koichi Sakata]

Hi David,

 > This is in relation to the hotspot part as these issues need to be
 > handled separately. I have filed:
 >
 > https://bugs.openjdk.java.net/browse/JDK-8247818

Thank you, David.I have something to ask you.
Should I send only the other part of the patch (i.e. NetworkInterface.c 
and k_standard.c) to core-lib ML again? I've sent the whole one to 
core-lib before.

 > I'm not really clear on the warning here but this is an area where we
 > trick the compiler somewhat. The _body[] is declared with a size of 2,
 > but when we allocate Symbols we allocate sufficient memory for _body to
 > contain the entire symbol.
 >
 > That said I'm struggling to see how we allocate the additional space
 > needed for the _hash_and_refcount and _length fields ???

I was thinking exactly the same thing. I've learned a lot from Ioi's 
explanation.

 > The check for length==0 introduces more overhead than just always
 > setting _body[0]=0, so there is no need to add it.

I understood that clearly. Thank you for teaching me.

Thanks,
Koichi

On 2020/06/18 10:56, David Holmes wrote:
> Hi Koichi,
> 
> This is in relation to the hotspot part as these issues need to be 
> handled separately. I have filed:
> 
> https://bugs.openjdk.java.net/browse/JDK-8247818
> 
> On 18/06/2020 8:46 am, Koichi Sakata wrote:
>> Hi all,
>>
>> I tried to build OpenJDK fastdebug with GCC 10.1 on Ubuntu 18.04, but 
>> I saw some compiler warnings as follows:
>>
>> In file included from 
>> /home/jyukutyo/code/jdk/src/hotspot/share/classfile/systemDictionary.hpp:31, 
>>
>>                   from 
>> /home/jyukutyo/code/jdk/src/hotspot/share/classfile/javaClasses.hpp:28,
>>                   from 
>> /home/jyukutyo/code/jdk/src/hotspot/share/precompiled/precompiled.hpp:35:
>> In member function 'void Symbol::byte_at_put(int, u1)',
>>      inlined from 'Symbol::Symbol(const u1*, int, int)' at 
>> /home/jyukutyo/code/jdk/src/hotspot/share/oops/symbol.cpp:55:16:
>> /home/jyukutyo/code/jdk/src/hotspot/share/oops/symbol.hpp:130:18: 
>> error: writing 1 byte into a region of size 0 
>> [-Werror=stringop-overflow=]
>>    130 |     _body[index] = value;
>>        |     ~~~~~~~~~~~~~^~~~~~~
> 
> I'm not really clear on the warning here but this is an area where we 
> trick the compiler somewhat. The _body[] is declared with a size of 2, 
> but when we allocate Symbols we allocate sufficient memory for _body to 
> contain the entire symbol.
> 
> That said I'm struggling to see how we allocate the additional space 
> needed for the _hash_and_refcount and _length fields ???
> 
>> I can resolve them with the following patch. I believe it fixes those 
>> potential bugs, so I'd like to contribute it.
>> (Our company has signed OCA.)
>>
>> Thanks,
>> Koichi
>>
>> ===== PATCH =====
>> diff -r 20d92fe3ac52 src/hotspot/share/oops/symbol.cpp
>> --- a/src/hotspot/share/oops/symbol.cpp    Tue Jun 16 03:16:41 2020 +0000
>> +++ b/src/hotspot/share/oops/symbol.cpp    Thu Jun 18 07:08:50 2020 +0900
>> @@ -50,9 +50,10 @@
>>   Symbol::Symbol(const u1* name, int length, int refcount) {
>>     _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]);
>> +  if (length == 0) {
>> +    _body[0] = 0;
> 
> The check for length==0 introduces more overhead than just always 
> setting _body[0]=0, so there is no need to add it.
> 
>> +  } else {
>> +    memcpy(_body, name, length);
>>     }
>>   }
> 
> So you are replacing byte_at_put with a memcpy call. On the surface that 
> seems reasonable, but I have to wonder why we were using the loop in the 
> first place. It may just be historical or it may relate to an alignment 
> issue, or something else. Hopefully someone else (e.g. Coleen :) ) can 
> shed more light here.
> 
> Thanks,
> David
> -----
> 
>> diff -r 20d92fe3ac52 src/hotspot/share/oops/symbol.hpp
>> --- a/src/hotspot/share/oops/symbol.hpp    Tue Jun 16 03:16:41 2020 +0000
>> +++ b/src/hotspot/share/oops/symbol.hpp    Thu Jun 18 07:08:50 2020 +0900
>> @@ -125,11 +125,6 @@
>>       return (int)heap_word_size(byte_size(length));
>>     }
>>
>> -  void byte_at_put(int index, u1 value) {
>> -    assert(index >=0 && index < length(), "symbol index overflow");
>> -    _body[index] = value;
>> -  }
>> -
>>     Symbol(const u1* name, int length, int refcount);
>>     void* operator new(size_t size, int len) throw();
>>     void* operator new(size_t size, int len, Arena* arena) throw();
> 
>