RFR: 8232084: HotSpot build failed with GCC 9.2.1

[serguei.spitsyn at oracle.com]

Hi Yasumasa,


On 10/10/19 21:33, Yasumasa Suenaga wrote:
> Hi Serguei,
>
> On 2019/10/11 12:38, serguei.spitsyn at oracle.com wrote:
>> Hi Yasumasa,
>>
>> If this problem is local then I'd prefer variant A as this solution 
>> is local too.
>> If this problem is observed in other places then variant C would be 
>> better.
>
> This problem might occur in other places in the future.
> In fact we can see warnings which have not warned in GCC 8.
>
> At least Ioi pointed out it in [1].
> I guess it is caused by static analysis of GCC - tracing memory 
> allocation, str length, and copy operation.
> It will be improved as Kim pointed [2].

Okay, thanks.
Yes, I see other emails on this.
So, disabling these GCC 9.2.* warnings looks reasonable.
Ioi even considered to avoid supporting gcc 9.2 until these issues have 
been fixed.
Now, it needs to be reviewed by hotspot-dev and build-dev as Chris 
suggested.

Thanks,
Serguei

> Thanks,
>
> Yasumasa
>
>
> [1] 
> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029565.html
> [2] 
> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029567.html
>
>
>> Thanks,
>> Serguei
>>
>>
>> On 10/10/19 18: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.
>>>>
>>