Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions substratevm/mx.substratevm/mx_substratevm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1820,10 +1820,14 @@ def cinterfacetutorial(args):
@mx.command(suite.name, 'javaagenttest', 'Runs tests for java agent with native image')
def java_agent_test(args):
def build_and_run(args, binary_path, native_image, agents, agents_arg):
test_cp = os.pathsep.join([classpath('com.oracle.svm.test')] + agents)
mx.log('Run agent with JVM as baseline')
test_cp = os.pathsep.join([classpath('com.oracle.svm.test')])
java_run_cp = os.pathsep.join([test_cp, mx.dependency('org.graalvm.nativeimage').classpath_repr()])
mx.run_java( agents_arg + ['-cp', java_run_cp, 'com.oracle.svm.test.javaagent.AgentTest'])
test_cp = os.pathsep.join([test_cp] + agents)
native_agent_premain_options = ['-XXpremain:com.oracle.svm.test.javaagent.agent1.TestJavaAgent1:test.agent1=true', '-XXpremain:com.oracle.svm.test.javaagent.agent2.TestJavaAgent2:test.agent2=true']
image_args = ['-cp', test_cp, '-J-ea', '-J-esa', '-H:+ReportExceptionStackTraces', '-H:Class=com.oracle.svm.test.javaagent.AgentTest']
native_image(image_args + svm_experimental_options(['-H:PremainClasses=' + agents_arg]) + ['-o', binary_path] + args)
native_image(image_args + svm_experimental_options(agents_arg) + ['-o', binary_path] + args)
mx.run([binary_path] + native_agent_premain_options)

def build_and_test_java_agent_image(native_image, args):
Expand All @@ -1840,18 +1844,24 @@ def build_and_test_java_agent_image(native_image, args):
# Note: we are not using MX here to avoid polluting the suite.py and requiring extra build flags
mx.log("Building agent jars from " + test_classpath)
agents = []
for i in range(1, 2):
for i in range(1, 3):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the agent3 in the PR? Is it committed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python's range(x, y) includes x but excludes y.

agent = join(tmp_dir, "testagent%d.jar" % (i))
agent_test_classpath = join(test_classpath, 'com', 'oracle', 'svm', 'test', 'javaagent', 'agent' + str(i))
class_list = [join(test_classpath, 'com', 'oracle', 'svm', 'test', 'javaagent', 'agent' + str(i), f) for f in os.listdir(agent_test_classpath) if os.path.isfile(os.path.join(agent_test_classpath, f)) and f.endswith(".class")]
mx.run([mx.get_jdk().jar, 'cmf', join(test_classpath, 'resources', 'javaagent' + str(i), 'MANIFEST.MF'), agent] + class_list, cwd = tmp_dir)
current_dir = os.getcwd()
# Change to test classpath to create agent jar file
os.chdir(test_classpath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to change the dir?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running jar cmf xxx.jar package/SomeClass.class to pack a jar file requires changing working directory to the root of class package path.

agent_test_classpath = join('com', 'oracle', 'svm', 'test', 'javaagent', 'agent' + str(i))
class_list = [join('com', 'oracle', 'svm', 'test', 'javaagent', 'agent' + str(i), f) for f in os.listdir(agent_test_classpath) if os.path.isfile(os.path.join(agent_test_classpath, f)) and f.endswith(".class")]
class_list.append(join('com', 'oracle', 'svm', 'test', 'javaagent', 'AgentPremainHelper.class'))
class_list.append(join('com', 'oracle', 'svm', 'test', 'javaagent', 'AssertInAgent.class'))
mx.run([mx.get_jdk().jar, 'cmf', join(test_classpath, 'resources', 'javaagent' + str(i), 'MANIFEST.MF'), agent] + class_list, cwd = test_classpath)
agents.append(agent)
os.chdir(current_dir)

mx.log("Building images with different agent orders ")
build_and_run(args, join(tmp_dir, 'agenttest1'), native_image, agents,'com.oracle.svm.test.javaagent.agent1.TestJavaAgent1,com.oracle.svm.test.javaagent.agent2.TestJavaAgent2')
build_and_run(args, join(tmp_dir, 'agenttest1'), native_image, agents,[f'-javaagent:{agents[0]}=test.agent1=true', f'-javaagent:{agents[1]}=test.agent2=true'])

# Switch the premain sequence of agent1 and agent2
build_and_run(args, join(tmp_dir, 'agenttest2'), native_image, agents, 'com.oracle.svm.test.javaagent.agent2.TestJavaAgent2,com.oracle.svm.test.javaagent.agent1.TestJavaAgent1')
build_and_run(args, join(tmp_dir, 'agenttest2'), native_image, agents, [f'-javaagent:{agents[1]}=test.agent2=true', f'-javaagent:{agents[0]}=test.agent1=true'])

native_image_context_run(build_and_test_java_agent_image, args)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, String ol
public static OptionEnabledHandler<Boolean> imageLayerEnabledHandler;
public static OptionEnabledHandler<Boolean> imageLayerCreateEnabledHandler;

@APIOption(name = "-javaagent", valueSeparator = ':')//
@Option(help = "Enable the specified java agent in native image. Usage: -javaagent:<jarpath>[=<options>]. " +
"The java agent will run at image build time to take its effects in the output native image. " +
"Be noticed: The java agent's premain method will be re-executed at native image runtime. " +
"The agent should isolate the executions according to runtime environment.", type = User, stability = OptionStability.EXPERIMENTAL)//
public static final HostedOptionKey<AccumulatingLocatableMultiOptionValue.Strings> JavaAgent = new HostedOptionKey<>(AccumulatingLocatableMultiOptionValue.Strings.build());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This partially conflicts with the basic JVMTI runtime support that was merged a couple weeks ago (see #9558). I will need to think about that a bit and I will try to come up with an approach to unify the JVMTI build- and runtime support.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does the conflict come from? Feels to me that with javaagent transformations happen mostly at build time (except the premain), while JVMTI provides hooks for run-time events.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a conflict, as you said they are working on different phases. Class transformation has nothing to do with JVMTI events.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both JVMTI and Java agents can be used to instrument Java classes. I was worried about those approaches conflicting. However, I think that you are right, this probably shouldn't result in worse situations than on HotSpot. Besides that, the same restrictions apply to both instrumentation approaches, so there is also no difference in terms of behavior there:

  • It is impossible to instrument most of the GraalVM-internal classes.
  • It is impossible to instrument certain JDK code (e.g., code that native image calls internally in low-level code, such as Throwable, java.lang.String, ...)
  • It is impossible to instrument code that Native Image substitutes (@TargetClass and similar annotations)


@Fold
public static boolean getSourceLevelDebug() {
return SourceLevelDebug.getValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,8 @@ private static <T> String oR(OptionKey<T> option) {
final String oHDeadlockWatchdogInterval = oH(SubstrateOptions.DeadlockWatchdogInterval);
final String oHLayerCreate = oH(SubstrateOptions.LayerCreate);

final String oHJavaAgent = oH(SubstrateOptions.JavaAgent);

final Map<String, String> imageBuilderEnvironment = new HashMap<>();
private final ArrayList<String> imageBuilderArgs = new ArrayList<>();
private final Set<String> imageBuilderUniqueLeftoverArgs = Collections.newSetFromMap(new IdentityHashMap<>());
Expand Down Expand Up @@ -1528,6 +1530,7 @@ private List<String> getAgentArguments() {
String agentOptions = "";
List<ArgumentEntry> traceClassInitializationOpts = getHostedOptionArgumentValues(imageBuilderArgs, oHTraceClassInitialization);
List<ArgumentEntry> traceObjectInstantiationOpts = getHostedOptionArgumentValues(imageBuilderArgs, oHTraceObjectInstantiation);
List<ArgumentEntry> javaAgentOpts = getHostedOptionArgumentValues(imageBuilderArgs, oHJavaAgent);
if (!traceClassInitializationOpts.isEmpty()) {
agentOptions = getAgentOptions(traceClassInitializationOpts, "c");
}
Expand All @@ -1546,6 +1549,12 @@ private List<String> getAgentArguments() {
args.add("-agentlib:native-image-diagnostics-agent=" + agentOptions);
}

if (!javaAgentOpts.isEmpty()) {
for (ArgumentEntry javaAgentOpt : javaAgentOpts) {
args.add("-javaagent:" + javaAgentOpt.value);
}
}

return args;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,33 +35,26 @@
import org.graalvm.nativeimage.hosted.Feature;

import com.oracle.svm.core.PreMainSupport;
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
import com.oracle.svm.core.feature.InternalFeature;
import com.oracle.svm.core.option.AccumulatingLocatableMultiOptionValue;
import com.oracle.svm.core.option.HostedOptionKey;
import com.oracle.svm.core.option.SubstrateOptionsParser;
import com.oracle.svm.core.util.BasedOnJDKFile;
import com.oracle.svm.core.util.UserError;
import com.oracle.svm.hosted.reflect.ReflectionFeature;

import jdk.graal.compiler.options.Option;
import jdk.graal.compiler.options.OptionStability;
import java.io.IOException;
import java.util.jar.JarFile;

/**
* This feature supports instrumentation in native image.
*/
@AutomaticallyRegisteredFeature
public class InstrumentFeature implements InternalFeature {
public static class Options {
@Option(help = "Specify premain-class list. Multiple classes are separated by comma, and order matters. This is an experimental option.", stability = OptionStability.EXPERIMENTAL)//
public static final HostedOptionKey<AccumulatingLocatableMultiOptionValue.Strings> PremainClasses = new HostedOptionKey<>(
AccumulatingLocatableMultiOptionValue.Strings.buildWithCommaDelimiter());

}

@Override
public boolean isInConfiguration(IsInConfigurationAccess access) {
return !Options.PremainClasses.getValue().values().isEmpty();
return !SubstrateOptions.JavaAgent.getValue().values().isEmpty();
}

@Override
Expand All @@ -77,28 +70,50 @@ public void afterRegistration(AfterRegistrationAccess access) {
PreMainSupport support = new PreMainSupport();
ImageSingletons.add(PreMainSupport.class, support);

List<String> premainClasses = Options.PremainClasses.getValue().values();
for (String clazz : premainClasses) {
addPremainClass(support, cl, clazz);
List<String> agentOptions = SubstrateOptions.JavaAgent.getValue().values();
for (String agentOption : agentOptions) {
addPremainClass(support, cl, agentOption);
}
}

private static void addPremainClass(PreMainSupport support, ClassLoader cl, String premainClass) {
private static void addPremainClass(PreMainSupport support, ClassLoader cl, String javaagentOption) {
int separatorIndex = javaagentOption.indexOf("=");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably use SubstrateUtil.split(...) instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jar file path which is prior to the first = is extracted here, while rest of the option that may contain more = should be kept in one piece for latter parsing.

String agent;
String premainClass = null;
String options = "";
// Get the agent file
if (separatorIndex == -1) {
agent = javaagentOption;
} else {
agent = javaagentOption.substring(0, separatorIndex);
options = javaagentOption.substring(separatorIndex + 1);
}
// Read MANIFEST in agent jar
try {
JarFile agentJarFile = new JarFile(agent);
premainClass = agentJarFile.getManifest().getMainAttributes().getValue("Premain-Class");
Copy link
Member

@vjovanov vjovanov Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when there is no Manifest? There should be a special error message for that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this moment, the java agent has been already loaded and run by JVM.
If there is no Manifest, the JVM can't get started with -javaagent.
But in the proxy agent case, the actual agent will be loaded by -cp, we should check the existence of Manifest then.

Copy link
Member

@christianhaeubl christianhaeubl Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, HotSpot already executed the method PremainClass.premain during the image build but native image should execute the same method again at run-time?

I think that this will only work for a very small number of Java agents (i.e., agents that contain Native Image-specific logic or that hardly execute any code in premain). If so, then I don't think that we should register the premain method automatically and use an opt-in approach instead.

Ideally, the agent manifest would contained some information to identify if the premain method supports re-execution at run-time. Alternatively, we could execute a different method as the Java agent probably needs to explicitly support native image anyways?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From users' aspect, they need to manually isolate premain logic in JVM runtime and native image runtime anyway, a smoother and more convenient approach would be better.
Some of the agent premain logic could be shared between these two runtimes. Take OT agent for example, we carefully isolated class transformation and other staffs by runtime environment. The modifications are adding runtime environment checks for a few code sections, and most of the premain code is untouched. In this case, using a different method from premain will reduce the reusing of existing code.

It is possible some simple agents don't have complicated framework to init in premain as OT agent. It is not necessary to re-execute the premain in native image. But the end users may not know such details. We should not try to educate them when to use which option to enable premain and when to omit it. It should leave to agent developer to decide how to implement the premain, and the end users just use it as what they do in JVM mode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christianhaeubl the correct solution here would be that the Java agents don't do both transformation and initialization from the premain method, but have two methods instead. However, this would require a spec change which we can't do that easily.

We are hoping here that most of the users won't need to re-write their agents as the Instrumentation we pass ignores all the method calls and says that no classes are transformable.

We are implementing this as an experimental feature to experiment, and if it works great. If most of the agents need to be rewritten, then I would go with a two-method solution that you propose.

} catch (IOException e) {
// This should never happen because the image build process (HotSpot) already loaded the
// agent during startup.
throw UserError.abort(e, "Can't read the agent jar %s. Please check option %s", agent,
SubstrateOptionsParser.commandArgument(SubstrateOptions.JavaAgent, ""));
}

try {
Class<?> clazz = Class.forName(premainClass, false, cl);
Method premain = findPremainMethod(premainClass, clazz);

List<Object> args = new ArrayList<>();
/* The first argument contains the premain options, which will be set at runtime. */
args.add("");
args.add(options);
if (premain.getParameterCount() == 2) {
args.add(new PreMainSupport.NativeImageNoOpRuntimeInstrumentation());
}

support.registerPremainMethod(premainClass, premain, args.toArray(new Object[0]));
} catch (ClassNotFoundException e) {
UserError.abort("Could not register agent premain method because class %s was not found. Please check your %s setting.", premainClass,
SubstrateOptionsParser.commandArgument(Options.PremainClasses, ""));
throw UserError.abort("Could not register agent premain method because class %s was not found. Please check your %s setting.", premainClass,
SubstrateOptionsParser.commandArgument(SubstrateOptions.JavaAgent, ""));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2024, 2024, Alibaba Group Holding Limited. All rights reserved.
* Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2025, 2025, Alibaba Group Holding Limited. 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
Expand Down Expand Up @@ -36,14 +36,6 @@ public static void load(Class<?> agentClass) {
}
}

public static String getFirst() {
return System.getProperty("first.load.agent");
}

public static String getSecond() {
return System.getProperty("second.load.agent");
}

public static void parseOptions(String agentArgs) {
if (agentArgs != null && !agentArgs.isBlank()) {
String[] argPairs = agentArgs.split(",");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ private static void testAgentOptions() {
}

private static void testPremainSequence() {
String first = AgentPremainHelper.getFirst();
String second = AgentPremainHelper.getSecond();
String first = System.getProperty("first.load.agent");
String second = System.getProperty("second.load.agent");
Assert.assertNotNull(first);
if (second != null) {
String agentName = TestJavaAgent1.class.getName();
Expand All @@ -58,10 +58,20 @@ private static void testPremainSequence() {
}
}

private static void testInstrumentation() {
// The return value of getCounter() should be changed by agent
Assert.assertEquals(11, getCounter());
}

private static int getCounter() {
return 10;
}

public static void main(String[] args) {
testPremain();
testAgentOptions();
testPremainSequence();
testInstrumentation();
System.out.println("Finished running Agent test.");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2025, 2025, Alibaba Group Holding Limited. 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. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* 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.
*/

package com.oracle.svm.test.javaagent;

/**
* Assertions used inside agent when not using JUNIT.
*/
public class AssertInAgent {
public static void assertNotNull(Object o) {
if (o == null) {
throw new RuntimeException("Object input is null, but expected to be non-null");
}
}

public static void assertEquals(boolean expected, boolean actual) {
if (expected != actual) {
throw new RuntimeException(String.format("Expected(%s) is not equal to actual(%s)", expected, actual));
}
}

public static void assertEquals(long expected, long actual) {
if (expected != actual) {
throw new RuntimeException(String.format("Expected(%s) is not equal to actual(%s)", expected, actual));
}
}

public static void assertEquals(Object expected, Object actual) {
if (expected != actual) {
if (expected != null) {
assertNotNull(actual);
if (!expected.equals(actual)) {
throw new RuntimeException(String.format("Expected(%s) is not equal to actual(%s)", expected, actual));
}
} else {
throw new RuntimeException(String.format("Expected(null) is not equal to actual(%s)", actual));
}
}
}

public static void assertTrue(boolean actual) {
assertEquals(true, actual);
}
}
Loading
Loading