Possibility of integer overflow in JfrThreadSampler::run()

[Severin Gehwolf]

Hi Yasumasa,

On Thu, 2019-01-10 at 23:08 +0900, Yasumasa Suenaga wrote:
> 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/

Could you explain a little why you've modified it the way you did? It
seems overly convoluted for some rather theoretical overflow cases.
Perhaps somebody could shed some light on what a setting of:

_interval_java == 0

is supposed to mean for the JfrThreadSampler. It looks to me it's
max_jlong setting is the problematic case triggering this issue.

> I filed this issue to JBS:
> 
>    https://bugs.openjdk.java.net/browse/JDK-8216486

Thanks! Since you've got this bug now, please send it for official RFR
with a proper subject.

Cheers,
Severin

> 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