RFR: 8235669: G1: Stack walking API can expose AS_NO_KEEPALIVE oops

[erik.osterlund at oracle.com]

Hi Thomas,

Thanks for the review!

/Erik

On 1/7/20 12:15 PM, Thomas Schatzl wrote:
> Hi,
>
> On 07.01.20 03:46, Kim Barrett wrote:
>>> On Dec 10, 2019, at 11:02 AM, erik.osterlund at oracle.com wrote:
>>>
>>> Hi,
>>>
>>> When the stack is walked and e.g. locals are turned into 
>>> StackValues, it might be that said local was made a constant oop by 
>>> the JIT. In such cases, it is read from the nmethod using 
>>> ON_STRONG_OOP_REF | AS_NO_KEEPALIVE. However, these oops need to be 
>>> kept alive when concurrent marking is ongoing.
>>> While I haven't seen crashes obviously linked to this yet, I don't 
>>> think we should wait until we do, because it certainly will eventually.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8235669
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8235669/webrev.00/
>>>
>>> Thanks,
>>> /Erik
>>
>> Change looks good.
>
> To me too.
>
>>
>> I think it's kind of gross that oop_at returns an AS_NO_KEEPALIVE
>> value to some more or less arbitrary context without any indication
>> that this can happen.  The scope of AS_NO_KEEPALIVE values really
>> ought to be more constrained than that.
>>
>> I wonder if oop_at should do the phantom access, and there should be a
>> different function for use by those places that want / can cope with
>> an AS_NO_KEEPALIVE value. So I think there might be some naming / API
>> issues in this neighborhood, but that can be addressed in a post-14 RFE.
>>
>>
>
> +1
>
> Thanks,
>   Thomas