Request for review (XS): crash in C2 when using -XX:+CountCompiledCalls

[Krystal Mok]

Thanks Chris and Vladimir!

Kris

On 2012-5-19, at 3:40, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> For records, change is good.
> 
> Vladimir
> 
> On 5/18/12 12:19 PM, Christian Thalinger wrote:
>> 
>> On May 18, 2012, at 12:16 PM, Christian Thalinger wrote:
>> 
>>> 
>>> On May 16, 2012, at 9:12 AM, Vladimir Kozlov wrote:
>>> 
>>>> Kris,
>>>> 
>>>> Changes looks good. I will push it tomorrow.
>>> 
>>> I created:
>>> 
>>> 7170053: crash in C2 when using -XX:+CountCompiledCalls
>>> 
>>> and will push it in a minute.  Thanks for the fix.
>> 
>> Here is the webrev (for the record):
>> 
>> http://cr.openjdk.java.net/~twisti/7170053/
>> 
>> -- Chris
>> 
>>> 
>>> -- Chris
>>> 
>>>> 
>>>> Thanks,
>>>> Vladimir
>>>> 
>>>> On 5/16/12 6:02 AM, Krystal Mok wrote:
>>>>> Hi all,
>>>>> 
>>>>> Could I get some reviews for this patch, please?
>>>>> https://gist.github.com/2710000#file_count_compiled_calls.patch
>>>>> 
>>>>> Description:
>>>>> 
>>>>> C2 may crash when compiling methods with -XX:+CountCompiledCalls turned on. The cause is
>>>>> in Parse::count_compiled_calls(), where it made a TypeInstPtr from a ciMethod:
>>>>> 
>>>>> const TypeInstPtr* addr_type = TypeInstPtr::make(method());
>>>>> 
>>>>> Since the klass of a ciMethod is a ciMethodKlass, which isn't a ciInstanceKlass, an assertion is hit later in
>>>>> Compile::flatten_alias_type(), where it's expecting a TypeInstPtr to have a ciInstanceKlass:
>>>>> 
>>>>> ciInstanceKlass *k = to->klass()->as_instance_klass();
>>>>> 
>>>>> An example of the crash is available here: https://gist.github.com/2710000
>>>>> In the example, I started a Groovy shell with -XX:+CountCompiledCalls set, and it crashed quickly when compiling
>>>>> java.lang.String.charAt().
>>>>> 
>>>>> The fix is to use TypeOopPtr::make_from_constant(method()) instead of TypeInstPtr::make(method()).
>>>>> I did check the hg history, and looks like it's been like this since duke at 0. Wonder if there's any history behind this.
>>>>> 
>>>>> Note that this fix may have to be changed again when methodOopDesc's are moved out of PermGen.
>>>>> 
>>>>> Regards,
>>>>> Kris
>>> 
>>