RFR(S): 8069124 - runtime/NMT/MallocSiteHashOverflow.java failing in nightlies

David Holmes david.holmes at oracle.com
Thu Feb 26 04:04:12 UTC 2015


On 26/02/2015 1:55 PM, Coleen Phillimore wrote:
>
>
> In this case, I think 'long' is chosen because we want to turn address
> into an integral type to create the hashcode and that'll cover a 64 bit
> address.

Sounds like a job for intptr_t.

> Even if we change the signedness of the value, it's still a
> useful hash code.  I don't see a bug here, only unfortunate casts.

I'm surprised this change doesn't induce further warnings about mixing 
signed and unsigned. But the code screams "type confusion" to me.

> This change looks good to me.  It's a really good find, Christian! We've
> been chasing this bug for months.

The real bug was reading an uninitialized value no? In which case the 
change from signed to unsigned seems incidental, unless also guarding 
against overflow.

David
-----

>
> Thanks,
> Coleen
>
>
> On 2/24/15, 11:09 PM, David Holmes wrote:
>> Hi Christian,
>>
>> On 25/02/2015 12:57 PM, Christian Tornqvist wrote:
>>> Hi everyone,
>>>
>>>
>>>
>>> Please review this small fix for an issue with NMT.
>>
>> The use of long in this function looks wrong, particularly as
>> sizeof(long) might equal sizeof(int) and long is signed but now
>> _hash_value is unsigned:
>>
>>  72 // Hash code. Any better algorithm?
>>   73 unsigned int NativeCallStack::hash() const {
>>   74   long hash_val = _hash_value;
>>   75   if (hash_val == 0) {
>>   76     long pc;
>>   77     int  index;
>>   78     for (index = 0; index < NMT_TrackingStackDepth; index ++) {
>>   79       pc = (long)_stack[index];
>>   80       if (pc == 0) break;
>>   81       hash_val += pc;
>>   82     }
>>   83
>>   84     NativeCallStack* p = const_cast<NativeCallStack*>(this);
>>   85     p->_hash_value = (unsigned int)(hash_val & 0xFFFFFFFF);
>>   86   }
>>   87   return _hash_value;
>>   88 }
>>
>> Even with the original code the use of long seems wrong.
>>
>> David
>>
>>
>>>
>>>
>>> The failure was caused by reading random memory from the uninitialized
>>> _hash_value, when this value happened to be 0x80000000, hash_to_index()
>>> failed to negate the value and ended up with an index of -16:
>>>
>>>
>>>
>>> #  Internal Error
>>> (C:\\jprt\\T\\P1\\130630.ctornqvi\\s\\hotspot\\src\\share\\vm\\services\\mal
>>>
>>> locSiteTable.cpp:139), pid=5680, tid=4928
>>>
>>> #  assert(index >= 0) failed: Negative index -16
>>>
>>>
>>>
>>> Reproduced the issue and verified the fix using a debugger. Ran
>>> vm.quick and
>>> hotspot/test/:hotspot_jprt tests on Linux i586/x64 and Windows
>>> i586/x64 with
>>> -XX:NativeMemoryTracking=detail
>>>
>>>
>>>
>>> Webrev:
>>>
>>> http://cr.openjdk.java.net/~ctornqvi/webrev/8069124/webrev.00/
>>>
>>>
>>>
>>> Bug:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8069124
>>>
>>>
>>>
>>> Thanks,
>>>
>>> Christian
>>>
>>>
>>>
>


More information about the hotspot-runtime-dev mailing list