RFR: 8134389: Crash in HotSpot with jvm.dll+0x42b48 ciObjectFactory::create_new_metadata

[Jamsheed C m]

Hi Vladimir,

Thanks for the review,


On 9/19/2016 10:08 PM, Vladimir Ivanov wrote:
> Overall, the fix looks good.
>
> Some nitpicks:
>  (1) I'd prefer to avoid using ciMethod::is_compiled_lambda_form();
Ok. Hope there is no correctness reasons behind this?
>  (2) align with other uses of TypeCast for method handles.
There was a bug in closed arm port blocking this(8166441). i was getting 
failure in arm-32 closed port. its fixed and is out on review.
The issue was with null constant getting typecast. i chose alternate 
implementation to avoid all those cases. i am Ok for aligning with 
previous typecast implementation.

hope i needn't send  updated webrev?

Best Regards,
Jamsheed

>
> Also, ciType::is_klass() can be replaced with 
> !ciType::is_primitive_type() check, but IMO it doesn't matter much.
>
> Something like the following:
>
> diff --git a/src/share/vm/c1/c1_GraphBuilder.cpp 
> b/src/share/vm/c1/c1_GraphBuilder.cpp
> --- a/src/share/vm/c1/c1_GraphBuilder.cpp
> +++ b/src/share/vm/c1/c1_GraphBuilder.cpp
> @@ -1493,6 +1493,24 @@
>    // Check to see whether we are inlining. If so, Return
>    // instructions become Gotos to the continuation point.
>    if (continuation() != NULL) {
> +
> +    int invoke_bci = state()->caller_state()->bci();
> +
> +    if (x != NULL && !ignore_return) {
> +      ciMethod* caller = state()->scope()->caller()->method();
> +      Bytecodes::Code invoke_raw_bc = 
> caller->raw_code_at_bci(invoke_bci);
> +      if (invoke_raw_bc == Bytecodes::_invokehandle ||
> +          invoke_raw_bc == Bytecodes::_invokedynamic) {
> +        ciType* declared_ret_type = 
> caller->get_declared_signature_at_bci(invoke_bci)->return_type();
> +        if (declared_ret_type->is_klass() &&
> +            x->exact_type() == NULL &&
> +            x->declared_type() != declared_ret_type &&
> +            declared_ret_type != compilation()->env()->Object_klass()) {
> +          x = append(new TypeCast(declared_ret_type->as_klass(), x, 
> copy_state_before()));
> +        }
> +      }
> +    }
> +
>      assert(!method()->is_synchronized() || InlineSynchronizedMethods, 
> "can not inline synchronized methods yet");
>
>      if (compilation()->env()->dtrace_method_probes()) {
> @@ -1516,7 +1534,6 @@
>      // State at end of inlined method is the state of the caller
>      // without the method parameters on stack, including the
>      // return value, if any, of the inlined method on operand stack.
> -    int invoke_bci = state()->caller_state()->bci();
>      set_state(state()->caller_state()->copy_for_parsing());
>      if (x != NULL) {
>        if (!ignore_return) {
> diff --git a/src/share/vm/c1/c1_Instruction.cpp 
> b/src/share/vm/c1/c1_Instruction.cpp
> --- a/src/share/vm/c1/c1_Instruction.cpp
> +++ b/src/share/vm/c1/c1_Instruction.cpp
> @@ -360,7 +360,8 @@
>  }
>
>  ciType* Invoke::declared_type() const {
> -  ciType *t = _target->signature()->return_type();
> +  ciSignature* declared_signature = 
> state()->scope()->method()->get_declared_signature_at_bci(state()->bci());
> +  ciType *t = declared_signature->return_type();
>    assert(t->basic_type() != T_VOID, "need return value of void 
> method?");
>    return t;
>  }
> diff --git a/src/share/vm/ci/ciMethod.hpp b/src/share/vm/ci/ciMethod.hpp
> --- a/src/share/vm/ci/ciMethod.hpp
> +++ b/src/share/vm/ci/ciMethod.hpp
> @@ -255,6 +255,12 @@
>      ciSignature* ignored_declared_signature;
>      return get_method_at_bci(bci, ignored_will_link, 
> &ignored_declared_signature);
>    }
> +  ciSignature*  get_declared_signature_at_bci(int bci) {
> +    bool ignored_will_link;
> +    ciSignature* declared_signature;
> +    get_method_at_bci(bci, ignored_will_link, &declared_signature);
> +    return declared_signature;
> +  }
>
>    // Given a certain calling environment, find the monomorphic target
>    // for the call.  Return NULL if the call is not monomorphic in
>
> Best regards,
> Vladimir Ivanov
>
> On 9/11/16 2:51 PM, Jamsheed C m wrote:
>> i made some changes to my fix. webrev is updated in place.
>>
>> pit results with latest modification updated in bug(not still completed)
>>
>> Best Regards,
>>
>> Jamsheed
>>
>>
>> On 9/10/2016 3:53 AM, Jamsheed C m wrote:
>>>
>>> adding a little more description as per my understanding
>>>
>>> This issue can happen only for compiled lforms not inlined case
>>>
>>> there are two scenarios.
>>> 1) no compiled lforms inlined
>>> 2) some compiled lforms are inlined or  final method is not inlined
>>> (linkTo* not inlined)..    (i.e partially inlined)
>>>
>>> in all these cases *Invoke instruction* will be *return Value*. and
>>> will have erased type.
>>> so we reify return type either by type casting(for partially inlined
>>> case) or by directly pulling from callsite MT.
>>>
>>> Best Regards,
>>>
>>> Jamsheed
>>>
>>>
>>> On 9/8/2016 3:26 PM, Jamsheed C m wrote:
>>>> Hi All,
>>>>
>>>> bugid: https://bugs.openjdk.java.net/browse/JDK-8134389
>>>>
>>>> webrev: http://cr.openjdk.java.net/~jcm/8134389/webrev.00/
>>>>
>>>> return type information is not available in lforms, this causes
>>>> contradictions in operation like  store indexed. mh _linkTo* site arg
>>>> type casting. etc..
>>>>
>>>> fix: TypeCast to declared return type at lform return.
>>>>
>>>> Best Regards,
>>>>
>>>> Jamsheed
>>>>
>>>
>>