RFR: 8232084: HotSpot build failed with GCC 9.2.1

[Christos Zoulas]

At NetBSD we decided to change the Makefiles because  we compile with both clang and gcc
and clang does not have those warnings and it complains about the pragmas.

christos

> On Oct 11, 2019, at 2:54 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Yes I prefer:
> 
> D. Use #pragma ... at the callsite to disable this particular diagnostic.
> 
> over changing the makefiles.
> 
> David
> 
> On 11/10/2019 3:37 pm, Ioi Lam wrote:
>> 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.
>>>>> 
>>> 
>>>