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