8325530: Vague error message when com.sun.tools.attach.VirtualMachine fails to load agent library

Reviewed-by: sspitsyn, cjplummer
This commit is contained in:
Alex Menkov 2024-02-26 23:18:49 +00:00
parent b87d9cf2c9
commit fc67c2b4f1
6 changed files with 134 additions and 41 deletions

View File

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

View File

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

View File

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

View File

@ -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());
}
}

View File

@ -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);
}
}

View File

@ -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 "<agent> 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);
}
}
}