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: FYI: re-implement loading constraints


I'm checking this in on the trunk.

It turns out that I misunderstood loading constraints all these years.
It took a strange test case from Eclipse and a lot of prodding from
Andrew for me to figure out what is going on.

The test case is one where an interface is loaded via a user-defined
class loader.  This interface declares a method, and the return type
of that method is not visible to this class loader.

link.cc rejected this, because the types were not resolveable.

However, the real rule is that this is ok provided that the name (of
the return type in this case) is always uniquely resolved to the same
type.

This patch implements a good chunk of this.  It lets the Eclipse case
work.  There are still some cases we reject that the JDK does not,
because we link earlier.

And, now I think we do the wrong thing for BC type assertions -- I
suspect that some or most of those should be loading constraints
instead.  I'm not planning to implement that, though, unless some
important test case crops up.

Tom

ChangeLog:
2008-03-13  Tom Tromey  <tromey@redhat.com>

	* java/lang/natClassLoader.cc (_Jv_RegisterInitiatingLoader):
	Check loading constraints.
	(_Jv_CheckOrCreateLoadingConstraint): New function.
	* java/lang/ClassLoader.java (loadingConstraints): New field.
	* link.cc (_Jv_Linker::find_field): Use
	_Jv_CheckOrCreateLoadingConstraint.
	(_Jv_Linker::check_loading_constraints): New function.
	(_Jv_Linker::resolve_method_entry): Use
	check_loading_constraints.
	(_Jv_Linker::append_partial_itable): Likewise.
	(_Jv_Linker::layout_vtable_methods): Likewise.
	* include/jvm.h (_Jv_Linker::check_loading_constraints): Declare.
	(_Jv_CheckOrCreateLoadingConstraint): Declare.

Index: include/jvm.h
===================================================================
--- include/jvm.h	(revision 133170)
+++ include/jvm.h	(working copy)
@@ -1,6 +1,6 @@
 // jvm.h - Header file for private implementation information. -*- c++ -*-
 
-/* Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007  Free Software Foundation
+/* Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -265,6 +265,7 @@
 				      jclass, jclass *);
   static _Jv_Field *find_field(jclass, jclass, jclass *, _Jv_Utf8Const *,
 			       _Jv_Utf8Const *);
+  static void check_loading_constraints (_Jv_Method *, jclass, jclass);
   static void prepare_constant_time_tables(jclass);
   static jshort get_interfaces(jclass, _Jv_ifaces *);
   static void link_symbol_table(jclass);
@@ -557,6 +558,9 @@
 				jboolean is_jni_call = true,
 				jclass iface = NULL);
 
+extern void _Jv_CheckOrCreateLoadingConstraint (jclass,
+						java::lang::ClassLoader *);
+
 extern jobject _Jv_NewMultiArray (jclass, jint ndims, jint* dims)
   __attribute__((__malloc__));
 
Index: link.cc
===================================================================
--- link.cc	(revision 133170)
+++ link.cc	(working copy)
@@ -246,13 +246,9 @@
   if (_Jv_CheckAccess (klass, *found_class, the_field->flags))
     {
       // Note that the field returned by find_field_helper is always
-      // resolved.  There's no point checking class loaders here,
-      // since we already did the work to look up all the types.
-      // FIXME: being lazy here would be nice.
-      if (the_field->type != field_type)
-	throw new java::lang::LinkageError
-	  (JvNewStringLatin1 
-	   ("field type mismatch with different loaders"));
+      // resolved.  However, we still use the constraint mechanism
+      // because this may affect other lookups.
+      _Jv_CheckOrCreateLoadingConstraint (klass, (*found_class)->loader);
     }
   else
     {
@@ -269,6 +265,23 @@
   return the_field;
 }
 
+// Check loading constraints for method.
+void
+_Jv_Linker::check_loading_constraints (_Jv_Method *method, jclass self_class,
+				       jclass other_class)
+{
+  JArray<jclass> *klass_args;
+  jclass klass_return;
+
+  _Jv_GetTypesFromSignature (method, self_class, &klass_args, &klass_return);
+  jclass *klass_arg = elements (klass_args);
+  java::lang::ClassLoader *found_loader = other_class->loader;
+
+  _Jv_CheckOrCreateLoadingConstraint (klass_return, found_loader);
+  for (int i = 0; i < klass_args->length; i++)
+    _Jv_CheckOrCreateLoadingConstraint (*(klass_arg++), found_loader);
+}
+
 _Jv_Method *
 _Jv_Linker::resolve_method_entry (jclass klass, jclass &found_class,
 				  int class_index, int name_and_type_index,
@@ -359,39 +372,10 @@
       throw new java::lang::NoSuchMethodError (sb->toString());
     }
 
-  // if (found_class->loader != klass->loader), then we
-  // must actually check that the types of arguments
-  // correspond.  That is, for each argument type, and
-  // the return type, doing _Jv_FindClassFromSignature
-  // with either loader should produce the same result,
-  // i.e., exactly the same jclass object. JVMS 5.4.3.3
+  // if (found_class->loader != klass->loader), then we must actually
+  // check that the types of arguments correspond.  JVMS 5.4.3.3.
   if (found_class->loader != klass->loader)
-    {
-      JArray<jclass> *found_args, *klass_args;
-      jclass found_return, klass_return;
-
-      _Jv_GetTypesFromSignature (the_method,
-				 found_class,
-				 &found_args,
-				 &found_return);
-      _Jv_GetTypesFromSignature (the_method,
-				 klass,
-				 &klass_args,
-				 &klass_return);
-
-      jclass *found_arg = elements (found_args);
-      jclass *klass_arg = elements (klass_args);
-
-      for (int i = 0; i < found_args->length; i++)
-	{
-	  if (*(found_arg++) != *(klass_arg++))
-	    throw new java::lang::LinkageError (JvNewStringLatin1 
-	      ("argument type mismatch with different loaders"));
-	}
-      if (found_return != klass_return)
-	throw new java::lang::LinkageError (JvNewStringLatin1
-	  ("return type mismatch with different loaders"));
-    }
+    check_loading_constraints (the_method, klass, found_class);
   
   return the_method;
 }
@@ -925,7 +909,8 @@
 	continue;
 
       meth = NULL;
-      for (jclass cl = klass; cl; cl = cl->getSuperclass())
+      jclass cl;
+      for (cl = klass; cl; cl = cl->getSuperclass())
         {
 	  meth = _Jv_GetMethodLocal (cl, iface->methods[j].name,
 				     iface->methods[j].signature);
@@ -947,6 +932,9 @@
 	    itable[pos] = (void *) &_Jv_ThrowAbstractMethodError;
 	  else
 	    itable[pos] = meth->ncode;
+
+	  if (cl->loader != iface->loader)
+	    check_loading_constraints (meth, cl, iface);
 	}
       else
         {
@@ -1501,6 +1489,11 @@
 		  sb->append(_Jv_GetMethodString(declarer, super_meth));
 		  throw new VerifyError(sb->toString());
 		}
+	      else if (declarer->loader != klass->loader)
+		{
+		  // JVMS 5.4.2.
+		  check_loading_constraints (meth, klass, declarer);
+		}
 	    }
 	}
 
Index: java/lang/ClassLoader.h
===================================================================
--- java/lang/ClassLoader.h	(revision 133170)
+++ java/lang/ClassLoader.h	(working copy)
@@ -86,6 +86,7 @@
   void checkInitialized();
 public: // actually package-private
   ::java::util::HashMap * __attribute__((aligned(__alignof__( ::java::lang::Object)))) loadedClasses;
+  ::java::util::HashMap * loadingConstraints;
   ::java::util::HashMap * definedPackages;
 private:
   ::java::lang::ClassLoader * parent;
Index: java/lang/ClassLoader.java
===================================================================
--- java/lang/ClassLoader.java	(revision 133170)
+++ java/lang/ClassLoader.java	(working copy)
@@ -1,5 +1,5 @@
 /* ClassLoader.java -- responsible for loading classes into the VM
-   Copyright (C) 1998, 1999, 2001, 2002, 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+   Copyright (C) 1998, 1999, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -45,6 +45,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.ref.WeakReference;
 import java.net.URL;
 import java.nio.ByteBuffer;
 import java.security.CodeSource;
@@ -130,6 +131,15 @@
   final HashMap loadedClasses = new HashMap();
 
   /**
+   * Loading constraints registered with this classloader.  This maps
+   * a class name to a weak reference to a class.  When the reference
+   * is non-null, it means that a reference to the name must resolve
+   * to the indicated class.
+   */
+  final HashMap<String, WeakReference<Class>> loadingConstraints
+    = new HashMap<String, WeakReference<Class>>();
+
+  /**
    * All packages defined by this classloader. It is not private in order to
    * allow native code (and trusted subclasses) access to this field.
    */
Index: java/lang/natClassLoader.cc
===================================================================
--- java/lang/natClassLoader.cc	(revision 133170)
+++ java/lang/natClassLoader.cc	(working copy)
@@ -1,6 +1,6 @@
 // natClassLoader.cc - Implementation of java.lang.ClassLoader native methods.
 
-/* Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006  Free Software Foundation
+/* Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2008  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -41,6 +41,7 @@
 #include <java/lang/StringBuffer.h>
 #include <java/io/Serializable.h>
 #include <java/lang/Cloneable.h>
+#include <java/lang/ref/WeakReference.h>
 #include <java/util/HashMap.h>
 #include <gnu/gcj/runtime/BootClassLoader.h>
 #include <gnu/gcj/runtime/SystemClassLoader.h>
@@ -143,7 +144,21 @@
       // them later.
       return;
     }
-  loader->loadedClasses->put(klass->name->toString(), klass);
+
+  JvSynchronize sync (loader->loadingConstraints);
+
+  using namespace java::lang::ref;
+
+  jstring name = klass->getName();
+  WeakReference *ref = (WeakReference *) loader->loadingConstraints->get (name);
+  if (ref)
+    {
+      jclass constraint = (jclass) ref->get();
+      if (constraint && constraint != klass)
+	throw new java::lang::LinkageError(JvNewStringLatin1("loading constraint violated"));
+    }
+  loader->loadingConstraints->put(name, new WeakReference(klass));
+  loader->loadedClasses->put(name, klass);
 }
 
 // If we found an error while defining an interpreted class, we must
@@ -156,7 +171,47 @@
   loader->loadedClasses->remove(klass->name->toString());
 }
 
+// Check a loading constraint.  In particular check that, if there is
+// a constraint for the name of KLASS in LOADER, that it maps to
+// KLASS.  If there is no such constraint, make a new one.  If the
+// constraint is violated, throw an exception.  Do nothing for
+// primitive types.
+void
+_Jv_CheckOrCreateLoadingConstraint (jclass klass,
+				    java::lang::ClassLoader *loader)
+{
+  // Strip arrays.
+  while (klass->isArray())
+    klass = klass->getComponentType();
+  // Ignore primitive types.
+  if (klass->isPrimitive())
+    return;
 
+  if (! loader)
+    loader = java::lang::VMClassLoader::bootLoader;
+  jstring name = klass->getName();
+
+  JvSynchronize sync (loader->loadingConstraints);
+
+  using namespace java::lang::ref;
+
+  WeakReference *ref = (WeakReference *) loader->loadingConstraints->get (name);
+  if (ref)
+    {
+      jclass constraint = (jclass) ref->get();
+      if (constraint)
+	{
+	  if (klass != constraint)
+	    throw new java::lang::LinkageError(JvNewStringLatin1("loading constraint violated"));
+	  // Otherwise, all is ok.
+	  return;
+	}
+    }
+  // No constraint (or old constraint GC'd).  Make a new one.
+  loader->loadingConstraints->put(name, new WeakReference(klass));
+}
+
+
 // Class registration.
 //
 // There are two kinds of functions that register classes.  


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