RFR: 8232084: HotSpot build failed with GCC 9.2.1

[Magnus Ihse Bursie]

On 2019-10-11 15: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/
Please note that if you chose to go with route C, you need to get this 
reviewed on build-dev (as all changes in the makefiles are). The patch 
as currently suggested is not acceptable; the built-in functions for 
testing compiler version needs to be used, as a minimum.

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