RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

Doug Simon @ Oracle
Please review this simple fix for a VM crash when calling ResolvedTypeType.getClassInitializer on an array type.

In addition to fixing the code for ResolvedTypeType.getClassInitializer, this patch also makes CompilerToVM.getImplementor more robust in case it is called with a ResolvedjavaType representing a non-interface type.

Lastly, there are a few minor comment formatting changes that were made automatically by Eclipse. I'd like to keep them as Eclipse is the preferred tool for developing JVMCI code.

https://bugs.openjdk.java.net/browse/JDK-8193930
http://cr.openjdk.java.net/~dnsimon/8193930/

-Doug
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

dean.long
Instead of


 998   if (klass->is_array_klass()) {
 999     return NULL;
1000   }
1001   InstanceKlass* iklass = (InstanceKlass*) klass;
how about

 998   if (!klass->is_instance_klass()) {
 999     return NULL;
1000   }
1001   InstanceKlass* iklass = InstanceKlass::cast(klass);

The rest looks good.

dl

On 12/21/17 4:33 AM, Doug Simon wrote:
Please review this simple fix for a VM crash when calling ResolvedTypeType.getClassInitializer on an array type.

In addition to fixing the code for ResolvedTypeType.getClassInitializer, this patch also makes CompilerToVM.getImplementor more robust in case it is called with a ResolvedjavaType representing a non-interface type.

Lastly, there are a few minor comment formatting changes that were made automatically by Eclipse. I'd like to keep them as Eclipse is the preferred tool for developing JVMCI code.

https://bugs.openjdk.java.net/browse/JDK-8193930
http://cr.openjdk.java.net/~dnsimon/8193930/

-Doug

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

Doug Simon @ Oracle


> On 21 Dec 2017, at 19:44, [hidden email] wrote:
>
> Instead of
>
>
>  998   if (klass->is_array_klass()) {
>
>  999     return NULL;
> 1000   }
> 1001   InstanceKlass* iklass = (InstanceKlass*) klass;
> how about
>
>  998   if (!klass->is_instance_klass()) {
>  999     return NULL;
> 1000   }
> 1001   InstanceKlass* iklass = InstanceKlass::cast(klass);

Thanks for the suggestion (and review). I've updated the webrev to include it.

-Doug

> On 12/21/17 4:33 AM, Doug Simon wrote:
>> Please review this simple fix for a VM crash when calling ResolvedTypeType.getClassInitializer on an array type.
>>
>> In addition to fixing the code for ResolvedTypeType.getClassInitializer, this patch also makes CompilerToVM.getImplementor more robust in case it is called with a ResolvedjavaType representing a non-interface type.
>>
>> Lastly, there are a few minor comment formatting changes that were made automatically by Eclipse. I'd like to keep them as Eclipse is the preferred tool for developing JVMCI code.
>>
>>
>> https://bugs.openjdk.java.net/browse/JDK-8193930
>> http://cr.openjdk.java.net/~dnsimon/8193930/
>>
>>
>> -Doug
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

dean.long
On 12/22/17 2:05 AM, Doug Simon wrote:

>
>> On 21 Dec 2017, at 19:44, [hidden email] wrote:
>>
>> Instead of
>>
>>
>>   998   if (klass->is_array_klass()) {
>>
>>   999     return NULL;
>> 1000   }
>> 1001   InstanceKlass* iklass = (InstanceKlass*) klass;
>> how about
>>
>>   998   if (!klass->is_instance_klass()) {
>>   999     return NULL;
>> 1000   }
>> 1001   InstanceKlass* iklass = InstanceKlass::cast(klass);
> Thanks for the suggestion (and review). I've updated the webrev to include it.

I don't see the use of InstanceKlass::cast() in the update.

dl

> -Doug
>
>> On 12/21/17 4:33 AM, Doug Simon wrote:
>>> Please review this simple fix for a VM crash when calling ResolvedTypeType.getClassInitializer on an array type.
>>>
>>> In addition to fixing the code for ResolvedTypeType.getClassInitializer, this patch also makes CompilerToVM.getImplementor more robust in case it is called with a ResolvedjavaType representing a non-interface type.
>>>
>>> Lastly, there are a few minor comment formatting changes that were made automatically by Eclipse. I'd like to keep them as Eclipse is the preferred tool for developing JVMCI code.
>>>
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8193930
>>> http://cr.openjdk.java.net/~dnsimon/8193930/
>>>
>>>
>>> -Doug
>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

Tom Rodriguez-2
In reply to this post by Doug Simon @ Oracle
Looks good.

tom

Doug Simon wrote:

> Please review this simple fix for a VM crash when calling ResolvedTypeType.getClassInitializer on an array type.
>
> In addition to fixing the code for ResolvedTypeType.getClassInitializer, this patch also makes CompilerToVM.getImplementor more robust in case it is called with a ResolvedjavaType representing a non-interface type.
>
> Lastly, there are a few minor comment formatting changes that were made automatically by Eclipse. I'd like to keep them as Eclipse is the preferred tool for developing JVMCI code.
>
> https://bugs.openjdk.java.net/browse/JDK-8193930
> http://cr.openjdk.java.net/~dnsimon/8193930/
>
> -Doug
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

Doug Simon @ Oracle
In reply to this post by dean.long


> On 22 Dec 2017, at 17:59, [hidden email] wrote:
>
> On 12/22/17 2:05 AM, Doug Simon wrote:
>
>>
>>> On 21 Dec 2017, at 19:44, [hidden email] wrote:
>>>
>>> Instead of
>>>
>>>
>>>  998   if (klass->is_array_klass()) {
>>>
>>>  999     return NULL;
>>> 1000   }
>>> 1001   InstanceKlass* iklass = (InstanceKlass*) klass;
>>> how about
>>>
>>>  998   if (!klass->is_instance_klass()) {
>>>  999     return NULL;
>>> 1000   }
>>> 1001   InstanceKlass* iklass = InstanceKlass::cast(klass);
>> Thanks for the suggestion (and review). I've updated the webrev to include it.
>
> I don't see the use of InstanceKlass::cast() in the update.

Sorry - I missed that was part of the suggested change (in addition to changing the `if` test).

While fixing that, I also noticed that I should be using InstanceKlass::cast in getImplementor as well.

Please double check that the webrev for jvmciCompilerToVM.cpp now looks right.

-Doug

>
> dl
>
>> -Doug
>>
>>> On 12/21/17 4:33 AM, Doug Simon wrote:
>>>> Please review this simple fix for a VM crash when calling ResolvedTypeType.getClassInitializer on an array type.
>>>>
>>>> In addition to fixing the code for ResolvedTypeType.getClassInitializer, this patch also makes CompilerToVM.getImplementor more robust in case it is called with a ResolvedjavaType representing a non-interface type.
>>>>
>>>> Lastly, there are a few minor comment formatting changes that were made automatically by Eclipse. I'd like to keep them as Eclipse is the preferred tool for developing JVMCI code.
>>>>
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8193930
>>>> http://cr.openjdk.java.net/~dnsimon/8193930/
>>>>
>>>>
>>>> -Doug

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8193930: [JVMCI] calling ResolvedTypeType.getClassInitializer on an array type crashes

dean.long
On 12/22/17 9:30 AM, Doug Simon wrote:

>
>> On 22 Dec 2017, at 17:59, [hidden email] wrote:
>>
>> On 12/22/17 2:05 AM, Doug Simon wrote:
>>
>>>> On 21 Dec 2017, at 19:44, [hidden email] wrote:
>>>>
>>>> Instead of
>>>>
>>>>
>>>>   998   if (klass->is_array_klass()) {
>>>>
>>>>   999     return NULL;
>>>> 1000   }
>>>> 1001   InstanceKlass* iklass = (InstanceKlass*) klass;
>>>> how about
>>>>
>>>>   998   if (!klass->is_instance_klass()) {
>>>>   999     return NULL;
>>>> 1000   }
>>>> 1001   InstanceKlass* iklass = InstanceKlass::cast(klass);
>>> Thanks for the suggestion (and review). I've updated the webrev to include it.
>> I don't see the use of InstanceKlass::cast() in the update.
> Sorry - I missed that was part of the suggested change (in addition to changing the `if` test).
>
> While fixing that, I also noticed that I should be using InstanceKlass::cast in getImplementor as well.
>
> Please double check that the webrev for jvmciCompilerToVM.cpp now looks right.

Looks good.

dl

> -Doug
>
>> dl
>>
>>> -Doug
>>>
>>>> On 12/21/17 4:33 AM, Doug Simon wrote:
>>>>> Please review this simple fix for a VM crash when calling ResolvedTypeType.getClassInitializer on an array type.
>>>>>
>>>>> In addition to fixing the code for ResolvedTypeType.getClassInitializer, this patch also makes CompilerToVM.getImplementor more robust in case it is called with a ResolvedjavaType representing a non-interface type.
>>>>>
>>>>> Lastly, there are a few minor comment formatting changes that were made automatically by Eclipse. I'd like to keep them as Eclipse is the preferred tool for developing JVMCI code.
>>>>>
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8193930
>>>>> http://cr.openjdk.java.net/~dnsimon/8193930/
>>>>>
>>>>>
>>>>> -Doug