RFR: 8232084: HotSpot build failed with GCC 9.2.1
Yasumasa Suenaga
suenaga at oss.nttdata.com
Sat Oct 12 03:22:53 UTC 2019
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.
>>>>
More information about the hotspot-compiler-dev
mailing list