diff --git a/src/hotspot/share/prims/jvmtiAgentList.cpp b/src/hotspot/share/prims/jvmtiAgentList.cpp index a32eeb7076c..2a0f7172888 100644 --- a/src/hotspot/share/prims/jvmtiAgentList.cpp +++ b/src/hotspot/share/prims/jvmtiAgentList.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -124,11 +124,11 @@ void JvmtiAgentList::add(JvmtiAgent* agent) { } while (Atomic::cmpxchg(&_list, next, agent) != next); } -void JvmtiAgentList::add(const char* name, char* options, bool absolute_path) { +void JvmtiAgentList::add(const char* name, const char* options, bool absolute_path) { add(new JvmtiAgent(name, options, absolute_path)); } -void JvmtiAgentList::add_xrun(const char* name, char* options, bool absolute_path) { +void JvmtiAgentList::add_xrun(const char* name, const char* options, bool absolute_path) { JvmtiAgent* agent = new JvmtiAgent(name, options, absolute_path); agent->set_xrun(); add(agent); @@ -198,18 +198,14 @@ void JvmtiAgentList::load_xrun_agents() { } // Invokes Agent_OnAttach for agents loaded dynamically during runtime. -jint JvmtiAgentList::load_agent(const char* agent_name, const char* absParam, - const char* options, outputStream* st) { - // The abs parameter should be "true" or "false" - const bool is_absolute_path = (absParam != nullptr) && (strcmp(absParam, "true") == 0); +void JvmtiAgentList::load_agent(const char* agent_name, bool is_absolute_path, + const char* options, outputStream* st) { JvmtiAgent* const agent = new JvmtiAgent(agent_name, options, is_absolute_path, /* dynamic agent */ true); if (agent->load(st)) { add(agent); } else { delete agent; } - // Agent_OnAttach executed so completion status is JNI_OK - return JNI_OK; } // Send any Agent_OnUnload notifications diff --git a/src/hotspot/share/prims/jvmtiAgentList.hpp b/src/hotspot/share/prims/jvmtiAgentList.hpp index cf698c69c01..c4a497dfa9e 100644 --- a/src/hotspot/share/prims/jvmtiAgentList.hpp +++ b/src/hotspot/share/prims/jvmtiAgentList.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -65,14 +65,15 @@ class JvmtiAgentList : AllStatic { static void initialize(); static void convert_xrun_agents(); - public: static void add(JvmtiAgent* agent) NOT_JVMTI_RETURN; - static void add(const char* name, char* options, bool absolute_path) NOT_JVMTI_RETURN; - static void add_xrun(const char* name, char* options, bool absolute_path) NOT_JVMTI_RETURN; + + public: + static void add(const char* name, const char* options, bool absolute_path) NOT_JVMTI_RETURN; + static void add_xrun(const char* name, const char* options, bool absolute_path) NOT_JVMTI_RETURN; static void load_agents() NOT_JVMTI_RETURN; - static jint load_agent(const char* agent, const char* absParam, - const char* options, outputStream* st) NOT_JVMTI_RETURN_(0); + static void load_agent(const char* agent, bool is_absolute_path, + const char* options, outputStream* st) NOT_JVMTI_RETURN; static void load_xrun_agents() NOT_JVMTI_RETURN; static void unload_agents() NOT_JVMTI_RETURN; diff --git a/src/hotspot/share/services/attachListener.cpp b/src/hotspot/share/services/attachListener.cpp index 545563ba8ec..40b1a744018 100644 --- a/src/hotspot/share/services/attachListener.cpp +++ b/src/hotspot/share/services/attachListener.cpp @@ -135,7 +135,12 @@ static jint load_agent(AttachOperation* op, outputStream* out) { } } - return JvmtiAgentList::load_agent(agent, absParam, options, out); + // The abs parameter should be "true" or "false". + const bool is_absolute_path = (absParam != nullptr) && (strcmp(absParam, "true") == 0); + JvmtiAgentList::load_agent(agent, is_absolute_path, options, out); + + // Agent_OnAttach result or error message is written to 'out'. + return JNI_OK; } // Implementation of "properties" command. diff --git a/src/hotspot/share/services/diagnosticCommand.cpp b/src/hotspot/share/services/diagnosticCommand.cpp index 21c9b2a8f94..6e6a7172290 100644 --- a/src/hotspot/share/services/diagnosticCommand.cpp +++ b/src/hotspot/share/services/diagnosticCommand.cpp @@ -308,7 +308,7 @@ void JVMTIAgentLoadDCmd::execute(DCmdSource source, TRAPS) { if (is_java_agent) { if (_option.value() == nullptr) { - JvmtiAgentList::load_agent("instrument", "false", _libpath.value(), output()); + JvmtiAgentList::load_agent("instrument", false, _libpath.value(), output()); } else { size_t opt_len = strlen(_libpath.value()) + strlen(_option.value()) + 2; if (opt_len > 4096) { @@ -325,12 +325,12 @@ void JVMTIAgentLoadDCmd::execute(DCmdSource source, TRAPS) { } jio_snprintf(opt, opt_len, "%s=%s", _libpath.value(), _option.value()); - JvmtiAgentList::load_agent("instrument", "false", opt, output()); + JvmtiAgentList::load_agent("instrument", false, opt, output()); os::free(opt); } } else { - JvmtiAgentList::load_agent(_libpath.value(), "true", _option.value(), output()); + JvmtiAgentList::load_agent(_libpath.value(), true, _option.value(), output()); } } diff --git a/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java b/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java index 2c2e865a086..93656cdf513 100644 --- a/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java +++ b/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -96,13 +96,14 @@ public abstract class HotSpotVirtualMachine extends VirtualMachine { } String msgPrefix = "return code: "; - InputStream in = execute("load", - agentLibrary, - isAbsolute ? "true" : "false", - options); - try (BufferedReader reader = new BufferedReader(new InputStreamReader(in))) { - String result = reader.readLine(); - if (result == null) { + String errorMsg = "Failed to load agent library"; + try { + InputStream in = execute("load", + agentLibrary, + isAbsolute ? "true" : "false", + options); + String result = readErrorMessage(in); + if (result.isEmpty()) { throw new AgentLoadException("Target VM did not respond"); } else if (result.startsWith(msgPrefix)) { int retCode = Integer.parseInt(result.substring(msgPrefix.length())); @@ -110,8 +111,15 @@ public abstract class HotSpotVirtualMachine extends VirtualMachine { throw new AgentInitializationException("Agent_OnAttach failed", retCode); } } else { - throw new AgentLoadException(result); + if (!result.isEmpty()) { + errorMsg += ": " + result; + } + throw new AgentLoadException(errorMsg); } + } catch (AttachOperationFailedException ex) { + // execute() throws AttachOperationFailedException if attach agent reported error. + // Convert it to AgentLoadException. + throw new AgentLoadException(errorMsg + ": " + ex.getMessage()); } } @@ -364,6 +372,9 @@ public abstract class HotSpotVirtualMachine extends VirtualMachine { StringBuilder message = new StringBuilder(); BufferedReader br = new BufferedReader(new InputStreamReader(in)); while ((s = br.readLine()) != null) { + if (message.length() > 0) { + message.append(' '); + } message.append(s); } return message.toString(); @@ -399,20 +410,10 @@ public abstract class HotSpotVirtualMachine extends VirtualMachine { throw new IOException("Protocol mismatch with target VM"); } - // Special-case the "load" command so that the right exception is - // thrown. - if (cmd.equals("load")) { - String msg = "Failed to load agent library"; - if (!message.isEmpty()) { - msg += ": " + message; - } - throw new AgentLoadException(msg); - } else { - if (message.isEmpty()) { - message = "Command failed in target VM"; - } - throw new AttachOperationFailedException(message); + if (message.isEmpty()) { + message = "Command failed in target VM"; } + throw new AttachOperationFailedException(message); } } diff --git a/test/jdk/com/sun/tools/attach/FailedLoadAgentTest.java b/test/jdk/com/sun/tools/attach/FailedLoadAgentTest.java new file mode 100644 index 00000000000..d4e15ef6e52 --- /dev/null +++ b/test/jdk/com/sun/tools/attach/FailedLoadAgentTest.java @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8325530 + * @summary Test that failed VirtualMachine.loadAgentPath/loadAgentLibrary reports detailed reason + * @requires vm.jvmti + * @modules jdk.attach + * @library /test/lib + * @run driver FailedLoadAgentTest + */ + +import java.nio.file.Path; +import com.sun.tools.attach.AgentLoadException; +import com.sun.tools.attach.VirtualMachine; + +import jdk.test.lib.Platform; +import jdk.test.lib.Utils; +import jdk.test.lib.apps.LingeredApp; + +public class FailedLoadAgentTest { + private static final String jvmtiAgentLib = "FailedLoadAgentTestNotExists"; + private static final String jvmtiAgentPath = getLibPath(jvmtiAgentLib); + + private static String getLibPath(String libName) { + String fullName = Platform.buildSharedLibraryName(libName); + return Path.of(Utils.TEST_NATIVE_PATH, fullName).toAbsolutePath().toString(); + } + + private interface TestAction { + void test() throws Exception; + } + + private static void test(TestAction action) throws Exception { + try { + action.test(); + throw new RuntimeException("AgentLoadException not thrown"); + } catch (AgentLoadException ex) { + System.out.println("AgentLoadException thrown as expected:"); + ex.printStackTrace(System.out); + String msg = ex.getMessage(); + // Attach agent prints general " was not loaded." message on error. + // But additionally we expect detailed message with the reason. + String parts[] = msg.split("was not loaded."); + if (parts.length < 2 || parts[1].isEmpty()) { + throw new RuntimeException("AgentLoadException message is vague"); + } + } + } + + public static void main(String[] args) throws Exception { + LingeredApp theApp = null; + try { + theApp = new LingeredApp(); + LingeredApp.startApp(theApp, "-XX:+EnableDynamicAgentLoading"); + + VirtualMachine vm = VirtualMachine.attach(Long.toString(theApp.getPid())); + + // absolute path + test(() -> vm.loadAgentPath(jvmtiAgentPath)); + // relative path + test(() -> vm.loadAgentLibrary(jvmtiAgentLib)); + + } finally { + LingeredApp.stopApp(theApp); + } + } + +}