This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Patch (please comment): class loader and interpreter fixes


This patch fixes a few different interpreter and class loader
problems.  The other patches I checked in over the last couple days
were originally part of this, but I tried to split it into more
manageable chunks.

* _Jv_IsInterpretedClass was implemented incorrectly.
  This patch adds a new ACC_INTERPRET flag which we use instead.
  This is a potentially bogus operation.  One idea I had was to put
  the new flag into Modifier (as package-private) so that if Sun ever
  adds new flags we'll be more likely to catch the problem.  Comments?

* The low-level class cache, which is used to do internal lookups for
  initiating loaders, would sometimes register things incorrectly.
  There is confusion in libgcj about whether the system class loader
  should be represented by NULL or by its actual value.  This patch
  clears that up, at least for the cache.

* Miranda methods weren't properly handled.  We now handle them by
  adding new method slots to an abstract class.  We mark these slots
  as ACC_SYNTHETIC so that they aren't visible to reflection.  (They
  are still visible to JNI, but does that matter?)  The same comments
  for ACC_INTERPRET apply to ACC_SYNTHETIC.

* This fixes a small memory leak in the internal class cache.

* This fixes the vtable layout code to correctly handle final
  classes -- a new method in a final class doesn't get a vtable slot.
  Also, we put a special function into "abstract" vtable slots, just
  in case.


After this patch, if I disable the GC and the verifier I'm able to get
Eclipse to start up.  The verifier bugs are known and they're on my
to-do list.  The GC problem is less known; it could be a bug in
libgcj, the GC, or perhaps some native part of Eclipse.


Comments are welcome.

For the Miranda methods fix, I took the approach I did because it
seemed much more efficient than the alternatives.  If someone can
think of a better way to do this, I'm happy to implement it.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	* include/jvm.h (ACC_SYNTHETIC): New define.
	* resolve.cc (_Jv_PrepareMissingMethods): New function.
	(_Jv_PrepareClass): Use it.
	* include/java-interp.h (ACC_INTERPRET): New define.
	(_Jv_IsInterpretedClass): Use it.
	(_Jv_InterpClass): _Jv_PrepareMissingMethods now friend.
	* java/lang/Class.h (Class::getModifiers): Mask with ALL_FLAGS.
	(Class): _Jv_PrepareMissingMethods now friend.
	* java/lang/natClassLoader.cc (defineClass0): Use JvSynchronize.
	Record `NULL' for system class loader.
	(_Jv_RegisterInitiatingLoader): Use JvSynchronize.  Special case
	system class loader.
	(_Jv_FindClassInCache): Likewise.
	(_Jv_UnregisterClass): Use JvSynchronize.  Free old loader info.
	(_Jv_FindClass): Special case system class loader.
	(defineClass0): Add ACC_INTERPRET to flags.
	* java/lang/natClass.cc (_Jv_abstractMethodError): New function.
	(_Jv_SetVTableEntries): Put _Jv_abstractMethodError into empty
	vtable slots.
	(_Jv_LayoutVTableMethods): Don't generate vtable slot for a method
	in a final class.
	(_getDeclaredMethod): Don't return synthetic methods.
	(getDeclaredMethods): Likewise.
	(_getMethod): Likewise.
	(_getMethods): Likewise.

Index: resolve.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/resolve.cc,v
retrieving revision 1.34
diff -u -r1.34 resolve.cc
--- resolve.cc 5 Dec 2002 07:43:44 -0000 1.34
+++ resolve.cc 11 Dec 2002 19:26:32 -0000
@@ -238,7 +238,7 @@
 
       // First search the class itself.
       the_method = _Jv_SearchMethodInClass (owner, klass, 
-	           method_name, method_signature);
+					    method_name, method_signature);
 
       if (the_method != 0)
         {
@@ -246,9 +246,10 @@
           goto end_of_method_search;
 	}
 
-      // If we are resolving an interface method, search the interface's 
-      // superinterfaces (A superinterface is not an interface's superclass - 
-      // a superinterface is implemented by the interface).
+      // If we are resolving an interface method, search the
+      // interface's superinterfaces (A superinterface is not an
+      // interface's superclass - a superinterface is implemented by
+      // the interface).
       if (pool->tags[index] == JV_CONSTANT_InterfaceMethodref)
         {
 	  _Jv_ifaces ifaces;
@@ -257,8 +258,8 @@
 	  ifaces.list = (jclass *) _Jv_Malloc (ifaces.len * sizeof (jclass *));
 
 	  _Jv_GetInterfaces (owner, &ifaces);	  
-          
-	  for (int i=0; i < ifaces.count; i++)
+
+	  for (int i = 0; i < ifaces.count; i++)
 	    {
 	      jclass cls = ifaces.list[i];
 	      the_method = _Jv_SearchMethodInClass (cls, klass, method_name, 
@@ -269,9 +270,9 @@
                   break;
 		}
 	    }
-	  
+
 	  _Jv_Free (ifaces.list);
-	  
+
 	  if (the_method != 0)
 	    goto end_of_method_search;
 	}
@@ -281,7 +282,7 @@
            cls = cls->getSuperclass ())
 	{
 	  the_method = _Jv_SearchMethodInClass (cls, klass, 
-	               method_name, method_signature);
+						method_name, method_signature);
           if (the_method != 0)
 	    {
 	      found_class = cls;
@@ -363,6 +364,57 @@
   return 0;
 }
 
+// A helper for _Jv_PrepareClass.  This adds missing `Miranda methods'
+// to a class.
+void
+_Jv_PrepareMissingMethods (jclass base2, jclass iface_class)
+{
+  _Jv_InterpClass *base = reinterpret_cast<_Jv_InterpClass *> (base2);
+  for (int i = 0; i < iface_class->interface_count; ++i)
+    {
+      for (int j = 0; j < iface_class->interfaces[i]->method_count; ++j)
+	{
+	  _Jv_Method *meth = &iface_class->interfaces[i]->methods[j];
+	  // Don't bother with <clinit>.
+	  if (meth->name->data[0] == '<')
+	    continue;
+	  _Jv_Method *new_meth = _Jv_LookupDeclaredMethod (base, meth->name,
+							   meth->signature);
+	  if (! new_meth)
+	    {
+	      // We assume that such methods are very unlikely, so we
+	      // just reallocate the method array each time one is
+	      // found.  This greatly simplifies the searching --
+	      // otherwise we have to make sure that each such method
+	      // found is really unique among all superinterfaces.
+	      int new_count = base->method_count + 1;
+	      _Jv_Method *new_m
+		= (_Jv_Method *) _Jv_AllocBytes (sizeof (_Jv_Method)
+						 * new_count);
+	      memcpy (new_m, base->methods,
+		      sizeof (_Jv_Method) * base->method_count);
+
+	      // Add new method.
+	      new_m[base->method_count] = *meth;
+	      new_m[base->method_count].index = (_Jv_ushort) -1;
+	      new_m[base->method_count].accflags |= ACC_SYNTHETIC;
+
+	      _Jv_MethodBase **new_im
+		= (_Jv_MethodBase **) _Jv_AllocBytes (sizeof (_Jv_MethodBase *)
+						      * new_count);
+	      memcpy (new_im, base->interpreted_methods,
+		      sizeof (_Jv_MethodBase *) * base->method_count);
+
+	      base->methods = new_m;
+	      base->interpreted_methods = new_im;
+	      base->method_count = new_count;
+	    }
+	}
+
+      _Jv_PrepareMissingMethods (base, iface_class->interfaces[i]);
+    }
+}
+
 void 
 _Jv_PrepareClass(jclass klass)
 {
@@ -516,12 +568,23 @@
 	}
     }
 
-  if (clz->accflags & Modifier::INTERFACE)
+  if ((clz->accflags & Modifier::INTERFACE))
     {
       clz->state = JV_STATE_PREPARED;
       clz->notifyAll ();
       return;
     }
+
+  // A class might have so-called "Miranda methods".  This is a method
+  // that is declared in an interface and not re-declared in an
+  // abstract class.  Some compilers don't emit declarations for such
+  // methods in the class; this will give us problems since we expect
+  // a declaration for any method requiring a vtable entry.  We handle
+  // this here by searching for such methods and constructing new
+  // internal declarations for them.  We only need to do this for
+  // abstract classes.
+  if ((clz->accflags & Modifier::ABSTRACT))
+    _Jv_PrepareMissingMethods (clz, clz);
 
   clz->vtable_method_count = -1;
   _Jv_MakeVTable (clz);
Index: include/java-interp.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/java-interp.h,v
retrieving revision 1.20
diff -u -r1.20 java-interp.h
--- include/java-interp.h 6 Dec 2002 23:41:38 -0000 1.20
+++ include/java-interp.h 11 Dec 2002 19:26:32 -0000
@@ -27,10 +27,15 @@
 #include <ffi.h>
 }
 
+// This is used to indicate an interpreted class.
+// It is based on the values in Modifier; if more flags are added
+// there then this will need to change.
+#define ACC_INTERPRET 0x1000
+
 extern inline jboolean
 _Jv_IsInterpretedClass (jclass c)
 {
-  return (c->loader != 0);
+  return (c->accflags & ACC_INTERPRET) != 0;
 }
 
 struct _Jv_ResolvedMethod;
@@ -158,6 +163,7 @@
   friend class _Jv_ClassReader;
   friend class _Jv_InterpMethod;
   friend void  _Jv_PrepareClass(jclass);
+  friend void  _Jv_PrepareMissingMethods (jclass base2, jclass iface_class);
   friend void  _Jv_InitField (jobject, jclass, int);
 #ifdef JV_MARKOBJ_DECL
   friend JV_MARKOBJ_DECL;
Index: include/jvm.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/jvm.h,v
retrieving revision 1.52
diff -u -r1.52 jvm.h
--- include/jvm.h 29 Aug 2002 17:53:28 -0000 1.52
+++ include/jvm.h 11 Dec 2002 19:26:32 -0000
@@ -30,6 +30,10 @@
 #include <gcj/cni.h>
 #include <gcj/field.h>
 
+// This access flag is set on a synthetic method.  This is a method we
+// declare internally but which not must be visible to reflection.
+#define ACC_SYNTHETIC  0x1000
+
 /* Structure of the virtual table.  */
 struct _Jv_VTable
 {
Index: java/lang/Class.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/Class.h,v
retrieving revision 1.52
diff -u -r1.52 Class.h
--- java/lang/Class.h 5 Dec 2002 07:43:45 -0000 1.52
+++ java/lang/Class.h 11 Dec 2002 19:26:33 -0000
@@ -182,9 +182,9 @@
   JArray<java::lang::reflect::Method *> *getMethods (void);
 
   inline jint getModifiers (void)
-    {
-      return accflags;
-    }
+  {
+    return accflags & java::lang::reflect::Modifier::ALL_FLAGS;
+  }
 
   jstring getName (void);
 
@@ -348,6 +348,7 @@
 					      _Jv_Utf8Const *method_signature);
 
   friend void _Jv_PrepareClass (jclass);
+  friend void _Jv_PrepareMissingMethods (jclass base, jclass iface_class);
 
   friend class _Jv_ClassReader;	
   friend class _Jv_InterpClass;
Index: java/lang/natClass.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natClass.cc,v
retrieving revision 1.57
diff -u -r1.57 natClass.cc
--- java/lang/natClass.cc 5 Dec 2002 07:43:45 -0000 1.57
+++ java/lang/natClass.cc 11 Dec 2002 19:26:35 -0000
@@ -343,7 +343,8 @@
   while (--i >= 0)
     {
       if (_Jv_equalUtf8Consts (methods[i].name, utf_name)
-	  && _Jv_equaln (methods[i].signature, partial_sig, p_len))
+	  && _Jv_equaln (methods[i].signature, partial_sig, p_len)
+	  && (methods[i].accflags & ACC_SYNTHETIC) == 0)
 	{
 	  // Found it.
 	  using namespace java::lang::reflect;
@@ -368,7 +369,8 @@
       if (method->name == NULL
 	  || _Jv_equalUtf8Consts (method->name, clinit_name)
 	  || _Jv_equalUtf8Consts (method->name, init_name)
-	  || _Jv_equalUtf8Consts (method->name, finit_name))
+	  || _Jv_equalUtf8Consts (method->name, finit_name)
+	  || (methods[i].accflags & ACC_SYNTHETIC) != 0)
 	continue;
       numMethods++;
     }
@@ -382,7 +384,8 @@
       if (method->name == NULL
 	  || _Jv_equalUtf8Consts (method->name, clinit_name)
 	  || _Jv_equalUtf8Consts (method->name, init_name)
-	  || _Jv_equalUtf8Consts (method->name, finit_name))
+	  || _Jv_equalUtf8Consts (method->name, finit_name)
+	  || (methods[i].accflags & ACC_SYNTHETIC) != 0)
 	continue;
       java::lang::reflect::Method* rmethod
 	= new java::lang::reflect::Method ();
@@ -514,7 +517,8 @@
 	{
 	  // FIXME: access checks.
 	  if (_Jv_equalUtf8Consts (klass->methods[i].name, utf_name)
-	      && _Jv_equaln (klass->methods[i].signature, partial_sig, p_len))
+	      && _Jv_equaln (klass->methods[i].signature, partial_sig, p_len)
+	      && (klass->methods[i].accflags & ACC_SYNTHETIC) == 0)
 	    {
 	      // Found it.
 	      using namespace java::lang::reflect;
@@ -565,7 +569,8 @@
       if (method->name == NULL
 	  || _Jv_equalUtf8Consts (method->name, clinit_name)
 	  || _Jv_equalUtf8Consts (method->name, init_name)
-	  || _Jv_equalUtf8Consts (method->name, finit_name))
+	  || _Jv_equalUtf8Consts (method->name, finit_name)
+	  || (method->accflags & ACC_SYNTHETIC) != 0)
 	continue;
       // Only want public methods.
       if (! java::lang::reflect::Modifier::isPublic (method->accflags))
@@ -1564,6 +1569,13 @@
           && meth->name->data[0] != '<');
 }
 
+// This is put in empty vtable slots.
+static void
+_Jv_abstractMethodError (void)
+{
+  throw new java::lang::AbstractMethodError();
+}
+
 // Prepare virtual method declarations in KLASS, and any superclasses as 
 // required, by determining their vtable index, setting method->index, and
 // finally setting the class's vtable_method_count. Must be called with the
@@ -1594,13 +1606,16 @@
 	continue;
 
       if (superclass != NULL)
-        super_meth = _Jv_LookupDeclaredMethod (superclass, meth->name, 
-					       meth->signature);
+	{
+	  super_meth = _Jv_LookupDeclaredMethod (superclass, meth->name, 
+						 meth->signature);
+	}
 
       if (super_meth)
         meth->index = super_meth->index;
-      else if (! (meth->accflags & java::lang::reflect::Modifier::FINAL))
-        meth->index = index++;
+      else if (! (meth->accflags & java::lang::reflect::Modifier::FINAL)
+	       && ! (klass->accflags & java::lang::reflect::Modifier::FINAL))
+	meth->index = index++;
     }
 
   klass->vtable_method_count = index;
@@ -1626,8 +1641,7 @@
 	continue;
       if ((meth->accflags & Modifier::ABSTRACT))
 	{
-	  // FIXME: we should set abstract slots to a function that
-	  // throws AbstractMethodError.  How can we do that on IA-64?
+	  vtable->set_method(meth->index, (void *) &_Jv_abstractMethodError);
 	  flags[meth->index] = false;
 	}
       else
Index: java/lang/natClassLoader.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natClassLoader.cc,v
retrieving revision 1.55
diff -u -r1.55 natClassLoader.cc
--- java/lang/natClassLoader.cc 11 Dec 2002 19:18:59 -0000 1.55
+++ java/lang/natClassLoader.cc 11 Dec 2002 19:26:35 -0000
@@ -56,17 +56,18 @@
 				  sizeof (_Jv_InterpClass));
   _Jv_InitNewClassFields (klass);
 
-  // synchronize on the class, so that it is not
-  // attempted initialized until we're done loading.
-  _Jv_MonitorEnter (klass);
+  // Synchronize on the class, so that it is not attempted initialized
+  // until we're done loading.
+  JvSynchronize sync (klass);
+
+  // Record the defining loader.  For the system class loader, we
+  // record NULL.
+  if (this != java::lang::ClassLoader::getSystemClassLoader())
+    klass->loader = this;
 
-  // record which is the defining loader
-  klass->loader = this;
-
-  // register that we are the initiating loader...
   if (name != 0)
     {
-      _Jv_Utf8Const *   name2 = _Jv_makeUtf8Const (name);
+      _Jv_Utf8Const *name2 = _Jv_makeUtf8Const (name);
 
       if (! _Jv_VerifyClassName (name2))
 	throw new java::lang::ClassFormatError
@@ -86,28 +87,20 @@
 
       _Jv_UnregisterClass (klass);
 
-      _Jv_MonitorExit (klass);
-
       // FIXME: Here we may want to test that EX does
       // indeed represent a valid exception.  That is,
       // anything but ClassNotFoundException, 
       // or some kind of Error.
 
-      // FIXME: Rewrite this as a cleanup instead of
-      // as a catch handler.
-
       throw ex;
     }
     
+  klass->accflags |= ACC_INTERPRET;
   klass->protectionDomain = pd;
 
   // if everything proceeded sucessfully, we're loaded.
   JvAssert (klass->state == JV_STATE_LOADED);
 
-  // if an exception is generated, this is initially missed.
-  // however, we come back here in handleException0 below...
-  _Jv_MonitorExit (klass);
-
   return klass;
 
 #else // INTERPRETER
@@ -293,10 +286,11 @@
 // Hash function for Utf8Consts.
 #define HASH_UTF(Utf) (((Utf)->hash) % HASH_LEN)
 
-struct _Jv_LoaderInfo {
-    _Jv_LoaderInfo          *next;
-    java::lang::Class       *klass;
-    java::lang::ClassLoader *loader;
+struct _Jv_LoaderInfo
+{
+  _Jv_LoaderInfo          *next;
+  java::lang::Class       *klass;
+  java::lang::ClassLoader *loader;
 };
 
 static _Jv_LoaderInfo *initiated_classes[HASH_LEN];
@@ -309,9 +303,12 @@
 jclass
 _Jv_FindClassInCache (_Jv_Utf8Const *name, java::lang::ClassLoader *loader)
 {
-  _Jv_MonitorEnter (&java::lang::Class::class$);
+  JvSynchronize sync (&java::lang::Class::class$);
   jint hash = HASH_UTF (name);
 
+  if (loader && loader == java::lang::ClassLoader::getSystemClassLoader())
+    loader = NULL;
+
   // first, if LOADER is a defining loader, then it is also initiating
   jclass klass;
   for (klass = loaded_classes[hash]; klass; klass = klass->next)
@@ -337,15 +334,13 @@
 	}
     }
 
-  _Jv_MonitorExit (&java::lang::Class::class$);
-
   return klass;
 }
 
 void
 _Jv_UnregisterClass (jclass the_class)
 {
-  _Jv_MonitorEnter (&java::lang::Class::class$);
+  JvSynchronize sync (&java::lang::Class::class$);
   jint hash = HASH_UTF(the_class->name);
 
   jclass *klass = &(loaded_classes[hash]);
@@ -363,29 +358,32 @@
     {
       while (*info && (*info)->klass == the_class)
 	{
+	  _Jv_LoaderInfo *old = *info;
 	  *info = (*info)->next;
+	  _Jv_Free (old);
 	}
 
       if (*info == NULL)
 	break;
     }
-
-  _Jv_MonitorExit (&java::lang::Class::class$);
 }
 
 void
 _Jv_RegisterInitiatingLoader (jclass klass, java::lang::ClassLoader *loader)
 {
-  // non-gc alloc!
-  _Jv_LoaderInfo *info = (_Jv_LoaderInfo *) _Jv_Malloc (sizeof(_Jv_LoaderInfo));
+  if (loader && loader == java::lang::ClassLoader::getSystemClassLoader())
+    loader = NULL;
+
+  // This information can't be visible to the GC.
+  _Jv_LoaderInfo *info
+    = (_Jv_LoaderInfo *) _Jv_Malloc (sizeof(_Jv_LoaderInfo));
   jint hash = HASH_UTF(klass->name);
 
-  _Jv_MonitorEnter (&java::lang::Class::class$);
+  JvSynchronize sync (&java::lang::Class::class$);
   info->loader = loader;
   info->klass  = klass;
   info->next   = initiated_classes[hash];
   initiated_classes[hash] = info;
-  _Jv_MonitorExit (&java::lang::Class::class$);
 }
 
 // This function is called many times during startup, before main() is
@@ -471,6 +469,9 @@
     {
       jstring sname = _Jv_NewStringUTF (name->data);
 
+      java::lang::ClassLoader *sys
+	= java::lang::ClassLoader::getSystemClassLoader ();
+
       if (loader)
 	{
 	  // Load using a user-defined loader, jvmspec 5.3.2
@@ -479,14 +480,14 @@
 	  // If "loader" delegated the loadClass operation to another
 	  // loader, explicitly register that it is also an initiating
 	  // loader of the given class.
-	  if (klass && (klass->getClassLoader () != loader))
+	  java::lang::ClassLoader *delegate = (loader == sys
+					       ? NULL
+					       : loader);
+	  if (klass && klass->getClassLoaderInternal () != delegate)
 	    _Jv_RegisterInitiatingLoader (klass, loader);
 	}
       else 
 	{
-	  java::lang::ClassLoader *sys
-	    = java::lang::ClassLoader::getSystemClassLoader ();
-
 	  // Load using the bootstrap loader jvmspec 5.3.1.
 	  klass = sys->loadClass (sname, false); 
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]