Avoid some GCC 10.X warnings in HotSpot

[Koichi Sakata]

Thank you, John and Kim. I was able to understand that deeply.
I recognize that we can use memcpy in this situation.

I fixed my patch because it had unnecessary code that was pointed before.
I would appreciate if anyone could sponsor this patch.

Thanks,
Koichi

===== PATCH =====
diff -r f0792f0ffce9 src/hotspot/share/oops/symbol.cpp
--- a/src/hotspot/share/oops/symbol.cpp	Tue Jun 23 21:23:00 2020 -0700
+++ b/src/hotspot/share/oops/symbol.cpp	Wed Jul 08 17:51:27 2020 +0900
@@ -50,10 +50,8 @@
  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]);
-  }
+  _body[0] = 0;
+  memcpy(_body, name, length);
  }

  void* Symbol::operator new(size_t sz, int len) throw() {
diff -r f0792f0ffce9 src/hotspot/share/oops/symbol.hpp
--- a/src/hotspot/share/oops/symbol.hpp	Tue Jun 23 21:23:00 2020 -0700
+++ b/src/hotspot/share/oops/symbol.hpp	Wed Jul 08 17:51:27 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();




On 2020/07/08 8:29, Kim Barrett wrote:
>> On Jul 7, 2020, at 12:45 PM, John Rose <john.r.rose at oracle.com> wrote:
>>
>> On Jul 7, 2020, at 7:36 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>>
>>> Using memcpy instead of byte_at_put (and getting rid of byte_at_put)
>>> seems like a good idea to me.
>>
>> We have had problems with memcpy in the long past, and I’m
>> personally still nervous when I see a call to it.  The problem is
>> subtle, and escaped everybody’s notice the first time around,
>> and I’d prefer not to make more bugs of the same kind.
>>
>> What problem?  [… snipped long discussion …]
>>
>> All that said, I have no problem with memcpy being
>> used (though I prefer a locally documented alias!) in
>> the places where it is appearing in our source base.
>> Those places are, yes, private to some construction
>> process, or singly-threaded for some other reason,
>> perhaps a safepoint.  But I think you understand now
>> that I am nervous when I see more and more uses
>> of memcpy in our code, because I think that now
>> it’s just a matter of time before someone concludes
>> that memcpy is the new (old) best way to copy data,
>> and the hard work of codifying our practices in
>> Copy will start to be neglected.  A new programmer
>> in our code base could make the mistake of just
>> reaching for memcpy instead of doing the work of
>> deciding which kind of Copy to use.  And that will
>> eventually cause a difficult-to-detect bug.  Let’s not.
>>
>> — John
> 
> [This is somewhat summarizing an off-email discussion John and I had.]
> 
> It shouldn't be surprising that we have uses of bare memcpy, since we
> don't have Copy::disjoint_bytes. In the particular case at hand,
> there's no issue of word tearing since we aren't copying words, and
> we're not even doing an aligned copy. But I wouldn't object to, and in
> fact would encourage, the use of Copy::disjoint_bytes there if it
> existed. Not having that function effectively denormalizes Copy in
> favor of bare memcpy, making it easy to forget that Copy even exists.
> I think the split between bare memcpy and Copy::conjoint_words in
> HotSpot currently is close to even. And there are a score or so bare
> memmoves.
> 
> I think the places where word tearing is an issue ought to be using
> one of the "atomic" Copy functions.  (And it's not just word tearing
> that can be an issue, as we found with SPARC BIS.  You convinced me
> some time ago that memset_with_concurrent_readers (my fault for that)
> should have been an "atomic" Copy function.  I'll probably deal with
> that RFE soon; it's much easier now that SPARC has been removed.)
> 
> All of this is something of a digression from the change at hand.  In
> the absence of Copy::disjoint_bytes (which this change shouldn't be
> worrying about), I think using memcpy is fine for this change.
>