RFR: 8312077: Fix signed integer overflow, final part [v5]

Andrew Haley aph at openjdk.org
Fri Jul 14 11:03:13 UTC 2023


On Fri, 14 Jul 2023 09:21:50 GMT, Dean Long <dlong at openjdk.org> wrote:

>> src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp line 557:
>> 
>>> 555:     }
>>> 556: 
>>> 557:     if (next_j <= sleep_to_next) {
>> 
>> It looks to me like this changes semantics so that it's now broken if `X_period_millis` wraps around. Do you actually want to change semantics in this case?
>
> Yes, if you mean the case where X_period_millis is max_jlong, next_X is near max_jlong, and the old subtract of a negative sleep_to_next would have caused an overflow, I don't think we want to produce a sample in that case. But I could be wrong.
> 
> I had comments explaining all the pathological cases, but I guess I removed them.  The other one is when next_j < 0, next_n < 0, sleep_to_next < 0, next_j != next_n.

OK, I have no objections if you're covered all the bases. Having said that, I'm not sure that possibly-significant semantic changes like this one should be almost hidden in a patch like this one, because a maintenance programmer in a decade or two's time might think it was  a mistake.
So maybe the comments were good.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14883#discussion_r1263600566


More information about the hotspot-dev mailing list