Add premain for static agent support #8988

ziyilin · 2024-05-29T07:42:17Z

Java agent always has a premain method to initialize the agent. SVM needs the premain as well to support agent in native image.

At compile time, -H:PremainClasses= option is used to set the premain classes.
At runtime, premain runtime options are set along with main class' arguments in the format of -XX-premain:[class]:[options]

This PR is part of #8077

vjovanov · 2024-06-12T05:56:15Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/PreMainSupport.java

+
+ private static final String PREMAIN_OPTION_PREFIX = "-XX-premain:";
+
+ class PremainMethod {


This could be a record, right?

Should be static if it is a class.

This could be a record, right?
Yes, in my current implementation it's recorded by native-image-agent.

This could be a record, right?

Yes, in my current implementation it's recorded by native-image-agent.

When I say a record I mean a java record.

I see. Yes, this is a java record. I have fixed it.

vjovanov · 2024-06-12T06:00:32Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/PreMainSupport.java

+ // premain method must be static
+ premainMethod.method.invoke(null, args);
+ } catch (Throwable t) {
+ VMError.shouldNotReachHere("Fail to execute " + premainMethod.className + ".premain", t);


This should be a user-facing error, right? User code can throw, so I don't see this as something that is a VMError. What does the JVM do in this case?

In JVM, if the premain method fails, the JVM starting process shall be terminated. See https://github.com/openjdk/jdk21u/blob/2971cb5769121b47ac8c8db1078d67680a19341f/src/java.instrument/share/native/libinstrument/InvocationAdapter.c#L623-L629

I use VMError because it's a fatal error during VM starts up. But the cause is indeed an user error.

vjovanov · 2024-06-12T06:01:22Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/PreMainSupport.java

+ * registered premain method's second parameter. At native image runtime, no actual
+ * instrumentation work can do. So all the methods here are empty.
+ */
+ public static class SVMRuntimeInstrumentImpl implements Instrumentation {


Maybe NativeImageNoOpRuntimeInstrumentation? SVM is an internal name which we should not use. I also feel we should mention it is a noop and that it happens at runtime.

vjovanov · 2024-06-12T06:04:58Z

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/InstrumentFeature.java

+ }
+ preMainSupport.registerPremainMethod(premainClass, premain, args.toArray(new Object[0]));
+ } catch (ClassNotFoundException e) {
+ VMError.shouldNotReachHere("Can't register agent premain method, because the declaring class " + premainClass + " is not found", e);


This is a UserError as the user can make a mistake. We should also add a sentence on how to recover from that mistake to the error message.

vjovanov · 2024-06-12T06:43:39Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/PreMainSupport.java

+ * <ul>
+ * <li>Isolate code by checking current runtime. For example: <code>
+ * <pre>
+ * String vm = System.getProperty("java.vm.name");


We should guide the users to use the properties from ImageInfo. Then they can see if the agent is applied at build time or at runtime. We also need to make sure that this property is set before premain is called for the Native Image builder.

Fixed. This property can be obtained in premain.

vjovanov · 2024-06-12T06:45:25Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/PreMainSupport.java

+ * Keep premain options and return the rest args as main args.
+ * The premain options format:
+ * <br>
+ * -XX-premain:[full.qualified.premain.class]:[premain options]


I am not sure if this should be our final format for the flag. I think we can use it until we complete the feature and decide what is the user interface in the end.

Agreed. I added a //todo for future changing.

vjovanov · 2024-06-12T06:46:32Z

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/InstrumentFeature.java

+
+ public static class Options {
+ @Option(help = "Specify premain-class list. Multiple classes are separated by comma, and order matters.")//
+ public static final HostedOptionKey<LocatableMultiOptionValue.Strings> PremainClasses = new HostedOptionKey<>(null);


This is also an internal option. In the end we should probably pick the premain functions from the agent JARs.

sor-bae · 2024-06-14T04:41:18Z

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/InstrumentFeature.java

+ @Override
+ public void afterRegistration(AfterRegistrationAccess access) {
+ FeatureImpl.AfterRegistrationAccessImpl a = (FeatureImpl.AfterRegistrationAccessImpl) access;
+ cl = a.getImageClassLoader().getClassLoader();


I'm not sure this is still required, but I had to provide an agent JAR twice as -J-javaagent and -cp for NI build configurations in agent JARs to be picked up by the NI builder. So just checking, users can provide an agent JAR only once as -J-javaagent for their premain classes to be registered?

In this PR, we are not automatically picking agent from -cp or -J-javaagent. It will be handled in the following PRs. But I'm sure user doesn't need to provide agent jar multiple times.

vjovanov · 2024-06-17T15:40:07Z

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/InstrumentFeature.java

+ }
+ preMainSupport.registerPremainMethod(premainClass, premain, args.toArray(new Object[0]));
+ } catch (ClassNotFoundException e) {
+ UserError.abort(e,"Can't register agent premain method, because the given class %s is not found. Please check your -H:%s setting.", premainClass, Options.PremainClasses.getName());


It is better to use SubstrateOptionsParser.commandArgument to print the option name. This keeps the correct behavior when options become public or change the type.

vjovanov · 2024-06-17T16:42:52Z

To keep this feature working as expected we need to add a few tests into: com.oracle.svm.test

The tests can be executed from similarly to the tests for class initialization.

ziyilin · 2024-06-19T08:59:56Z

To keep this feature working as expected we need to add a few tests into: com.oracle.svm.test

The tests can be executed from similarly to the tests for class initialization.

I have added a test task agenttest. It now only checks the premain method, doesn't do any class transformation yet.

Java agent always has a premain method to initialize the agent. SVM needs the premain as well to support agent in native image. At compile time, -H:PremainClasses= option is used to set the premain classes. At runtime, premain runtime options are set along with main class' arguments in the format of -XX-premain:[class]:[options]

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label May 29, 2024

vjovanov reviewed Jun 12, 2024

View reviewed changes

sor-bae reviewed Jun 14, 2024

View reviewed changes

ziyilin force-pushed the staticInstrument-premain branch 2 times, most recently from 8e2bd2f to c9d5652 Compare June 14, 2024 09:39

vjovanov reviewed Jun 17, 2024

View reviewed changes

ziyilin force-pushed the staticInstrument-premain branch from c9d5652 to 538ad3f Compare June 19, 2024 08:52

ziyilin force-pushed the staticInstrument-premain branch 2 times, most recently from 5671706 to 510b031 Compare June 26, 2024 07:36

ziyilin force-pushed the staticInstrument-premain branch from 510b031 to bc9e8ad Compare June 26, 2024 08:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add premain for static agent support #8988

Add premain for static agent support #8988

ziyilin commented May 29, 2024

vjovanov Jun 12, 2024

vjovanov Jun 12, 2024

ziyilin Jun 14, 2024

ziyilin Jun 14, 2024

vjovanov Jun 17, 2024

ziyilin Jun 19, 2024

vjovanov Jun 12, 2024

ziyilin Jun 14, 2024

vjovanov Jun 12, 2024 •

edited

Loading

ziyilin Jun 14, 2024

vjovanov Jun 12, 2024

ziyilin Jun 14, 2024

vjovanov Jun 12, 2024

ziyilin Jun 14, 2024

vjovanov Jun 12, 2024

ziyilin Jun 14, 2024

vjovanov Jun 12, 2024

sor-bae Jun 14, 2024

ziyilin Jun 14, 2024

vjovanov Jun 17, 2024

vjovanov commented Jun 17, 2024 •

edited

Loading

ziyilin commented Jun 19, 2024


		private static final String PREMAIN_OPTION_PREFIX = "-XX-premain:";

		class PremainMethod {

Add premain for static agent support #8988

Are you sure you want to change the base?

Add premain for static agent support #8988

Conversation

ziyilin commented May 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjovanov Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjovanov commented Jun 17, 2024 • edited Loading

ziyilin commented Jun 19, 2024

vjovanov Jun 12, 2024 •

edited

Loading

vjovanov commented Jun 17, 2024 •

edited

Loading