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

[Vladimir Ivanov]

Overall, the fix looks good.

Some nitpicks:
  (1) I'd prefer to avoid using ciMethod::is_compiled_lambda_form();
  (2) align with other uses of TypeCast for method handles.

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
>>>
>>
>