RFR: 8339242: Fix overflow issues in AdlArena [v2]

[Kim Barrett]

On Tue, 3 Sep 2024 14:01:25 GMT, Casper Norrbin <duke at openjdk.org> wrote:

>> src/hotspot/share/adlc/adlArena.cpp line 154:
>> 
>>> 152:   if( (c_old+old_size == _hwm) &&            // Adjusting recent thing
>>> 153:       ((size_t)(_max-c_old) >= new_size) ) { // Still fits where it sits, safe from overflow
>>> 154: 
>> 
>> It appears that this change isn't worrying about bad `old_ptr` or `old_size`
>> arguments, which is fine.  But the code can be further improved by replacing
>> lines 144-157 with something like
>> 
>> // Reallocating the most recent allocation?
>> if ((c_old + old_size) == _hwm) {
>>   assert(_chunk->bottom() <= c_old, "invariant");
>>   // Reallocate in place if it fits.  This also handles shrinking.
>>   if (pointer_delta(_max, c_old) >= new_size) {
>>     _hwm = c_old + new_size;
>>     return c_old;
>>   }
>> }
>> 
>> Of course, in adlc you can't use HotSpot's pointer_delta utility, so there
>> you'll need to use something like what's in the PR for that calculation.
>> 
>> Any check for an "unreasonable" size should happen in Amalloc, not here.
>
> I believe this would miss the case where we shrink an allocation in place and we are not at the high water mark, where `new_size <= old_size`, but where `c_old + old_size) == _hwm` does not hold.

Oops, I managed to drop some of the code when pasting it into the github
comment. There should be an `else if` clause:

if ((c_old + old_size) == _hwm) {
  ...
} else if (new_size <= old_size) {
  return c_old;
}

I like this version better than the earlier version because it has only one
copy of the _hwm handling. It trades an additional pointer-delta in the
shrink-last-allocation case for avoiding a compare-and-branch in the
grow-last-allocation case, which I think is a good tradeoff, though likely
pretty minor.

I do not like the new saturated_add version.  I think that just makes things
(slightly) slower vs the pointer_delta compare, with no other benefit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20774#discussion_r1744877424