RFR: 8161550: [JVMCI] Crash: assert(sig_bt[member_arg_pos] == T_OBJECT)
Stefan Anzinger
stefan.anzinger at oracle.com
Thu Aug 18 16:57:16 UTC 2016
Hi Zoltan,
thank you for your input. With a bit back and fourth I came up with
another solution for this.
We've decided to always return null when someone tries to resolve a
method with @PolymorphicSignature annotation.
Please review the latest webrev [1]. Consider, this webrev depends on
Dougs change for 8164214.
[1] http://cr.openjdk.java.net/~sanzinger/8161550/webrev.04/
Thank you
Stefan
On 08/10/2016 12:27 PM, Zoltán Majó wrote:
> Hi Stefan,
>
>
> thank you for the updated webrev!
>
> On 08/09/2016 01:26 PM, Stefan Anzinger wrote:
>> Hi Zoltan,
>>
>> Tom tried to integrate this patch, but the assertion error was still in.
>> I've updated the webrev, to fix the assertion error too.
>>
>> Please see updated webrev [1].
>>
>> The issue there was, that the assertion triggered, when JVMCI was trying
>> to resolve MethodHandle.linkTo* methods. To avoid this kind of crash, I
>> updated method LinkResolver::lookup_method_in_klasses I removed the bool
>> argument checkpolymorphism.
>> * There are just two call sites, one call it with true, the other
>> with false
>> * The check performed when checkpolymorphism is true is done at the
>> very end of the method.
>> thus, I moved check code to the call site where the check should be
>> performed.
>
> I'm fine with moving the check from
>
> LinkResolver::lookup_method_in_klasses()
>
> to
>
> LinkResolver::resolve_method()
>
> (where it is actually needed). But it seems that the change in
> resolve_method()should be done a bit differently so that the method's
> original logic is preserved.
>
> Before you changes, calling lookup_method_in_klasses() in
> resolve_method() returned null for signature polymorphic methods
>
> 707 // 3. lookup method in resolved klass and its super klasses
> here --> 708 methodHandle resolved_method =
> lookup_method_in_klasses(link_info, true, false, CHECK_NULL);
>
> resolve_method() then called lookup_polymorphic_method() for signature
> polymorphic methods
>
> 709
> 710 // 4. lookup method in all the interfaces implemented by the
> resolved klass
> 711 if (resolved_method.is_null() &&
> !resolved_klass->is_array_klass()) { // not found in the class hierarchy
> 712 resolved_method = lookup_method_in_interfaces(link_info,
> CHECK_NULL);
> 713
> 714 if (resolved_method.is_null()) {
> 715 // JSR 292: see if this is an implicitly generated method
> MethodHandle.linkToVirtual(*...), etc
> here --> 716 resolved_method =
> lookup_polymorphic_method(link_info, (Handle*)NULL, (Handle*)NULL, THREAD);
> 717 if (HAS_PENDING_EXCEPTION) {
> 718 nested_exception = Handle(THREAD, PENDING_EXCEPTION);
> 719 CLEAR_PENDING_EXCEPTION;
> 720 }
> 721 }
> 722 }
>
> (because resolved_method was NULL at line 711 andthe MethodHandleclass
> is a proper class (i.e., not an array class).
>
> Itseems to me that with your proposed changeslookup_method_in_klasses()
> returns non-null for signature polymorphic methods
>
> 698 // 3. lookup method in resolved klass and its super klasses
> here --> 699 methodHandle resolved_method =
> lookup_method_in_klasses(link_info, false, CHECK_NULL);
>
> and then lookup_polymorphic_method() is not called (because resolved
> method is not null).
>
> 701 // 4. lookup method in all the interfaces implemented by
> the resolved klass
> here --> 702 if (resolved_method.is_null()) { // not found in the
> class hierarchy
> 703 if (!resolved_klass->is_array_klass()) {
> 704 resolved_method =
> lookup_method_in_interfaces(link_info, CHECK_NULL);
> 705 if (resolved_method.is_null()) {
> 706 // JSR 292: see if this is an implicitly generated
> method MethodHandle.linkToVirtual(*...), etc
> 707 resolved_method =
> lookup_polymorphic_method(link_info, (Handle*)NULL, (Handle*)NULL, THREAD);
> 708 if (HAS_PENDING_EXCEPTION) {
> 709 nested_exception = Handle(THREAD, PENDING_EXCEPTION);
> 710 CLEAR_PENDING_EXCEPTION;
> 711 }
> 712 }
>
> A solution would be to add
>
> if (!resolved_method.is_null() &&
> MethodHandles::is_signature_polymorphic(resolved_method->intrinsic_id())) {
> // Do not link directly to these. The VM must produce a synthetic
> one using lookup_polymorphic_method.
> resolved_method = NULL;
> }
>
> before
>
> 710 // 4. lookup method in all the interfaces implemented by the
> resolved klass
> 711 if (resolved_method.is_null() &&
> !resolved_klass->is_array_klass()) { // not found in the class hierarchy
>
> and remove
>
> 716 } else {
> 717 vmIntrinsics::ID iid = resolved_method->intrinsic_id();
> 718 if (MethodHandles::is_signature_polymorphic(iid)) {
> 719 // Do not link directly to these. The VM must produce
> a synthetic one using lookup_polymorphic_method.
> 720 resolved_method = NULL;
> 721 }
>
> Am I seeing this wrong?
>
> Also, please use correct indentation in linkResolver.cpp. Please use
> spaces instead of tabs.
>
> A small issue: Your comments in TestResolvedJavaType.java are missinga verb
>
> 582 // Polymorphic signatures do not be
> looked up.
> 617 // Polymorphic signatures do not be
> looked up.
>
> Maybe say something like "Polymorphic signatures are not looked up" (or
> should not be looked up).
>
> Could you please run the jdk java/lang/invoke tests for the next
> revision of the webrev (in addition to JPRT)?
>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>
>
>>
>> [1] http://cr.openjdk.java.net/~sanzinger/8161550/webrev.01/
>>
>> Stefan
>>
>>
>> On 08/03/2016 04:43 AM, Zoltán Majó wrote:
>>> Hi Stefan,
>>>
>>>
>>> thank you for fixing this issue! Your fix looks good to me.
>>>
>>> Do you need a sponsor? If yes, I can sponsor your change.
>>>
>>> Best regards,
>>>
>>>
>>> Zoltan
>>>
>>>
>>> On 08/02/2016 07:07 AM, Stefan Anzinger wrote:
>>>> Hello,
>>>>
>>>> this RFR fixes 8161550. JVMCI must not return intrinsified MethodHandle
>>>> methods. The test had to be updated and the shortcut evaluation in
>>>> HotSpotResolvedObjectTypeImpl.resolveMethod needs take care of this
>>>> fact.
>>>>
>>>> http://cr.openjdk.java.net/~sanzinger/8161550/webrev.00/
>>>>
>>>> Stefan
>
More information about the hotspot-compiler-dev
mailing list