Re: gcj trunk - getAnnotations NullPointerException / segmentation fault

[Andrew Haley <aph at redhat dot com>]

Hanno Meyer-Thurow writes:
 > Hi list,
 > with latest fixes to Annotation code and String.format(...) patch I get
 > a NullPointerException / segmentation fault by running JUnit-4.2 tests.
 > 
 > I do not think that I get a testcase this time. So, if you are interested get
 > JUnit-4.2 source from http://downloads.sourceforge.net/junit/junit4.2.zip
 > and run 'ant dist' to compile and run tests.
 > 
 > To run the tests alone do:
 > 
 > 	gij -classpath . org.junit.runner.JUnitCore org.junit.tests.AllTests
 > 
 > 1) initializationError0(org.junit.tests.AllTests)
 > java.lang.NullPointerException
 >    at $Proxy0.annotationType(Unknown Source:0)
 >    at java.lang.Class.getAnnotations(Class.java:1277)
 >    at java.lang.Class.getAnnotation(Class.java:1255)
 >    at org.junit.internal.requests.ClassRequest.getRunnerClass(ClassRequest.java:32)
 >    at org.junit.internal.requests.ClassRequest.getRunner(ClassRequest.java:20)
 >    at org.junit.internal.requests.ClassesRequest.getRunner(ClassesRequest.java:21)
 >    at org.junit.runners.Suite.<init>(Suite.java:50)
 >    at org.junit.runners.Suite.<init>(Suite.java:36)
 >    at java.lang.reflect.Constructor.newInstance(natConstructor.cc:92)
 >    at org.junit.internal.requests.ClassRequest.getRunner(ClassRequest.java:23)
 >    at org.junit.internal.requests.ClassesRequest.getRunner(ClassesRequest.java:21)
 >    at org.junit.runner.JUnitCore.run(JUnitCore.java:109)
 >    at org.junit.runner.JUnitCore.run(JUnitCore.java:100)
 >    at org.junit.runner.JUnitCore.runMain(JUnitCore.java:81)
 >    at org.junit.runner.JUnitCore.main(JUnitCore.java:44)

Agh.  This is my bug in Proxy, where I cache a Method in a reflection
structure that is not scanned by the GC, so the Method is reclaimed.
Then I try to use it.  :-(

OK, so I need to create the Method at the time the Proxy is invoked,
not cache it.  This turns out to be unexpectedly hard: creating
instances of Method() is difficult.

Andrew.


2007-03-05  Andrew Haley  <aph@redhat.com>

	* java/lang/reflect/natVMProxy.cc (ncode_closure.meth): Delete.
	(generateProxyClass): Don't pass method to ncode.
	(run_proxy): Call _Jv_GetReflectedMethod to find the proxy method.
	* java/lang/reflect/Method.h: Rebuild.
	* java/lang/reflect/Method.java (internalGetParameterTypes,
	internalGetExceptionTypes): New methods.
	* headers.txt (class java/lang/reflect/Method): Declare
	_Jv_GetReflectedMethod.  Be its friend.
	* java/lang/natClass.cc (_Jv_GetReflectedMethod): New method.
	* java/lang/Class.h: Declare it.  Be its friend.

Index: java/lang/natClass.cc
===================================================================
--- java/lang/natClass.cc	(revision 122545)
+++ java/lang/natClass.cc	(working copy)
@@ -1630,6 +1630,26 @@
   return NULL;
 }
 
+java::lang::reflect::Method *
+_Jv_GetReflectedMethod (jclass klass, _Jv_Utf8Const *name,
+		       _Jv_Utf8Const *signature)
+{
+  for (; klass; klass = klass->getSuperclass())
+    {
+      _Jv_Method *meth = _Jv_GetMethodLocal (klass, name, signature);
+      if (meth)
+	{
+	  using namespace java::lang::reflect;
+	  Method *rmethod = new Method ();
+	  rmethod->offset = (char*) meth - (char*) klass->methods;
+	  rmethod->declaringClass = klass;
+	  return rmethod;
+	}
+    }
+  
+  return NULL;
+}
+
 #ifdef HAVE_TLS
 
 // NOTE: MCACHE_SIZE should be a power of 2 minus one.
Index: java/lang/Class.h
===================================================================
--- java/lang/Class.h	(revision 122544)
+++ java/lang/Class.h	(working copy)
@@ -225,6 +225,9 @@
 
 _Jv_Method* _Jv_LookupDeclaredMethod (jclass, _Jv_Utf8Const *, 
 				      _Jv_Utf8Const*, jclass * = NULL);
+java::lang::reflect::Method *_Jv_GetReflectedMethod (jclass klass, 
+						    _Jv_Utf8Const *name,
+						    _Jv_Utf8Const *signature);
 jfieldID JvGetFirstInstanceField (jclass);
 jint JvNumInstanceFields (jclass);
 jfieldID JvGetFirstStaticField (jclass);
@@ -529,6 +532,9 @@
 
   friend _Jv_Method* ::_Jv_LookupDeclaredMethod (jclass, _Jv_Utf8Const *, 
 						 _Jv_Utf8Const*, jclass *);
+  friend java::lang::reflect::Method* ::_Jv_GetReflectedMethod (jclass klass, 
+						    _Jv_Utf8Const *name,
+						    _Jv_Utf8Const *signature);
   friend jfieldID (::JvGetFirstInstanceField) (jclass);
   friend jint (::JvNumInstanceFields) (jclass);
   friend jfieldID (::JvGetFirstStaticField) (jclass);
Index: headers.txt
===================================================================
--- headers.txt	(revision 122482)
+++ headers.txt	(working copy)
@@ -56,10 +56,12 @@
 class java/lang/reflect/Method
 prepend jmethodID _Jv_FromReflectedMethod (java::lang::reflect::Method *);
 prepend jobject _Jv_JNI_ToReflectedMethod (_Jv_JNIEnv *, jclass, jmethodID, jboolean);
+prepend ::java::lang::reflect::Method *_Jv_GetReflectedMethod (jclass, _Jv_Utf8Const*, _Jv_Utf8Const*);
 friend jmethodID (::_Jv_FromReflectedMethod) (java::lang::reflect::Method *);
 friend jobject (::_Jv_JNI_ToReflectedMethod) (_Jv_JNIEnv *, jclass, jmethodID, jboolean);
 friend class java::lang::Class;
 friend class java::io::ObjectInputStream;
+friend java::lang::reflect::Method* ::_Jv_GetReflectedMethod (jclass, _Jv_Utf8Const*, _Jv_Utf8Const*);
 
 class gnu/gcj/runtime/ExtensionClassLoader
 friend class ::java::lang::ClassLoader;
Index: java/lang/reflect/Method.java
===================================================================
--- java/lang/reflect/Method.java	(revision 122544)
+++ java/lang/reflect/Method.java	(working copy)
@@ -153,6 +153,15 @@
     return (Class<?>[]) parameter_types.clone();
   }
 
+  // Just like getParameterTypes, but don't clone the array.
+  // Package private for use by VMProxy.
+  final Class<?>[] internalGetParameterTypes ()
+  {
+    if (parameter_types == null)
+      getType();
+    return (Class<?>[]) parameter_types;
+  }
+
   /**
    * Get the exception types this method says it throws, in no particular
    * order. If the method has no throws clause, returns a 0-length array
@@ -167,6 +176,15 @@
     return (Class<?>[]) exception_types.clone();
   }
 
+  // Just like getExceptionTypes, but don't clone the array.
+  // Package private for use by VMProxy.
+  final Class<?>[] internalGetExceptionTypes ()
+  {
+    if (exception_types == null)
+      getType();
+    return (Class<?>[]) exception_types;
+  }
+
   /**
    * Compare two objects to see if they are semantically equivalent.
    * Two Methods are semantically equivalent if they have the same declaring
Index: java/lang/reflect/Method.h
===================================================================
--- java/lang/reflect/Method.h	(revision 122544)
+++ java/lang/reflect/Method.h	(working copy)
@@ -12,6 +12,7 @@
 
 jmethodID _Jv_FromReflectedMethod (java::lang::reflect::Method *);
 jobject _Jv_JNI_ToReflectedMethod (_Jv_JNIEnv *, jclass, jmethodID, jboolean);
+::java::lang::reflect::Method *_Jv_GetReflectedMethod (jclass, _Jv_Utf8Const*, _Jv_Utf8Const*);
 
 class java::lang::reflect::Method : public ::java::lang::reflect::AccessibleObject
 {
@@ -29,7 +30,13 @@
   jboolean isVarArgs();
   ::java::lang::Class * getReturnType();
   JArray< ::java::lang::Class * > * getParameterTypes();
+public: // actually package-private
+  JArray< ::java::lang::Class * > * internalGetParameterTypes();
+public:
   JArray< ::java::lang::Class * > * getExceptionTypes();
+public: // actually package-private
+  JArray< ::java::lang::Class * > * internalGetExceptionTypes();
+public:
   jboolean equals(::java::lang::Object *);
   jint hashCode();
   ::java::lang::String * toString();
@@ -71,6 +78,7 @@
   friend jobject (::_Jv_JNI_ToReflectedMethod) (_Jv_JNIEnv *, jclass, jmethodID, jboolean);
   friend class java::lang::Class;
   friend class java::io::ObjectInputStream;
+  friend java::lang::reflect::Method* ::_Jv_GetReflectedMethod (jclass, _Jv_Utf8Const*, _Jv_Utf8Const*);
 };
 
 #endif // __java_lang_reflect_Method__
Index: java/lang/reflect/natVMProxy.cc
===================================================================
--- java/lang/reflect/natVMProxy.cc	(revision 122544)
+++ java/lang/reflect/natVMProxy.cc	(working copy)
@@ -66,7 +66,7 @@
 using namespace java::lang;
 
 typedef void (*closure_fun) (ffi_cif*, void*, void**, void*);
-static void *ncode (_Jv_Method *self, closure_fun fun, Method *meth);
+static void *ncode (_Jv_Method *self, closure_fun fun);
 static void run_proxy (ffi_cif*, void*, void**, void*);
 
 typedef jobject invoke_t (jobject, Proxy *, Method *, JArray< jobject > *);
@@ -165,7 +165,7 @@
       // the interfaces of which it is a proxy will also be reachable,
       // so this is safe.
       method = imethod;
-      method.ncode = ncode (&method, run_proxy, elements(d->methods)[i]);
+      method.ncode = ncode (&method, run_proxy);
       method.accflags &= ~Modifier::ABSTRACT;
     }
 
@@ -283,7 +283,6 @@
     JvFail ("Bad ffi type in proxy");
 }
 
-
 // run_proxy is the entry point for all proxy methods.  It boxes up
 // all the arguments and then invokes the invocation handler's invoke()
 // method.  Exceptions are caught and propagated.
@@ -291,7 +290,6 @@
 typedef struct {
   ffi_closure  closure;
   ffi_cif   cif;
-  Method *meth;
   _Jv_Method *self;
   ffi_type *arg_types[0];
 } ncode_closure;
@@ -313,9 +311,15 @@
   Thread *thread = Thread::currentThread();
   _Jv_InterpFrame frame_desc (self->self, thread, proxy->getClass());
 
+  Method *meth = _Jv_GetReflectedMethod (proxy->getClass(), 
+					 self->self->name,
+					 self->self->signature);
+  JArray<jclass> *parameter_types = meth->internalGetParameterTypes ();
+  JArray<jclass> *exception_types = meth->internalGetExceptionTypes ();
+
   InvocationHandler *handler = proxy->h;
   void *poo 
-    = _Jv_NewObjectArray (self->meth->parameter_types->length, &Object::class$, NULL);
+    = _Jv_NewObjectArray (parameter_types->length, &Object::class$, NULL);
   JArray<jobject> *argsArray = (JArray<jobject> *) poo;
   jobject *jargs = elements(argsArray);
 
@@ -331,14 +335,14 @@
 
   // Copy and box all the args.
   int index = 1;
-  for (int i = 0; i < self->meth->parameter_types->length; i++, index++)
-    jargs[i] = box (args[index], elements(self->meth->parameter_types)[i],
+  for (int i = 0; i < parameter_types->length; i++, index++)
+    jargs[i] = box (args[index], elements(parameter_types)[i],
 		    cif->arg_types[index]->type);
   
   jobject ret;
   try
     {
-      ret = invoke (handler, proxy, self->meth, argsArray);
+      ret = invoke (handler, proxy, meth, argsArray);
     }
   catch (Throwable *t)
     {
@@ -346,15 +350,15 @@
 	  || _Jv_IsInstanceOf (t, &Error::class$))
 	throw t;
 
-      Class **throwables = elements (self->meth->exception_types);
-      for (int i = 0; i < self->meth->exception_types->length; i++)
+      Class **throwables = elements (exception_types);
+      for (int i = 0; i < exception_types->length; i++)
 	if (_Jv_IsInstanceOf (t, throwables[i]))
 	  throw t;
 
       throw new UndeclaredThrowableException (t);
     }
 
-  unbox (ret, self->meth->return_type, rvalue, cif->rtype->type);
+  unbox (ret, meth->return_type, rvalue, cif->rtype->type);
 }
 
 
@@ -362,7 +366,7 @@
 // the address of its closure.
 
 static void *
-ncode (_Jv_Method *self, closure_fun fun, Method *meth)
+ncode (_Jv_Method *self, closure_fun fun)
 {
   using namespace java::lang::reflect;
 
@@ -379,7 +383,6 @@
 		&closure->cif,
 		&closure->arg_types[0],
 		NULL);
-  closure->meth = meth;
   closure->self = self;
 
   JvAssert ((self->accflags & Modifier::NATIVE) == 0);