RFR(XXS) 8249087 Symbol constructor unnecessarily initializes _body[0]

Florian Weimer fweimer at redhat.com
Tue Jul 21 10:43:17 UTC 2020


* Ioi Lam:

> On 7/20/20 11:12 PM, Florian Weimer wrote:
>> * Ioi Lam:
>>
>>> Hi please review this very simple fix:
>>>
>>> diff -r 19f26d72a8d0 src/hotspot/share/oops/symbol.cpp
>>> --- a/src/hotspot/share/oops/symbol.cpp    Mon Jul 20 14:24:19 2020 -0700
>>> +++ b/src/hotspot/share/oops/symbol.cpp    Mon Jul 20 17:11:57 2020 -0700
>>> @@ -51,8 +51,11 @@
>>>   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
>>>     memcpy(_body, name, length);
>>> +  // For symbols of length 0 and 1: _body[0] (and _body[1]) are
>>> uninitialized and may
>>> +  // contain random values, which will only be read by
>>> Symbol::identity_hash(),
>>> +  // which would tolerate such randomness. These values never change
>>> during the lifetime
>>> +  // of the Symbol.
>>>   }
>> Won't this still trip memory debuggers?  Symbol::identity_hash() implies
>> that the result is eventually used in a conditional operation (a hash
>> comparison perhaps).  If it's possible one day to run Hotspot under
>> valgrind, this would result in false positives.
>
> Are you saying that valgrind will modify uninitialized memory
> periodically after the constructor has returned, and thus will cause
> Symbol::identity_hash() to return a different value?

No, it flags conditional branches that (indirectly) depend on
uninitialized memory.  But the chosen program path is deterministic.
Intentionally uninitialized values add clutter to the diagnostic output.

Things break for real if the compiler can observe that uninitialized
memory is read and treats this as an indicator that this execution path
is not taken, although compiler stories vary in this area, and
especially for byte access.

Thanks,
Florian



More information about the hotspot-runtime-dev mailing list