On Mon, 17 Mar 2025 19:37:48 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:

> Please review this fix that avoids `JvmtiAgent::convert_xrun_agent` from 
> prematurely exiting VM if `lookup_On_Load_entry_point` cannot load the agent 
> using `JVM_OnLoad` symbol. Thanks
> 
> `lookup_On_Load_entry_point` first tries to load the builtin agent from the 
> executable by checking the requested symbol (`JVM_OnLoad`). If no builtin 
> agent is found, it then tries to load the agent shared library (e.g. 
> `libjdwp.so`) by calling `load_library`. The issue is that `load_library` is 
> called with `vm_exit_on_error` set to `true`, which causes the VM to exit 
> immediately if the agent shared library is not loaded. Therefore, 
> `JvmtiAgent::convert_xrun_agent` has no chance to try loading the agent using 
> `Agent_OnLoad` symbol 
> (https://github.com/openjdk/jdk/blob/19154f7af34bf6f13d61d7a9f05d6277964845d8/src/hotspot/share/prims/jvmtiAgent.cpp#L352).
>  This is a hidden issue on regular JDK, since the `load_library` can 
> successfully find the agent shared library when 
> `JvmtiAgent::convert_xrun_agent` first tries to load the agent using 
> `JVM_OnLoad` symbol. The issue is noticed on static JDK as there is no 
> `libjdwp.so` in static JDK. It can be reproduced with jtreg 
> `runtime/6294277/Sou
 rceDebugExtension.java` test.
> 
> As part of the fix, I cleaned up following in `invoke_JVM_OnLoad` and 
> `invoke_Agent_OnLoad`. If there's an error, the VM should already have exited 
> during `lookup_<JVM|Agent>_OnLoad_entry_point` in those cases.
> 
> 
>   if (on_load_entry == nullptr) {
>     vm_exit_during_initialization("Could not find ... function in -Xrun 
> library", agent->name());
>   }

This seems reasonable to me. A couple of nits.

Thanks

src/hotspot/share/prims/jvmtiAgent.cpp line 360:

> 358:   // to try lookup_Agent_OnLoad_entry_point for Agent_OnLoad as well.
> 359:   OnLoadEntry_t on_load_entry = lookup_JVM_OnLoad_entry_point(
> 360:     this, /* vm exit on error */ false);

Suggestion:

  OnLoadEntry_t on_load_entry = lookup_JVM_OnLoad_entry_point(this, /* vm exit 
on error */ false);

src/hotspot/share/prims/jvmtiAgent.cpp line 365:

> 363:   if (on_load_entry == nullptr) {
> 364:     on_load_entry = lookup_Agent_OnLoad_entry_point(
> 365:       this, /* vm exit on error */ true);

Suggestion:

    on_load_entry = lookup_Agent_OnLoad_entry_point(this, /* vm exit on error 
*/ true);

-------------

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24086#pullrequestreview-2692846277
PR Review Comment: https://git.openjdk.org/jdk/pull/24086#discussion_r2000148624
PR Review Comment: https://git.openjdk.org/jdk/pull/24086#discussion_r2000148978

Reply via email to