RFR: 8232084: HotSpot build failed with GCC 9.2.1

[Ioi Lam]

For diagnosticArgument.cpp, I would prefer adding inline #pragma at only 
the sites affected (as suggested by David Holmes), with a comment about 
why this is necessary.

But if others decide to go with other solutions, that's fine with me, 
too, but please remember to add a comment :-)

Thanks
- Ioi

On 10/10/19 6:50 PM, Chris Plummer wrote:
> These solutions all have their merits and their warts, but we have to 
> pick something. I'm ok with with A or C. For B I'd rather you instead 
> used the #pragma at the warning site. If you go with (C), the review 
> should go out to all of hotspot-dev and build-dev.
>
> Chris
>
> On 10/10/19 6:34 PM, Yasumasa Suenaga wrote:
>> Hi,
>>
>> I want to get conclusion of this discussion.
>>
>> I understand the fix of macroAssembler_x86.hpp is ok, but we have not 
>> yet had conclusion
>> how we should fix diagnosticArgument.cpp .
>>
>> I think we can fix diagnosticArgument.cpp as following:
>>
>>
>>   A. Use memcpy()
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
>>
>>   B. Add -Wno-stringop-truncation to 
>> make/hotspot/lib/JvmOverrideFiles.gmk
>>        This option will be added diagnosticArgument.cpp only.
>>
>>   C. Set -Wno-stringop-truncation in globally
>>        make/hotspot/lib/CompileJvm.gmk
>>
>>
>> I prefer to fix like A because it affects minimally.
>> Some issues might be found out by stringop-truncation in future.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2019/10/11 5:54, Kim Barrett wrote:
>>>> On Oct 10, 2019, at 3:03 AM, David Holmes <david.holmes at oracle.com> 
>>>> wrote:
>>>>
>>>> On 10/10/2019 4:50 pm, Chris Plummer wrote:
>>>>>  From JBS:
>>>>> /home/ysuenaga/OpenJDK/jdk/src/hotspot/share/services/diagnosticArgument.cpp:154:14: 
>>>>> warning: 'char* strncpy(char*, const char*, size_t)' output 
>>>>> truncated before terminating nul copying as many bytes from a 
>>>>> string as its length [-Wstringop-truncation]
>>>>>    154 | strncpy(buf, str, len);
>>>>>        | ~~~~~~~^~~~~~~~~~~~~~~
>>>>> I assume this means that in all cases the "len" value is seen to 
>>>>> be derived from strlen, and therefore strncpy is always copying 
>>>>> one byte short of \0, and this is most likely not what the user 
>>>>> wants. I seem to
>>>>
>>>> Yes but we then explicitly set the NULL at buf[len] which is the 
>>>> expected/required pattern for this.
>>>>
>>>>> recall another recent similar fix that was done by switching to 
>>>>> using memcpy instead.
>>>>> Here's a discussion of interest, also suggesting memcpy:
>>>>> https://stackoverflow.com/questions/50198319/gcc-8-wstringop-truncation-what-is-the-good-practice 
>>>>>
>>>>
>>>> Seems to me that strncpy and memcpy are semantically equivalent 
>>>> here so all this does is avoid gcc's over zealous warnings. I'm 
>>>> inclined to use the:
>>>>
>>>> #pragma GCC diagnostic push
>>>> #pragma GCC diagnostic ignored "-Wstringop-truncation"
>>>>
>>>> solution.
>>>>
>>>> YMMV.
>>>
>>> We've run into and discussed problems with -Wstringop-truncation
>>> before.  (See discussions of JDK-8214777 and JDK-8223186.) This is a
>>> relatively recent warning option (introduced in gcc8, and included in
>>> -Wall), and seems to have a considerable bug tail:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88781
>>> A metabug for -Wstringop-truncation, currently with 16 open and 10
>>> resolved associated bugs.
>>>
>>> I'm not a fan of replacing correct and idiomatic uses of strncpy with
>>> strcpy or memcpy.  I've suggested in the past that we should turn off
>>> this warning while it is so buggy.
>>>
>
>