On 6/24/20 12:44, Mandy Chung wrote:


On 6/24/20 12:26 PM, serguei.spit...@oracle.com wrote:
On 6/24/20 05:25, David Holmes wrote:

Ah! The test class SimpleAgent is what is not public. That seems a bug in the test.

There are many such tests.
We can break some of the existing agents by rejecting non-public agent classes. I'm inclined to continue using the setAccessible and just add an extra check for non-public premain/agentmain methods.

There is only one non-public SimpleAgent which is shared by j.l.instrument tests.
  test/jdk/java/lang/instrument/SimpleAgent.java

There are many such tests:

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestLambdaFormRetransformation.java:class Agent implements ClassFileTransformer {

test/jdk/java/lang/instrument/NativeMethodPrefixAgent.java:class NativeMethodPrefixAgent { test/jdk/java/lang/instrument/PremainClass/NoPremainAgent.java:class NoPremainAgent {
test/jdk/java/lang/instrument/SimpleAgent.java:class SimpleAgent {
test/jdk/java/lang/instrument/RetransformAgent.java:class RetransformAgent {
test/jdk/java/lang/instrument/PremainClass/InheritAgent0001.java:class InheritAgent0001 extends InheritAgent0001Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0001.java:class InheritAgent0001Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0010.java:class InheritAgent0010 extends InheritAgent0010Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0010.java:class InheritAgent0010Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0011.java:class InheritAgent0011 extends InheritAgent0011Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0011.java:class InheritAgent0011Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0100.java:class InheritAgent0100 extends InheritAgent0100Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0100.java:class InheritAgent0100Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0101.java:class InheritAgent0101 extends InheritAgent0101Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0101.java:class InheritAgent0101Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0110.java:class InheritAgent0110 extends InheritAgent0110Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0110.java:class InheritAgent0110Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0111.java:class InheritAgent0111 extends InheritAgent0111Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent0111.java:class InheritAgent0111Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1000.java:class InheritAgent1000 extends InheritAgent1000Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1000.java:class InheritAgent1000Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1001.java:class InheritAgent1001 extends InheritAgent1001Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1001.java:class InheritAgent1001Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1010.java:class InheritAgent1010 extends InheritAgent1010Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1010.java:class InheritAgent1010Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1011.java:class InheritAgent1011 extends InheritAgent1011Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1011.java:class InheritAgent1011Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1100.java:class InheritAgent1100 extends InheritAgent1100Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1100.java:class InheritAgent1100Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1101.java:class InheritAgent1101 extends InheritAgent1101Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1101.java:class InheritAgent1101Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1110.java:class InheritAgent1110 extends InheritAgent1110Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1110.java:class InheritAgent1110Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1111.java:class InheritAgent1111 extends InheritAgent1111Super { test/jdk/java/lang/instrument/PremainClass/InheritAgent1111.java:class InheritAgent1111Super {


But is is not a big problem - all can be fixed.

test/hotspot/jtreg/runtime/cds/appcds/jvmti/dumpingWithAgent implements the agent properly (a public class and a public static void premain method).

As the popular Java agents are conforming the spec (publicly accessible premain method), the compatibility risk is low.

Unless such a  java agent exists and finds a strong compelling reason to argue that its premain method must be allowed non-public, I do not see the argument to change the spec to allow non-public agent classes.

A bad test case is not a representative existing java agent.

Okay, thanks.
I'll prepare a fix with a removed setAccessible.

Thanks,
Serguei


Mandy

Reply via email to