RFR: 8232084: HotSpot build failed with GCC 9.2.1

[serguei.spitsyn at oracle.com]

Hi Yasumasa,

I'm Okay with both A and B-1.
Personally, I like the simplicity of A but understand this issue is more 
broad.

Thanks,
Serguei


On 10/11/19 20:22, Yasumasa Suenaga wrote:
> Hi,
>
> I updated Plan B-1 (#pragma) due to the comment from Chris [1].
>
>
>   A. Use memcpy()
>        http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
>
>   B-1. Use #pragma
> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma.03/
>
>
> I dropped Plan C (disable stringop-truncation in globally) from the 
> solutions
> because Kim pointed this warning found out some actual bugs [2].
>
>
> Thanks,
>
> Yasumasa
>
>
> [1] 
> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029586.html
> [2] 
> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029590.html
>
>
> On 2019/10/11 22:38, Yasumasa Suenaga wrote:
>> Hi,
>>
>> Christos commented to B-1. Thanks!
>> clang defines __GNU_C__ , but stringop-truncation is not supported.
>>
>> So I updated Plan B-1. It works fine on my Fedora30 box.
>>
>>
>>    A. Use memcpy()
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
>>
>>    B-1. Use #pragma
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma.02/
>>
>>    C. Set -Wno-stringop-truncation in globally
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.C/
>>
>>
>> Of course I will push the change to submit repo before sending review 
>> request.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2019/10/11 15:55, Yasumasa Suenaga wrote:
>>> Hi,
>>>
>>> Thanks for a lot advises!
>>>
>>> We have following solutions for this issue.
>>> I'd like to send RFR again with much consented patch in early next 
>>> week.
>>>
>>>
>>>    A. Use memcpy()
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.02/
>>>
>>>    B-1. Use #pragma
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.B1-pragma/
>>>
>>>    C. Set -Wno-stringop-truncation in globally
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.C/
>>>
>>>
>>> If we fix with C, I will send RFR to hotspot-dev and build-dev.
>>> Plan C also fixes other stringop-truncation problems such as 
>>> JDK-8220074.
>>> Thus it affects all of JDK code, but it would be useful if 
>>> stringop-truncation
>>> should be disabled in JDK build process.
>>>
>>>
>>> Comments are welcome.
>>>
>>> Yasumasa
>>>
>>>
>>> On 2019/10/11 10:34, 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.
>>>>>