RFR: 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale [v3]

Kim Barrett kbarrett at openjdk.org
Wed Jan 11 02:16:17 UTC 2023


On Wed, 28 Dec 2022 00:11:15 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/services/memReporter.hpp line 80:
>> 
>>> 78: 
>>> 79:     size_t amount = s1 - s2;
>>> 80:     assert(amount <= SIZE_MAX - _scale / 2, "size_t overflow");
>> 
>> This assert can fail for sufficiently large `amount`, even though the desired
>> value is computable.  The value we want is
>> 
>> size_t scaled = (amount + _scale/2) / _scale;
>> 
>> rounded to nearest, without risk of overflow. We can split `amount` into `p + q`,
>> where `q = amount % _scale` and `p = amount - q` (which is also `(amount / _scale) * _scale`).
>> Then use
>> 
>> size_t scaled = (p + q + _scale/2) / _scale;
>> 
>> =>
>> 
>> size_t scaled = (p / _scale) + ((q + _scale/2) / _scale);
>> 
>> The lefthand side of the addition is exact.
>> The righthand side is 0 if `q <= (_scale - 1)/2`, else 1. (The -1 is to
>> account for odd _scale values.)  So
>> 
>> size_t scaled = (amount / _scale);
>> if ((amount % _scale) <= (_scale - 1)/2) {
>>   scaled += 1;
>> }
>
> And after all that I wrote the conditional backward.  That should be `(amount % _scale) > (_scale - 1)/2`.

Save future readers the trouble of reverse engineering how this works.  Add a comment with some info
about the derivation or something.

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

PR: https://git.openjdk.org/jdk/pull/11514


More information about the hotspot-runtime-dev mailing list