Possibility of integer overflow in JfrThreadSampler::run()

[Yasumasa Suenaga]

Hi Erik,

For example, `java_interval` might be `max_jlong` [1].
`next_j` is calculated with it [2].

If `last_java_ms - now_ms` is positive value, next_j might be overflown.
My patch can avoid this case because it does not add to `max_jlong`.

According to GCC 8 release note [3], signed integer overflow is
undefined by default (see description of `-fno-strict-overflow`). I
guess this issue is affected by this GCC change.
I can avoid this crash when I passed
`--with-extra-cflags=-fno-strict-overflow
--with-extra-cxxflags=-fno-strict-overflow` to configure script.


Thanks,

Yasumasa


[1] http://hg.openjdk.java.net/jdk/jdk/file/5e7174bf1259/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp#l471
[2] http://hg.openjdk.java.net/jdk/jdk/file/5e7174bf1259/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp#l476
[3] https://gcc.gnu.org/gcc-8/changes.html



2018-05-24 6:18 GMT+09:00 Erik Gahlin <erik.gahlin at oracle.com>:
> Can you explain how your fix prevents an overflow?
>
> Thanks
> Erik
>
>> On 21 May 2018, at 14:50, Yasumasa Suenaga <yasuenag at gmail.com> wrote:
>>
>>
>> Hi all,
>>
>> I tried JFR: `java -XX:StartFlightRecording=dumponexit=true,filename=test.jfr --version` with fastdebug VM, but it crashed.
>> I uploaded hs_err log here:
>>
>>  http://cr.openjdk.java.net/~ysuenaga/jfr-integer-overflow/hs_err_pid46934.log
>>
>> I encountered this crash on Fedora 28 x64. VM was built with GCC 8.1 .
>>
>> I checked core image, and I found possibility of integer overflow in JfrThreadSampler::run().
>> I paste GDB frame info:
>>
>> ```
>> (gdb) f 24
>> #24 0x00007f657a5937b7 in os::naked_short_sleep (ms=9223372036854775807)
>>    at /home/ysuenaga/OpenJDK/jdk/src/hotspot/os/linux/os_linux.cpp:4076
>> 4076      assert(ms < 1000, "Un-interruptable sleep, short time use only");
>> (gdb) p/x ms
>> $1 = 0x7fffffffffffffff
>> ```
>>
>> Frame #24 is an assertion which is caused this crash.
>> `ms` is max_jlong.
>> JfrThreadSampler::run() calculates `next_j` and `next_n` to calculate sleep time, but it might be overflowed.
>>
>> So I think it can be fixed as below:
>>  http://cr.openjdk.java.net/~ysuenaga/jfr-integer-overflow/webrev/
>>
>> Can it be accepted as a bug? If so, I will file it to JBS and will send review request.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>