15 RFR(XS): 8165276: Spec states that invoke the premain method in an agent class if it's public but implementation differs

[serguei.spitsyn at oracle.com]

On 6/24/20 12:44, Mandy Chung wrote:
>
>
> On 6/24/20 12:26 PM, serguei.spitsyn at 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