RFR (XS) 8008328: [partfait] Null pointer defererence in hotspot/src/cpu/x86/vm/frame_x86.inline.hpp

[Christian Thalinger]

Looks good.  -- Chris

On Mar 15, 2013, at 8:06 AM, Morris Meyer <morris.meyer at oracle.com> wrote:

> Here is the updated webrev Vladimir:
> 
>        --morris
> 
> WEBREV - http://cr.openjdk.java.net/~morris/8008328.02
> 
> On 3/14/13 5:16 PM, Vladimir Kozlov wrote:
>> guarantee() (use fatal()) should be under == NULL check to avoid always executing 2 checks (C++ should fold it, but I don't trust it :) ):
>> 
>>  if (result_adr != NULL) {
>>    ...
>>  } else {
>>    fatal("bad register save location");
>>  }
>> 
>> Vladimir
>> 
>> On 3/14/13 7:36 AM, Morris Meyer wrote:
>>> Folks,
>>> 
>>> Thanks for the review.  Here is the updated webrev which uses
>>> guarantee() to ensure this problem is caught early, and has updated
>>> variable names per Vladimir's comments.
>>> 
>>>         --morris
>>> 
>>> WEBREV - http://cr.openjdk.java.net/~morris/8008328.02
>>> 
>>> On 3/14/13 1:11 AM, Cristina Cifuentes wrote:
>>>> Hi Peter, Morris,
>>>> 
>>>> It seems that making the bug report more specific may help (so as not
>>>> to think the bug is a false positive)?
>>>> 
>>>> Regards,
>>>> Cristina
>>>> 
>>>> 
>>>> 
>>>> On 21/02/2013, at 4:33 AM, Peter B. Kessler wrote:
>>>> 
>>>>> Not an official review, because I forget if I'm still an official
>>>>> reviewer.
>>>>> 
>>>>> In what sense is this a "false positive"?  (Cc'ing Cristina for the
>>>>> Parfait definition of a false positive, because I don't know who the
>>>>> right Parfait contact is.)  The result of map->location() *should
>>>>> not* be null, but if it is, it's better to fail the assert than to
>>>>> segfault, in that it will be easier to track down the error wherever
>>>>> these functions get inlined.
>>>>> 
>>>>> In frame::set_saved_oop_result(RegisterMap*, oop), if o==null, who
>>>>> discovers, after this method returns, that the store *did not*
>>>>> happen?  That is, is there a better way of making the announcement of
>>>>> the failure closer to the point of failure?  Maybe the answer is that
>>>>> the assert() should be a guarantee()?
>>>>> 
>>>>>            ... peter
>>>>> 
>>>>> Morris Meyer wrote:
>>>>>> Folks,
>>>>>> Could I get a review of this small Parfait false positive?   This
>>>>>> has been through JPRT per the JPG workflow.
>>>>>> Thanks much.
>>>>>>        --mm
>>>>>> WEBREV - http://javaweb.us.oracle.com/~mameyer/webrevs/01/JDK-8008328
>>>>>> JIRA - https://jbs.oracle.com/bugs/browse/JDK-8008328
>>> 
>