[11u] RFR: 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

[Reingruber, Richard]

Hi Götz,

> This affects the patch of this change, so I made a new webrev.
> It also incorporates the comment I missed:
> http://cr.openjdk.java.net/~goetz/wr20/8212160-jvmti_crash_resolving-jdk11/04/

Seems you forgot to remove the commented-out version of JvmtiExport::post_compiled_method_load(JvmtiEnv* env, nmethod *nm)

See http://cr.openjdk.java.net/~goetz/wr20/8212160-jvmti_crash_resolving-jdk11/04/src/hotspot/share/prims/jvmtiExport.cpp.udiff.html

Cheers, Richard.

-----Original Message-----
From: Lindenmaier, Goetz <goetz.lindenmaier at sap.com> 
Sent: Dienstag, 24. November 2020 08:57
To: Reingruber, Richard <richard.reingruber at sap.com>; jdk-updates-dev at openjdk.java.net
Subject: RE: [11u] RFR: 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

Hi Richard, 

Thanks for reviewing!

Yes, you are right, downporting the predecessor makes sense.
It is a limited change and a pure fix as I understand. 
I requested downport and sent a RFR for it:
http://mail.openjdk.java.net/pipermail/jdk-updates-dev/2020-November/004220.html

This affects the patch of this change, so I made a new webrev.
It also incorporates the comment I missed:
http://cr.openjdk.java.net/~goetz/wr20/8212160-jvmti_crash_resolving-jdk11/04/

Best regards,
  Goetz.

> -----Original Message-----
> From: Reingruber, Richard <richard.reingruber at sap.com>
> Sent: Thursday, November 19, 2020 6:17 PM
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; jdk-updates-
> dev at openjdk.java.net
> Subject: RE: [11u] RFR: 8212160: JVMTI agent crashes with "assert(_value !=
> 0LL) failed: resolving NULL _value"
> 
> Hi Götz,
> 
> --- src/hotspot/share/prims/jvmtiExport.cpp
> 
> > I added void JvmtiExport::post_compiled_method_load(JvmtiEnv* env,
> nmethod *nm)
> > from head which is called in
> 
> It would be cleaner to downport the original bugfix but I'm ok with your
> solution.
> 
> --- src/hotspot/share/runtime/serviceThread.cpp
> 
> the patch you prepared seems to be missing the change marked with !!! of
> the original patch
> 
> @@ -209,7 +219,8 @@
>      if (_jvmti_event != NULL) {
>        _jvmti_event->nmethods_do(cf);
>      }
> +    // Requires a lock, because threads can be adding to this queue.     !!!
>      MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
> -    JvmtiDeferredEventQueue::nmethods_do(cf);
> +    _jvmti_service_queue.nmethods_do(cf);
> 
> This is inadvertently, isn't it?
> 
> The rest is good looking ;)
> 
> Cheers, Richard.
> 
> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> On Behalf
> Of Reingruber, Richard
> Sent: Mittwoch, 18. November 2020 22:29
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; jdk-updates-
> dev at openjdk.java.net
> Subject: RE: [11u] RFR: 8212160: JVMTI agent crashes with "assert(_value !=
> 0LL) failed: resolving NULL _value"
> 
> Hi Götz,
> 
> I'm getting ready to review this.
> 
> If I'm not mistaken, your patch has 8173658 as prerequisite which is currently
> out for review. It took me a few keystrokes and mouse clicks to figure this
> out. I'd suggest informing about prerequisite patches in the RFR mail.
> 
> Cheers, Richard.
> 
> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> On Behalf
> Of Lindenmaier, Goetz
> Sent: Dienstag, 17. November 2020 15:03
> To: jdk-updates-dev at openjdk.java.net
> Subject: [CAUTION] RE: [11u] RFR: 8212160: JVMTI agent crashes with
> "assert(_value != 0LL) failed: resolving NULL _value"
> 
> Hi,
> 
> I have another update to this change.
> I had to resolve it again because of the Minimal VM fix 8235218 pushed
> after 8173361.
> http://cr.openjdk.java.net/~goetz/wr20/8212160-jvmti_crash_resolving-
> jdk11/03/
> 
> Also, I reqested downport for Minimal VM fixes required after this change:
> JDK-8236124 Minimal VM slowdebug build failed after JDK-8212160
> JDK-8235456 Minimal VM is broken after JDK-8212160
> These both are clean downports once this is pushed.
> 
> I would appreciate reviews ��
> 
> Best regards,
>   Goetz.
> 
> 
> Hi,
> 
> Please look at
> http://cr.openjdk.java.net/~goetz/wr20/8212160-jvmti_crash_resolving-
> jdk11/02/
> 
> I had lost the changes to the test file because of C/C++ mismatch.
> 
> Best regards,
>   Goetz.
> 
> 
> -----Original Message-----
> From: Lindenmaier, Goetz
> Sent: Freitag, 6. November 2020 09:39
> To: jdk-updates-dev at openjdk.java.net
> Subject: [11u] RFR: 8212160: JVMTI agent crashes with "assert(_value != 0LL)
> failed: resolving NULL _value"
> 
> Hi
> 
> I am downporting this for parity with 11.0.10-oracle.
> 
> http://cr.openjdk.java.net/~goetz/wr20/8212160-jvmti_crash_resolving-
> jdk11/01/
> I had to resolve and fis in quite some places:
> 
> src/hotspot/share/code/nmethod.cpp
> Resolved. Few differences in code, e.g. MutexLocker --> MutexLockerEx.
> 
> Added 'this' to call of JvmtiDeferredEvent::compiled_method_unload_event()
> 
> src/hotspot/share/code/nmethod.hpp
> Trivial, resolved.
> 
> src/hotspot/share/prims/jvmtiCodeBlobEvents.cpp
> Resolved. Few differences in code, e.g. MutexLocker --> MutexLockerEx.
> 
> I removed NMethodIterator::only_alive_and_not_unloading and call
> next_alive instead.
> The NMethodIterator differs in head a lot.
> 
> src/hotspot/share/prims/jvmtiExport.cpp
> Resolved. Few differences in code, e.g. MutexLocker --> MutexLockerEx.
> 
> I added void JvmtiExport::post_compiled_method_load(JvmtiEnv* env,
> nmethod *nm)
> from head which is called in
> 
> src/hotspot/share/prims/jvmtiExport.hpp
> Resolved. Deleted method has different arguments.
> 
> src/hotspot/share/prims/jvmtiImpl.hpp
> Resolved.
> 
> src/hotspot/share/runtime/serviceThread.cpp
> Context different.
> 
> Please review.
> https://bugs.openjdk.java.net/browse/JDK-8212160
> https://hg.openjdk.java.net/jdk/jdk/rev/366c0f357ee6
> 
> Best regards,
>   Goetz