RFR: 8232084: HotSpot build failed with GCC 9.2.1

[Chris Plummer]

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.
>>