RFR: 8232084: HotSpot build failed with GCC 9.2.1

[David Holmes]

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