RFR(XS) 8008750 [partfait] Null pointer deference in hotspot/src/share/vm/oops/instanceKlass.hpp

[Vladimir Kozlov]

Coleen,

indentations are good, look on cdiffs. Only udiffs are screwed up.

Vladimir

On 3/5/13 5:13 PM, Coleen Phillimore wrote:
>
> Hi Morris,
>
> This looks better.  The indentation came out weird in the webrev, so
> please fix it if it's wrong (I don't need to see the indentation changes
> if you have to make them).
>
> thanks,
> Coleen
>
> On 03/05/2013 07:46 PM, Morris Meyer wrote:
>> Thanks for the review Coleen and Vladimir.  Here's the updated webrev.
>>
>>         --mm
>>
>> WEBREV - http://cr.openjdk.java.net/~morris/8008750.02
>>
>> On 3/5/13 6:16 PM, Vladimir Kozlov wrote:
>>> On 3/5/13 2:39 PM, Coleen Phillimore wrote:
>>>>
>>>> Oh, I see.  Can you still use assert rather then ShouldNotReachHere()?
>>>> The former will vanish in product but the latter is added in product.
>>>
>>> Yes, I think we can remove "else { ShouldNotReachHere();" in header
>>> file instanceKlass.hpp and add assert. Method set_host_klass() has
>>> the assert already, we need to add assert to set_implementor().
>>>
>>> Also in instanceKlass.cpp, Morris, could you reverse check
>>> InstanceKlass::clean_implementors_list()?
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>> Coleen
>>>>
>>>> On 03/05/2013 05:17 PM, Vladimir Kozlov wrote:
>>>>> We want to fix cases where Parfait gives "false" positives. That is
>>>>> why we need real != NULL check, assert does not help.
>>>>>
>>>>> Vladimir
>>>>>
>>>>> On 3/5/13 1:50 PM, Coleen Phillimore wrote:
>>>>>> I can't see the parfait link in the bug.
>>>>>>
>>>>>> What does this change do but check for null in the cases where host
>>>>>> class and adr_implementers can't be null?
>>>>>>
>>>>>> In instanceKlass.hpp ShouldNotReachHere() is enabled in product so
>>>>>> these
>>>>>> should be
>>>>>> assert(host != NULL, "not null");
>>>>>> *adr = host;
>>>>>>
>>>>>> I don't understand the motivation for this change and would rather
>>>>>> not
>>>>>> enable these ShouldNotReachHere() in product mode, ie use asserts
>>>>>> instead.
>>>>>>
>>>>>> Coleen
>>>>>>
>>>>>> On 03/05/2013 03:51 PM, Vladimir Kozlov wrote:
>>>>>>> Looks good. CC to runtime group.
>>>>>>>
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 3/5/13 12:37 PM, Morris Meyer wrote:
>>>>>>>> Folks,
>>>>>>>>
>>>>>>>> Could I get a review for this parfait issue? This has been through
>>>>>>>> JPRT.
>>>>>>>>
>>>>>>>> Thanks much,
>>>>>>>>
>>>>>>>>          --mm
>>>>>>>>
>>>>>>>> WEBREV - http://cr.openjdk.java.net/~morris/8008750.01
>>>>>>>> BUG - https://jbs.oracle.com/bugs/browse/JDK-8008750
>>>>>>
>>>>
>>
>