RFR: 8187578: BitMap::reallocate should check if old_map is NULL

[Stefan Karlsson]

Hi David,

On 2017-09-17 23:31, David Holmes wrote:
> Hi Erik,
> 
> On 15/09/2017 11:04 PM, Erik Helin wrote:
>> Hi all,
>>
>> I'm still trying to compile with gcc 7.1.1 and run into another small 
>> issue. BitMap::reallocate calls Copy::disjoint_words and there is a 
>> case in Copy::disjoint_words that might result in call to memcpy. The 
>> problem is that BitMap::reallocate does not check that the "from" 
>> argument to Copy::disjoint_words differs from NULL, and a call to 
>> memcpy with a NULL argument is undefined behavior.
> 
> Shouldn't this whole function be a no-op if old_map is NULL? Wouldn't 
> calling it with NULL be a programming error that we should be checking 
> via an assert?

When this function was written the intent was to give it similar 
semantics as that of realloc. Hence, old_map == NULL is supposed to be a 
valid input to reallocate.

This is explicitly used in the following function:
template <class Allocator>
bm_word_t* BitMap::allocate(const Allocator& allocator, idx_t 
size_in_bits, bool clear) {
   // Reuse reallocate to ensure that the new memory is cleared.
   return reallocate(allocator, NULL, 0, size_in_bits, clear);
}

Thanks,
StefanK

> 
> Thanks,
> David
> 
>> Webrev:
>> http://cr.openjdk.java.net/~ehelin/8187578/00/
>>
>> Patch:
>> --- old/src/hotspot/share/utilities/bitMap.cpp    2017-09-15 
>> 14:47:21.471113699 +0200
>> +++ new/src/hotspot/share/utilities/bitMap.cpp    2017-09-15 
>> 14:47:21.179112252 +0200
>> @@ -81,8 +81,10 @@
>>     if (new_size_in_words > 0) {
>>       map = allocator.allocate(new_size_in_words);
>>
>> -    Copy::disjoint_words((HeapWord*)old_map, (HeapWord*) map,
>> -                         MIN2(old_size_in_words, new_size_in_words));
>> +    if (old_map != NULL) {
>> +      Copy::disjoint_words((HeapWord*)old_map, (HeapWord*) map,
>> +                           MIN2(old_size_in_words, new_size_in_words));
>> +    }
>>
>>       if (new_size_in_words > old_size_in_words) {
>>         clear_range_of_words(map, old_size_in_words, new_size_in_words);
>>
>> Thanks,
>> Erik