Possibility of integer overflow in JfrThreadSampler::run()

[Yasumasa Suenaga]

Hi Severin,

> This fix works for me:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/ub-jfr-sampler-fix/webrev/
> 
> Feel free to use it.

Thanks!
I modified your patch as below. It works fine on my environment.

   http://cr.openjdk.java.net/~ysuenaga/jfr-integer-overflow/webrev.2/


I filed this issue to JBS:

   https://bugs.openjdk.java.net/browse/JDK-8216486


Yasumasa


On 2019/01/10 20:06, Severin Gehwolf wrote:
> Hi Yasumasa,
> 
> On Wed, 2019-01-09 at 20:27 +0100, Severin Gehwolf wrote:
>> Hi,
>>
>> On Wed, 2019-01-09 at 13:23 +0900, Yasumasa Suenaga wrote:
>>> Hi all,
>>>
>>> I posted this issue in May 2018 [1].
>>> It still occurs in current jdk/jdk when it was build by GCC 8.
>>>
>>> We can avoid it with -fno-strict-overflow GCC option. So I propose to add it
>>> to JvmOverrideFiles.gmk:
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/jfr-integer-overflow/webrev.1/
>>>
>>> JDK-8147466 has been fixed with similar change for jdk7u [2].
>>>
>>> What do you think?
>>
>> This fix is masking undefined behavior (signed integer overflow) in
>> code. We should fix the code instead. The previous fix looked better,
>> but would still overflow if:
>>
>> max(_interval_java, 10) + last_java_ms > max_jlong
> 
> This fix works for me:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/ub-jfr-sampler-fix/webrev/
> 
> Feel free to use it.
> 
> Thanks,
> Severin
> 
>> Note that the assertion triggers on a fastdebug build with GCC 8.2 and
>> running :jdk_jfr test group for me (18 tests fail). Failures go away
>> with -fno-strict-overflow.
>>
>>> I will file it to JBS if it is accepted.
>>
>> Please file a bug.
>>
>> Thanks,
>> Severin
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>> [1] https://mail.openjdk.java.net/pipermail/hotspot-jfr-dev/2018-May/000047.html
>>> [2] http://hg.openjdk.java.net/jdk7u/jdk7u/jdk/rev/35dcc0db31dc
>