8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs

Reviewed-by: mchung, dholmes, alanb

Author: Serguei Spitsyn

src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, 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
@@ -27,6 +27,7 @@
 
 import java.lang.instrument.UnmodifiableModuleException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.AccessibleObject;
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.ClassDefinition;
@@ -441,12 +442,6 @@ public Object run() {
         //
         // 1) declared with a signature of (String, Instrumentation)
         // 2) declared with a signature of (String)
-        // 3) inherited with a signature of (String, Instrumentation)
-        // 4) inherited with a signature of (String)
-        //
-        // So the declared version of either 1-arg or 2-arg always takes
-        // primary precedence over an inherited version. After that, the
-        // 2-arg version takes precedence over the 1-arg version.
         //
         // If no method is found then we throw the NoSuchMethodException
         // from the first attempt so that the exception text indicates
@@ -470,45 +465,25 @@ public Object run() {
             try {
                 m = javaAgentClass.getDeclaredMethod(methodname,
                                                  new Class<?>[] { String.class });
-            } catch (NoSuchMethodException x) {
-                // ignore this exception because we'll try
-                // two arg inheritance next
-            }
-        }
-
-        if (m == null) {
-            // now try the inherited 2-arg method
-            try {
-                m = javaAgentClass.getMethod( methodname,
-                                 new Class<?>[] {
-                                     String.class,
-                                     java.lang.instrument.Instrumentation.class
-                                 }
-                               );
-                twoArgAgent = true;
-            } catch (NoSuchMethodException x) {
-                // ignore this exception because we'll try
-                // one arg inheritance next
-            }
-        }
-
-        if (m == null) {
-            // finally try the inherited 1-arg method
-            try {
-                m = javaAgentClass.getMethod(methodname,
-                                             new Class<?>[] { String.class });
             } catch (NoSuchMethodException x) {
                 // none of the methods exists so we throw the
                 // first NoSuchMethodException as per 5.0
                 throw firstExc;
             }
         }
 
-        // the premain method should not be required to be public,
-        // make it accessible so we can call it
-        // Note: The spec says the following:
-        //     The agent class must implement a public static premain method...
-        setAccessible(m, true);
+        // reject non-public premain or agentmain method
+        if (!Modifier.isPublic(m.getModifiers())) {
+            String msg = "method " + classname + "." +  methodname + " must be declared public";
+            throw new IllegalAccessException(msg);
+        }
+
+        if (!Modifier.isPublic(javaAgentClass.getModifiers()) &&
+            !javaAgentClass.getModule().isNamed()) {
+            // If the java agent class is in an unnamed module, the java agent class can be non-public.
+            // Suppress access check upon the invocation of the premain/agentmain method.
+            setAccessible(m, true);
+        }
 
         // invoke the 1 or 2-arg method
         if (twoArgAgent) {

test/jdk/java/lang/instrument/PremainClass/NoPremainAgentTest.java test/jdk/java/lang/instrument/NegativeAgentRunner.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 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
@@ -21,38 +21,25 @@
  * questions.
  */
 
-import jdk.test.lib.process.OutputAnalyzer;
+package jdk.java.lang.instrument;
+
+import java.lang.RuntimeException;
 import jdk.test.lib.process.ProcessTools;
-import jdk.test.lib.Utils;
+import jdk.test.lib.process.OutputAnalyzer;
 
-/*
- * @test
- * @bug 6289149
- * @summary test when the agent's class is missing the premain() function.
- * @library /test/lib
- * @modules java.management
- *          java.instrument
- * @run build DummyMain
- * @run shell ../MakeJAR3.sh NoPremainAgent
- * @run main/othervm -XX:-CreateCoredumpOnCrash NoPremainAgentTest
- */
-public class NoPremainAgentTest {
-    // Use a javaagent without the premain() function.
-    // Verify that we get the correct exception.
-    public static void main(String[] a) throws Exception {
-        String testArgs = String.format(
-                "-javaagent:NoPremainAgent.jar -classpath %s DummyMain",
-                System.getProperty("test.classes", "."));
+public class NegativeAgentRunner {
 
+    public static void main(String argv[]) throws Exception {
+        if (argv.length != 2) {
+            throw new RuntimeException("Agent and exception class names are expected in arguments");
+        }
+        String agentClassName = argv[0];
+        String excepClassName = argv[1];
         ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
-                Utils.addTestJavaOpts(testArgs.split("\\s+")));
-        System.out.println("testjvm.cmd:" + Utils.getCommandLine(pb));
-
-        OutputAnalyzer output = ProcessTools.executeProcess(pb);
-        System.out.println("testjvm.stdout:" + output.getStdout());
-        System.out.println("testjvm.stderr:" + output.getStderr());
-
-        output.stderrShouldContain("java.lang.NoSuchMethodException");
+                "-javaagent:" + agentClassName + ".jar",
+                agentClassName);
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        output.shouldContain(excepClassName);
         if (0 == output.getExitValue()) {
             throw new RuntimeException("Expected error but got exit value 0");
         }

test/jdk/java/lang/instrument/PremainClass/InheritAgent0001.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2020, 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
@@ -23,17 +23,20 @@
 
 /**
  * @test
- * @bug 6289149
+ * @bug 6289149 8165276
  * @summary test config (0,0,0,1): declared 1-arg in agent class
  * @author Daniel D. Daugherty, Sun Microsystems
  *
- * @run shell ../MakeJAR3.sh InheritAgent0001
+ * @library /test/lib
+ * @build jdk.java.lang.instrument.PremainClass.InheritAgent0001
+ * @run driver jdk.test.lib.util.JavaAgentBuilder
+ *             InheritAgent0001 InheritAgent0001.jar
  * @run main/othervm -javaagent:InheritAgent0001.jar DummyMain
  */
 
 import java.lang.instrument.*;
 
-class InheritAgent0001 extends InheritAgent0001Super {
+public class InheritAgent0001 extends InheritAgent0001Super {
 
     //
     // This agent has a single argument premain() method which

test/jdk/java/lang/instrument/PremainClass/InheritAgent0010.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2020, 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
@@ -23,17 +23,20 @@
 
 /**
  * @test
- * @bug 6289149
+ * @bug 6289149 8165276
  * @summary test config (0,0,1,0): declared 2-arg in agent class
  * @author Daniel D. Daugherty, Sun Microsystems
  *
- * @run shell ../MakeJAR3.sh InheritAgent0010
+ * @library /test/lib
+ * @build jdk.java.lang.instrument.PremainClass.InheritAgent0010
+ * @run driver jdk.test.lib.util.JavaAgentBuilder
+ *             InheritAgent0010 InheritAgent0010.jar
  * @run main/othervm -javaagent:InheritAgent0010.jar DummyMain
  */
 
 import java.lang.instrument.*;
 
-class InheritAgent0010 extends InheritAgent0010Super {
+public class InheritAgent0010 extends InheritAgent0010Super {
 
     // This agent does NOT have a single argument premain() method.
 

test/jdk/java/lang/instrument/PremainClass/InheritAgent0011.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2020, 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
@@ -23,17 +23,20 @@
 
 /**
  * @test
- * @bug 6289149
+ * @bug 6289149 8165276
  * @summary test config (0,0,1,1): declared 2-arg and declared 1-arg in agent class
  * @author Daniel D. Daugherty, Sun Microsystems
  *
- * @run shell ../MakeJAR3.sh InheritAgent0011
+ * @library /test/lib
+ * @build jdk.java.lang.instrument.PremainClass.InheritAgent0011
+ * @run driver jdk.test.lib.util.JavaAgentBuilder
+ *             InheritAgent0011 InheritAgent0011.jar
  * @run main/othervm -javaagent:InheritAgent0011.jar DummyMain
  */
 
 import java.lang.instrument.*;
 
-class InheritAgent0011 extends InheritAgent0011Super {
+public class InheritAgent0011 extends InheritAgent0011Super {
 
     //
     // This agent has a single argument premain() method which

test/jdk/java/lang/instrument/PremainClass/InheritAgent0100.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 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
@@ -21,34 +21,32 @@
  * questions.
  */
 
-/**
+/*
  * @test
- * @bug 6289149
- * @summary test config (0,1,0,0): inherited 1-arg in agent class
- * @author Daniel D. Daugherty, Sun Microsystems
- *
- * @run shell ../MakeJAR3.sh InheritAgent0100
- * @run main/othervm -javaagent:InheritAgent0100.jar DummyMain
+ * @bug 6289149 8165276
+ * @summary test config (0,1,0,0): 1-arg premain method in superclass of agent class must be rejected
+ * @library /test/lib
+ * @library /test
+ * @modules java.instrument
+ * @build jdk.java.lang.instrument.PremainClass.InheritAgent0100
+ * @run driver jdk.test.lib.util.JavaAgentBuilder
+ *             InheritAgent0100 InheritAgent0100.jar
+ * @run main/othervm jdk.java.lang.instrument.NegativeAgentRunner InheritAgent0100 NoSuchMethodException
  */
 
-import java.lang.instrument.*;
+public class InheritAgent0100 extends InheritAgent0100Super {
 
-class InheritAgent0100 extends InheritAgent0100Super {
+     // This agent does NOT have a single argument premain() method.
+     // This agent does NOT have a double argument premain() method.
 
-    // This agent does NOT have a single argument premain() method.
-
-    // This agent does NOT have a double argument premain() method.
 }
 
 class InheritAgent0100Super {
-
-    //
-    // This agent has a single argument premain() method which
-    // is the one that should be called.
-    //
+    // This agent class has a single argument premain() method which should NOT be called.
     public static void premain (String agentArgs) {
         System.out.println("Hello from Single-Arg InheritAgent0100Super!");
+        throw new Error("ERROR: THIS AGENT SHOULD NOT HAVE BEEN CALLED.");
     }
 
-    // This agent does NOT have a double argument premain() method.
+    // This agent class does NOT have a double argument premain() method.
 }

test/jdk/java/lang/instrument/PremainClass/InheritAgent0101.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2020, 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
@@ -23,18 +23,21 @@
 
 /**
  * @test
- * @bug 6289149
- * @summary test config (0,1,0,1): inherited 1-arg and declared 1-arg in agent class
+ * @bug 6289149 8165276
+ * @summary test config (0,1,0,1): 1-arg in superclass and declared 1-arg in agent class
  * @author Daniel D. Daugherty, Sun Microsystems
  *
  * @key intermittent
- * @run shell ../MakeJAR3.sh InheritAgent0101
+ * @library /test/lib
+ * @build jdk.java.lang.instrument.PremainClass.InheritAgent0101
+ * @run driver jdk.test.lib.util.JavaAgentBuilder
+ *             InheritAgent0101 InheritAgent0101.jar
  * @run main/othervm -javaagent:InheritAgent0101.jar DummyMain
  */
 
 import java.lang.instrument.*;
 
-class InheritAgent0101 extends InheritAgent0101Super {
+public class InheritAgent0101 extends InheritAgent0101Super {
 
     //
     // This agent has a single argument premain() method which

test/jdk/java/lang/instrument/PremainClass/InheritAgent0110.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2020, 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
@@ -23,17 +23,20 @@
 
 /**
  * @test
- * @bug 6289149
- * @summary test config (0,1,1,0): inherited 1-arg and declared 2-arg in agent class
+ * @bug 6289149 8165276
+ * @summary test config (0,1,1,0): 1-arg in superclass and declared 2-arg in agent class
  * @author Daniel D. Daugherty, Sun Microsystems
  *
- * @run shell ../MakeJAR3.sh InheritAgent0110
+ * @library /test/lib
+ * @build jdk.java.lang.instrument.PremainClass.InheritAgent0110
+ * @run driver jdk.test.lib.util.JavaAgentBuilder
+ *             InheritAgent0110 InheritAgent0110.jar
  * @run main/othervm -javaagent:InheritAgent0110.jar DummyMain
  */
 
 import java.lang.instrument.*;
 
-class InheritAgent0110 extends InheritAgent0110Super {
+public class InheritAgent0110 extends InheritAgent0110Super {
 
     // This agent does NOT have a one argument premain() method.
 

test/jdk/java/lang/instrument/PremainClass/InheritAgent0111.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2020, 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
@@ -23,17 +23,20 @@
 
 /**
  * @test
- * @bug 6289149
- * @summary test config (0,1,1,1): inherited 1-arg, declared 2-arg and declared 1-arg in agent class
+ * @bug 6289149 8165276
+ * @summary test config (0,1,1,1): 1-arg in superclass, declared 2-arg and 1-arg in agent class
  * @author Daniel D. Daugherty, Sun Microsystems
  *
- * @run shell ../MakeJAR3.sh InheritAgent0111
+ * @library /test/lib
+ * @build jdk.java.lang.instrument.PremainClass.InheritAgent0111
+ * @run driver jdk.test.lib.util.JavaAgentBuilder
+ *             InheritAgent0111 InheritAgent0111.jar
  * @run main/othervm -javaagent:InheritAgent0111.jar DummyMain
  */
 
 import java.lang.instrument.*;
 
-class InheritAgent0111 extends InheritAgent0111Super {
+public class InheritAgent0111 extends InheritAgent0111Super {
 
     //
     // This agent has a single argument premain() method which