RFR: 8232084: HotSpot build failed with GCC 9.2.1
David Holmes
david.holmes at oracle.com
Thu Oct 17 04:21:34 UTC 2019
On 17/10/2019 2:20 pm, Yasumasa Suenaga wrote:
> On 2019/10/17 12:49, David Holmes wrote:
>> On 17/10/2019 12:45 pm, Yasumasa Suenaga wrote:
>>> On 2019/10/17 11:39, David Holmes wrote:
>>>> On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
>>>>> On 2019/10/17 9:34, David Holmes wrote:
>>>>>> On 17/10/2019 10:07 am, serguei.spitsyn at oracle.com wrote:
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> It looks good.
>>>>>>> One tip:
>>>>>>>
>>>>>>> + // This code would be warned as "stringop-truncation" by GCC 8
>>>>>>> or later.
>>>>>>> +PRAGMA_DIAG_PUSH
>>>>>>> +PRAGMA_STRINGOP_TRUNCATION_IGNORED
>>>>>>> strncpy(buf, str, len);
>>>>>>> +PRAGMA_DIAG_POP
>>>>>>>
>>>>>>>
>>>>>>> I'd suggest to place the comment before the line with strncopy().
>>>>>>
>>>>>> I'd suggest just dropping the comment as it adds nothing, further
>>>>>> gcc 8 is not flagging this use of strncpy.
>>>>>
>>>>> The comment is suggested in [1].
>>>>> Indeed stringop-truncation warning was not reported by GCC 8, but
>>>>> this option introduced in it,
>>>>> and I agree with Ioi to add the comment for other developers why
>>>>> this #pragma is needed.
>>>>
>>>> PRAGMA_STRINGOP_TRUNCATION_IGNORED
>>>>
>>>> seems to pretty clearly state what the issue is to me. But if you
>>>> want to add a comment to make that more clear fine. But don't
>>>> mention gcc 8 as it is not relevant to the need for the pragma now.
>>>
>>> I'd like to change the comment as below:
>>>
>>> This code would be warned as "stringop-truncation" by modern GCC
>>>
>>> Is it ok?
>>
>> Grammatically it needs work. I propose
>>
>> // This code can cause a "stringop-truncation" warning with gcc
>>
>> "modern" will become inaccurate as time goes by.
>
> Thanks!
> To be clear that our code is correct, I will change the comment as below:
>
> // This code can cause a "stringop-truncation" warning with gcc
> incorrectly
// This code can incorrectly cause a "stringop-truncation" warning with gcc
Assuming Ioi feels that is sufficient.
Thanks,
David
>
> Yasumasa
>
>
>> Thanks,
>> David
>>
>>>
>>> Yasumasa
>>>
>>>
>>>> Thanks,
>>>> David
>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> [1]
>>>>> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029578.html
>>>>>
>>>>>
>>>>>> Otherwise okay.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> Not subject for re-review.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>> On 10/16/19 16:25, Yasumasa Suenaga wrote:
>>>>>>>> (Re-send email because I could not send original email to
>>>>>>>> serviceability-dev and hotspot-compiler-dev)
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> We discussed the fix for JDK-8232084 in [1], and I think we
>>>>>>>> should fix it with #pragma.
>>>>>>>> I uploaded new webrev. Could you review it?
>>>>>>>>
>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8232084
>>>>>>>> webrev:
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.03/
>>>>>>>>
>>>>>>>> This change has passed the tests on submit repo
>>>>>>>> (mach5-one-ysuenaga-JDK-8232084-1-20191016-1534-5969882).
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029547.html
>>>>>>>>
>>>>>>>
More information about the serviceability-dev
mailing list