Corrected behavior of Constructor.newInstance on abstract classes.

The VM now throws an exception when somebody tries to create an instance
of an abstract class through a Constructor object.  It also ensures that
the class is initialized before attempting to create an instance.

This change adds some Constructor tests to the 042-new-instance VM test.

diff --git a/tests/042-new-instance/expected.txt b/tests/042-new-instance/expected.txt
index 37a654f..53447db 100644 (file)
--- a/tests/042-new-instance/expected.txt
+++ b/tests/042-new-instance/expected.txt
@@ -1,4 +1,8 @@
 LocalClass succeeded
 Got expected PackageAccess complaint
-LocalClass2 succeeded
+LocalClass3 succeeded
 Got expected InstantationError
+Cons LocalClass failed as expected
+Cons LocalClass2 succeeded
+Cons got expected PackageAccess complaint
+Cons got expected InstantationException

diff --git a/tests/042-new-instance/info.txt b/tests/042-new-instance/info.txt
index 08127da..49c9e02 100644 (file)
--- a/tests/042-new-instance/info.txt
+++ b/tests/042-new-instance/info.txt
@@ -1,6 +1,2 @@
-This is a miscellaneous test that was imported into the new-at-the-time
-runtime test framework. The test is intended to exercise basic features,
-and as such cannot be build on top of junit, since failure of such basic
-features might disrupt junit.
-
-TODO: Real description goes here.
+Test various permutations of Class.newInstance and Constructor.newInstance,
+looking for correct handling of access rights and abstract classes.

diff --git a/tests/042-new-instance/src/Main.java b/tests/042-new-instance/src/Main.java
index 0cd2532..1f54fae 100644 (file)
--- a/tests/042-new-instance/src/Main.java
+++ b/tests/042-new-instance/src/Main.java
@@ -14,11 +14,21 @@
  * limitations under the License.
  */
 
+import java.lang.reflect.Constructor;
+
 /**
  * Test instance creation.
  */
 public class Main {
     public static void main(String[] args) {
+        testClassNewInstance();
+        testConstructorNewInstance();
+    }
+
+    /**
+     * Tests Class.newInstance().
+     */
+    static void testClassNewInstance() {
         // should succeed
         try {
             Class c = Class.forName("LocalClass");
@@ -41,7 +51,7 @@ public class Main {
             ex.printStackTrace();
         }
 
-        LocalClass2.main();
+        LocalClass3.main();
 
         try {
             MaybeAbstract ma = new MaybeAbstract();
@@ -52,20 +62,78 @@ public class Main {
             System.err.println("Got unexpected MaybeAbstract failure");
         }
     }
+
+    /**
+     * Tests Constructor.newInstance().
+     */
+    static void testConstructorNewInstance() {
+        // should fail -- getConstructor only returns public constructors
+        try {
+            Class c = Class.forName("LocalClass");
+            Constructor cons = c.getConstructor(new Class[0] /*(Class[])null*/);
+            System.err.println("Cons LocalClass succeeded unexpectedly");
+        } catch (NoSuchMethodException nsme) {
+            System.out.println("Cons LocalClass failed as expected");
+        } catch (Exception ex) {
+            System.err.println("Cons LocalClass failed strangely");
+            ex.printStackTrace();
+        }
+
+        // should succeed
+        try {
+            Class c = Class.forName("LocalClass2");
+            Constructor cons = c.getConstructor((Class[]) null);
+            Object obj = cons.newInstance();
+            System.out.println("Cons LocalClass2 succeeded");
+        } catch (Exception ex) {
+            System.err.println("Cons LocalClass2 failed");
+            ex.printStackTrace();
+        }
+
+        // should fail
+        try {
+            Class c = Class.forName("otherpackage.PackageAccess");
+            Constructor cons = c.getConstructor(new Class[0] /*(Class[])null*/);
+            System.err.println("ERROR: Cons PackageAccess succeeded unexpectedly");
+        } catch (NoSuchMethodException nsme) {
+            System.out.println("Cons got expected PackageAccess complaint");
+        } catch (Exception ex) {
+            System.err.println("Cons got unexpected PackageAccess failure");
+            ex.printStackTrace();
+        }
+
+        // should fail
+        try {
+            Class c = Class.forName("MaybeAbstract");
+            Constructor cons = c.getConstructor(new Class[0] /*(Class[])null*/);
+            Object obj = cons.newInstance();
+            System.err.println("ERROR: Cons MaybeAbstract succeeded unexpectedly");
+        } catch (InstantiationException ie) {
+            // note InstantiationException vs. InstantiationError
+            System.out.println("Cons got expected InstantationException");
+        } catch (Exception ex) {
+            System.err.println("Cons got unexpected MaybeAbstract failure");
+            ex.printStackTrace();
+        }
+    }
 }
 
 class LocalClass {
-  // this class has a default constructor with package visibility
+    // this class has a default constructor with package visibility
 }
 
-
 class LocalClass2 {
+    public LocalClass2() {}
+}
+
+
+class LocalClass3 {
     public static void main() {
         try {
             CC.newInstance();
-            System.out.println("LocalClass2 succeeded");
+            System.out.println("LocalClass3 succeeded");
         } catch (Exception ex) {
-            System.err.println("Got unexpected LocalClass2 failure");
+            System.err.println("Got unexpected LocalClass3 failure");
             ex.printStackTrace();
         }
     }

diff --git a/tests/042-new-instance/src/MaybeAbstract.java b/tests/042-new-instance/src/MaybeAbstract.java
index d87bd20..43c002b 100644 (file)
--- a/tests/042-new-instance/src/MaybeAbstract.java
+++ b/tests/042-new-instance/src/MaybeAbstract.java
@@ -15,6 +15,7 @@
  */
 
 public /*abstract*/ class MaybeAbstract {
+    public MaybeAbstract() {}
     int foo() { return 0; }
 }
 

diff --git a/tests/042-new-instance/src2/MaybeAbstract.java b/tests/042-new-instance/src2/MaybeAbstract.java
index 9ac3ceb..bfbfd45 100644 (file)
--- a/tests/042-new-instance/src2/MaybeAbstract.java
+++ b/tests/042-new-instance/src2/MaybeAbstract.java
@@ -15,6 +15,7 @@
  */
 
 public abstract class MaybeAbstract {
+    public MaybeAbstract() {}
     int foo() { return 0; }
 }
 

diff --git a/vm/native/java_lang_reflect_Constructor.c b/vm/native/java_lang_reflect_Constructor.c
index 82b72ee..6878f7b 100644 (file)
--- a/vm/native/java_lang_reflect_Constructor.c
+++ b/vm/native/java_lang_reflect_Constructor.c
@@ -39,6 +39,12 @@ static void Dalvik_java_lang_reflect_Constructor_getConstructorModifiers(
 /*
  * public int constructNative(Object[] args, Class declaringClass,
  *     Class[] parameterTypes, int slot, boolean noAccessCheck)
+ *
+ * We get here through Constructor.newInstance().  The Constructor object
+ * would not be available if the constructor weren't public (per the
+ * definition of Class.getConstructor), so we can skip the method access
+ * check.  We can also safely assume the constructor isn't associated
+ * with an interface, array, or primitive class.
  */
 static void Dalvik_java_lang_reflect_Constructor_constructNative(
     const u4* args, JValue* pResult)
@@ -52,6 +58,22 @@ static void Dalvik_java_lang_reflect_Constructor_constructNative(
     Object* newObj;
     Method* meth;
 
+    if (dvmIsAbstractClass(declaringClass)) {
+        dvmThrowExceptionWithClassMessage("Ljava/lang/InstantiationException;",
+            declaringClass->descriptor);
+        RETURN_VOID();
+    }
+
+    /* initialize the class if it hasn't been already */
+    if (!dvmIsClassInitialized(declaringClass)) {
+        if (!dvmInitClass(declaringClass)) {
+            LOGW("Class init failed in Constructor.constructNative (%s)\n",
+                declaringClass->descriptor);
+            assert(dvmCheckException(dvmThreadSelf()));
+            RETURN_VOID();
+        }
+    }
+
     newObj = dvmAllocObject(declaringClass, ALLOC_DEFAULT);
     if (newObj == NULL)
         RETURN_PTR(NULL);