[9] RFR(S): 8040085: dtrace/jsdt tests crash on solaris. found an unadvertised bad scavengable oop in the code cache

[Igor Veresov]

Looks good to me.

igor

On Apr 27, 2014, at 10:37 PM, Albert <albert.noll at oracle.com> wrote:

> Hi,
> 
> could I get a second review (if one is needed)?
> 
> Many thanks,
> Albert
> 
> On 04/25/2014 10:44 AM, Albert wrote:
>> Thank you, Roland.
>> 
>> Best,
>> Albert
>> On 04/25/2014 10:41 AM, Roland Westrelin wrote:
>>>> http://cr.openjdk.java.net/~anoll/8040085/webrev.02/
>>> That looks good to me.
>>> 
>>> Roland.
>>> 
>>>> Best,
>>>> Albert
>>>> 
>>>> On 04/25/2014 10:27 AM, Roland Westrelin wrote:
>>>>>> Here is a new webrev, which includes the clean-up.
>>>>>> http://cr.openjdk.java.net/~anoll/8040085/webrev.01/
>>>>> I think we still want the:
>>>>> 
>>>>> tty->print("---   n  “);
>>>>> 
>>>>> part to match the PrintCompilation output for other methods (it doesn’t really match anymore, actually).
>>>>> 
>>>>> Roland.
>>>>> 
>>>>>> Best,
>>>>>> Albert
>>>>>> 
>>>>>> On 04/25/2014 09:59 AM, Roland Westrelin wrote:
>>>>>>>> 7017732 was pushed 3 years ago. What does this show up now? Also, CodeCache::add_scavenge_root_nmethod() is required if the nmethod embeds oops but I don’t see where in SharedRuntime::generate_dtrace_nmethod() we have constant oops.
>>>>>>> I took a look and found that the oop comes from:
>>>>>>> 
>>>>>>> // Add the class loader of Method* for the nmethod itself
>>>>>>> append_oop_references(&oops, mh->method_holder());
>>>>>>> 
>>>>>>> in finalize_oop_references() that is called from nmethod::new_dtrace_nmethod()
>>>>>>> 
>>>>>>> So Albert’s fix look good to me.
>>>>>>> 
>>>>>>> I also noticed this in AdapterHandlerLibrary::create_dtrace_nmethod():
>>>>>>>   if (PrintCompilation) {
>>>>>>>     ttyLocker ttyl;
>>>>>>>     tty->print("---   n%s  ");
>>>>>>> 
>>>>>>> %s but no argument passed. Can you clean that up as well, Albert?
>>>>>>> 
>>>>>>> Roland.
>> 
>