From 1a07d542ec810282eb78653698d098a24b35686f Mon Sep 17 00:00:00 2001 From: David Holmes Date: Thu, 28 Nov 2024 02:24:55 +0000 Subject: [PATCH] 8343703: Symbol name cleanups after JEP 479 Reviewed-by: kbarrett, amenkov --- src/hotspot/share/include/jvm.h | 7 --- src/hotspot/share/prims/jvmtiAgent.cpp | 39 +++++++-------- src/hotspot/share/runtime/os.cpp | 32 ++++-------- src/hotspot/share/runtime/os.hpp | 6 +-- .../share/native/libjava/NativeLibraries.c | 50 +++++++------------ 5 files changed, 46 insertions(+), 88 deletions(-) diff --git a/src/hotspot/share/include/jvm.h b/src/hotspot/share/include/jvm.h index bc46a2cf852..474d5baa487 100644 --- a/src/hotspot/share/include/jvm.h +++ b/src/hotspot/share/include/jvm.h @@ -1147,13 +1147,6 @@ JVM_GetClassFileVersion(JNIEnv *env, jclass current); JNIEXPORT jboolean JNICALL JVM_PrintWarningAtDynamicAgentLoad(void); -#define JNI_ONLOAD_SYMBOLS {"JNI_OnLoad"} -#define JNI_ONUNLOAD_SYMBOLS {"JNI_OnUnload"} -#define JVM_ONLOAD_SYMBOLS {"JVM_OnLoad"} -#define AGENT_ONLOAD_SYMBOLS {"Agent_OnLoad"} -#define AGENT_ONUNLOAD_SYMBOLS {"Agent_OnUnload"} -#define AGENT_ONATTACH_SYMBOLS {"Agent_OnAttach"} - /* * This structure is used by the launcher to get the default thread * stack size from the VM using JNI_GetDefaultJavaVMInitArgs() with a diff --git a/src/hotspot/share/prims/jvmtiAgent.cpp b/src/hotspot/share/prims/jvmtiAgent.cpp index 6dd205082a0..06702773d58 100644 --- a/src/hotspot/share/prims/jvmtiAgent.cpp +++ b/src/hotspot/share/prims/jvmtiAgent.cpp @@ -27,7 +27,6 @@ #include "cds/cds_globals.hpp" #include "cds/cdsConfig.hpp" #include "jni.h" -#include "jvm.h" #include "jvm_io.h" #include "jvmtifiles/jvmtiEnv.hpp" #include "prims/jvmtiEnvBase.hpp" @@ -268,10 +267,10 @@ static void assert_preload(const JvmtiAgent* agent) { // For statically linked agents we can't rely on os_lib == nullptr because // statically linked agents could have a handle of RTLD_DEFAULT which == 0 on some platforms. // If this function returns true, then agent->is_static_lib() && agent->is_loaded(). -static bool load_agent_from_executable(JvmtiAgent* agent, const char* on_load_symbols[], size_t num_symbol_entries) { +static bool load_agent_from_executable(JvmtiAgent* agent, const char* on_load_symbol) { DEBUG_ONLY(assert_preload(agent);) - assert(on_load_symbols != nullptr, "invariant"); - return os::find_builtin_agent(agent, &on_load_symbols[0], num_symbol_entries); + assert(on_load_symbol != nullptr, "invariant"); + return os::find_builtin_agent(agent, on_load_symbol); } // Load the library from the absolute path of the agent, if available. @@ -310,7 +309,7 @@ static void* load_agent_from_relative_path(JvmtiAgent* agent, bool vm_exit_on_er } // For absolute and relative paths. -static void* load_library(JvmtiAgent* agent, const char* on_symbols[], size_t num_symbol_entries, bool vm_exit_on_error) { +static void* load_library(JvmtiAgent* agent, bool vm_exit_on_error) { return agent->is_absolute_path() ? load_agent_from_absolute_path(agent, vm_exit_on_error) : load_agent_from_relative_path(agent, vm_exit_on_error); } @@ -321,12 +320,11 @@ extern "C" { } // Find the OnLoad entry point for -agentlib: -agentpath: -Xrun agents. -// num_symbol_entries must be passed-in since only the caller knows the number of symbols in the array. -static OnLoadEntry_t lookup_On_Load_entry_point(JvmtiAgent* agent, const char* on_load_symbols[], size_t num_symbol_entries) { +static OnLoadEntry_t lookup_On_Load_entry_point(JvmtiAgent* agent, const char* on_load_symbol) { assert(agent != nullptr, "invariant"); if (!agent->is_loaded()) { - if (!load_agent_from_executable(agent, on_load_symbols, num_symbol_entries)) { - void* const library = load_library(agent, on_load_symbols, num_symbol_entries, /* vm exit on error */ true); + if (!load_agent_from_executable(agent, on_load_symbol)) { + void* const library = load_library(agent, /* vm exit on error */ true); assert(library != nullptr, "invariant"); agent->set_os_lib(library); agent->set_loaded(); @@ -334,17 +332,15 @@ static OnLoadEntry_t lookup_On_Load_entry_point(JvmtiAgent* agent, const char* o } assert(agent->is_loaded(), "invariant"); // Find the OnLoad function. - return CAST_TO_FN_PTR(OnLoadEntry_t, os::find_agent_function(agent, false, on_load_symbols, num_symbol_entries)); + return CAST_TO_FN_PTR(OnLoadEntry_t, os::find_agent_function(agent, false, on_load_symbol)); } static OnLoadEntry_t lookup_JVM_OnLoad_entry_point(JvmtiAgent* lib) { - const char* on_load_symbols[] = JVM_ONLOAD_SYMBOLS; - return lookup_On_Load_entry_point(lib, on_load_symbols, sizeof(on_load_symbols) / sizeof(char*)); + return lookup_On_Load_entry_point(lib, "JVM_OnLoad"); } static OnLoadEntry_t lookup_Agent_OnLoad_entry_point(JvmtiAgent* agent) { - const char* on_load_symbols[] = AGENT_ONLOAD_SYMBOLS; - return lookup_On_Load_entry_point(agent, on_load_symbols, sizeof(on_load_symbols) / sizeof(char*)); + return lookup_On_Load_entry_point(agent, "Agent_OnLoad"); } void JvmtiAgent::convert_xrun_agent() { @@ -499,14 +495,13 @@ static bool invoke_Agent_OnAttach(JvmtiAgent* agent, outputStream* st) { assert(agent->is_dynamic(), "invariant"); assert(st != nullptr, "invariant"); assert(JvmtiEnvBase::get_phase() == JVMTI_PHASE_LIVE, "not in live phase!"); - const char* on_attach_symbols[] = AGENT_ONATTACH_SYMBOLS; - const size_t num_symbol_entries = ARRAY_SIZE(on_attach_symbols); + const char* on_attach_symbol = "Agent_OnAttach"; void* library = nullptr; bool previously_loaded; - if (load_agent_from_executable(agent, &on_attach_symbols[0], num_symbol_entries)) { + if (load_agent_from_executable(agent, on_attach_symbol)) { previously_loaded = JvmtiAgentList::is_static_lib_loaded(agent->name()); } else { - library = load_library(agent, &on_attach_symbols[0], num_symbol_entries, /* vm_exit_on_error */ false); + library = load_library(agent, /* vm_exit_on_error */ false); if (library == nullptr) { st->print_cr("%s was not loaded.", agent->name()); if (*ebuf != '\0') { @@ -531,10 +526,10 @@ static bool invoke_Agent_OnAttach(JvmtiAgent* agent, outputStream* st) { assert(agent->is_loaded(), "invariant"); // The library was loaded so we attempt to lookup and invoke the Agent_OnAttach function. OnAttachEntry_t on_attach_entry = CAST_TO_FN_PTR(OnAttachEntry_t, - os::find_agent_function(agent, false, &on_attach_symbols[0], num_symbol_entries)); + os::find_agent_function(agent, false, on_attach_symbol)); if (on_attach_entry == nullptr) { - st->print_cr("%s is not available in %s", on_attach_symbols[0], agent->name()); + st->print_cr("%s is not available in %s", on_attach_symbol, agent->name()); unload_library(agent, library); return false; } @@ -629,10 +624,10 @@ extern "C" { } void JvmtiAgent::unload() { - const char* on_unload_symbols[] = AGENT_ONUNLOAD_SYMBOLS; + const char* on_unload_symbol = "Agent_OnUnload"; // Find the Agent_OnUnload function. Agent_OnUnload_t unload_entry = CAST_TO_FN_PTR(Agent_OnUnload_t, - os::find_agent_function(this, false, &on_unload_symbols[0], ARRAY_SIZE(on_unload_symbols))); + os::find_agent_function(this, false, on_unload_symbol)); if (unload_entry != nullptr) { // Invoke the Agent_OnUnload function JavaThread* thread = JavaThread::current(); diff --git a/src/hotspot/share/runtime/os.cpp b/src/hotspot/share/runtime/os.cpp index d8e539ca115..deb2c30500c 100644 --- a/src/hotspot/share/runtime/os.cpp +++ b/src/hotspot/share/runtime/os.cpp @@ -546,49 +546,35 @@ void* os::native_java_library() { * executable if agent_lib->is_static_lib() == true or in the shared library * referenced by 'handle'. */ -void* os::find_agent_function(JvmtiAgent *agent_lib, bool check_lib, - const char *syms[], size_t syms_len) { +void* os::find_agent_function(JvmtiAgent *agent_lib, bool check_lib, const char *sym) { assert(agent_lib != nullptr, "sanity check"); - const char *lib_name; void *handle = agent_lib->os_lib(); void *entryName = nullptr; - char *agent_function_name; - size_t i; // If checking then use the agent name otherwise test is_static_lib() to // see how to process this lookup - lib_name = ((check_lib || agent_lib->is_static_lib()) ? agent_lib->name() : nullptr); - for (i = 0; i < syms_len; i++) { - agent_function_name = build_agent_function_name(syms[i], lib_name, agent_lib->is_absolute_path()); - if (agent_function_name == nullptr) { - break; - } + const char *lib_name = ((check_lib || agent_lib->is_static_lib()) ? agent_lib->name() : nullptr); + + char* agent_function_name = build_agent_function_name(sym, lib_name, agent_lib->is_absolute_path()); + if (agent_function_name != nullptr) { entryName = dll_lookup(handle, agent_function_name); FREE_C_HEAP_ARRAY(char, agent_function_name); - if (entryName != nullptr) { - break; - } } return entryName; } // See if the passed in agent is statically linked into the VM image. -bool os::find_builtin_agent(JvmtiAgent* agent, const char *syms[], - size_t syms_len) { - void *ret; - void *proc_handle; - void *save_handle; - +bool os::find_builtin_agent(JvmtiAgent* agent, const char* sym) { assert(agent != nullptr, "sanity check"); if (agent->name() == nullptr) { return false; } - proc_handle = get_default_process_handle(); + void* proc_handle = get_default_process_handle(); // Check for Agent_OnLoad/Attach_lib_name function - save_handle = agent->os_lib(); + void* save_handle = agent->os_lib(); // We want to look in this process' symbol table. agent->set_os_lib(proc_handle); - ret = find_agent_function(agent, true, syms, syms_len); + void* ret = find_agent_function(agent, true, sym); if (ret != nullptr) { // Found an entry point like Agent_OnLoad_lib_name so we have a static agent agent->set_static_lib(); diff --git a/src/hotspot/share/runtime/os.hpp b/src/hotspot/share/runtime/os.hpp index 0bc543957c9..2e52440ead4 100644 --- a/src/hotspot/share/runtime/os.hpp +++ b/src/hotspot/share/runtime/os.hpp @@ -777,12 +777,10 @@ class os: AllStatic { static void* get_default_process_handle(); // Check for static linked agent library - static bool find_builtin_agent(JvmtiAgent *agent_lib, const char *syms[], - size_t syms_len); + static bool find_builtin_agent(JvmtiAgent* agent_lib, const char* sym); // Find agent entry point - static void *find_agent_function(JvmtiAgent *agent_lib, bool check_lib, - const char *syms[], size_t syms_len); + static void* find_agent_function(JvmtiAgent* agent_lib, bool check_lib, const char* sym); // Provide wrapper versions of these functions to guarantee NUL-termination // in all cases. diff --git a/src/java.base/share/native/libjava/NativeLibraries.c b/src/java.base/share/native/libjava/NativeLibraries.c index df5ff343d61..b6624eccd1d 100644 --- a/src/java.base/share/native/libjava/NativeLibraries.c +++ b/src/java.base/share/native/libjava/NativeLibraries.c @@ -64,45 +64,31 @@ static jboolean initIDs(JNIEnv *env) */ static void *findJniFunction(JNIEnv *env, void *handle, const char *cname, jboolean isLoad) { - const char *onLoadSymbols[] = JNI_ONLOAD_SYMBOLS; - const char *onUnloadSymbols[] = JNI_ONUNLOAD_SYMBOLS; - const char **syms; - int symsLen; + const char *sym; void *entryName = NULL; char *jniFunctionName; - int i; size_t len; // Check for JNI_On(Un)Load<_libname> function - if (isLoad) { - syms = onLoadSymbols; - symsLen = sizeof(onLoadSymbols) / sizeof(char *); - } else { - syms = onUnloadSymbols; - symsLen = sizeof(onUnloadSymbols) / sizeof(char *); + sym = isLoad ? "JNI_OnLoad" : "JNI_OnUnload"; + + // sym + '_' + cname + '\0' + if ((len = strlen(sym) + (cname != NULL ? (strlen(cname) + 1) : 0) + 1) > + FILENAME_MAX) { + goto done; } - for (i = 0; i < symsLen; i++) { - // cname + sym + '_' + '\0' - if ((len = (cname != NULL ? strlen(cname) : 0) + strlen(syms[i]) + 2) > - FILENAME_MAX) { - goto done; - } - jniFunctionName = malloc(len); - if (jniFunctionName == NULL) { - JNU_ThrowOutOfMemoryError(env, NULL); - goto done; - } - strcpy(jniFunctionName, syms[i]); - if (cname != NULL) { - strcat(jniFunctionName, "_"); - strcat(jniFunctionName, cname); - } - entryName = JVM_FindLibraryEntry(handle, jniFunctionName); - free(jniFunctionName); - if(entryName) { - break; - } + jniFunctionName = malloc(len); + if (jniFunctionName == NULL) { + JNU_ThrowOutOfMemoryError(env, NULL); + goto done; } + strcpy(jniFunctionName, sym); + if (cname != NULL) { + strcat(jniFunctionName, "_"); + strcat(jniFunctionName, cname); + } + entryName = JVM_FindLibraryEntry(handle, jniFunctionName); + free(jniFunctionName); done: return entryName;