This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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]

PR libgcj/27352: SecurityManager.checkPermission() called unnecessarily


I noticed this when I was profiling some server code: we were calling
_Jv_StackTrace::GetCallingClass from getClassLoader() far too often.

This fixes the compiled case by rewriting 

   java.lang.Class.getClassLoader() -> java.lang.Class.getClassLoader(class$)

and adding a new private version of getClassLoader() thath is passed a
claller's class rather than unwinding the stack to find it.  I did the
same for

   java.lang.Class.forName("foo") -> java.lang.Class.forName("foo", class$)

This is OK because java.lang.Class is final and can't be replaced by
the user.

We can do this in a few other places too.

This only fixes the problem for compiled code.  Interpreted code has a
similar problem, but will need to be fixed in a different way.

Andrew.


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

	* expr.c (maybe_rewrite_invocation): New function.
	(rewrite_arglist_getclass): Likewise.
	(rules): New.
	(expand_invoke): Call maybe_rewrite_invocation.
	* parse.y (patch_invoke): Likewise.
	* java-tree.h: (maybe_rewrite_invocation): New function.

Index: gcc/java/parse.y
===================================================================
--- gcc/java/parse.y	(revision 113252)
+++ gcc/java/parse.y	(working copy)
@@ -11066,6 +11066,7 @@
 	case INVOKE_STATIC:
 	  {
 	    tree signature = build_java_signature (TREE_TYPE (method));
+	    maybe_rewrite_invocation (&method, &args, &signature);
 	    func = build_known_method_ref (method, TREE_TYPE (method),
 					   DECL_CONTEXT (method),
 					   signature, args);
Index: expr.c
===================================================================
--- expr.c	(revision 113252)
+++ expr.c	(working copy)
@@ -2020,6 +2020,86 @@
   return init;
 }
 
+
+
+/* Rewrite expensive calls that require stack unwinding at runtime to
+   cheaper alternatives.  The logic here performs thse
+   transformations:
+
+   java.lang.Class.forName("foo") -> java.lang.Class.forName("foo", class$)
+   java.lang.Class.getClassLoader() -> java.lang.Class.getClassLoader(class$)
+
+*/
+
+typedef struct
+{
+  const char *classname;
+  const char *method;
+  const char *signature;
+  const char *new_signature;
+  int flags;
+  tree (*rewrite_arglist) (tree arglist);
+} rewrite_rule;
+
+/* Add this.class to the end of an arglist.  */
+
+static tree 
+rewrite_arglist_getclass (tree arglist)
+{
+  return chainon (arglist, 
+		  tree_cons (NULL_TREE, build_class_ref (output_class), NULL_TREE));
+}
+
+static rewrite_rule rules[] =
+  {{"java.lang.Class", "getClassLoader", "()Ljava/lang/ClassLoader;", 
+    "(Ljava/lang/Class;)Ljava/lang/ClassLoader;", 
+    ACC_FINAL|ACC_PRIVATE, rewrite_arglist_getclass},
+   {"java.lang.Class", "forName", "(Ljava/lang/String;)Ljava/lang/Class;",
+    "(Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Class;",
+    ACC_FINAL|ACC_PRIVATE|ACC_STATIC, rewrite_arglist_getclass},
+   {NULL, NULL, NULL, NULL, 0, NULL}};
+
+/* Scan the rules list for replacements for *METHOD_P and replace the
+   args accordingly.  */
+
+void
+maybe_rewrite_invocation (tree *method_p, tree *arg_list_p, 
+			  tree *method_signature_p)
+{
+  tree context = DECL_NAME (TYPE_NAME (DECL_CONTEXT (*method_p)));
+  rewrite_rule *p;
+  for (p = rules; p->classname; p++)
+    {
+      if (get_identifier (p->classname) == context)
+	{
+	  tree method = DECL_NAME (*method_p);
+	  if (get_identifier (p->method) == method
+	      && get_identifier (p->signature) == *method_signature_p)
+	    {
+	      tree maybe_method
+		= lookup_java_method (DECL_CONTEXT (*method_p),
+				      method,
+				      get_identifier (p->new_signature));
+	      if (! maybe_method && ! flag_verify_invocations)
+		{
+		  maybe_method
+		    = add_method (DECL_CONTEXT (*method_p), p->flags, 
+				  method, get_identifier (p->new_signature));
+		  DECL_EXTERNAL (maybe_method) = 1;
+		}
+	      *method_p = maybe_method;
+	      gcc_assert (*method_p);
+	      *arg_list_p = p->rewrite_arglist (*arg_list_p);
+	      *method_signature_p = get_identifier (p->new_signature);
+
+	      break;
+	    }
+	}
+    }
+}
+
+
+
 tree
 build_known_method_ref (tree method, tree method_type ATTRIBUTE_UNUSED,
 			tree self_type, tree method_signature ATTRIBUTE_UNUSED,
@@ -2394,6 +2474,8 @@
   arg_list = pop_arguments (TYPE_ARG_TYPES (method_type));
   flush_quick_stack ();
 
+  maybe_rewrite_invocation (&method, &arg_list, &method_signature);
+
   func = NULL_TREE;
   if (opcode == OPCODE_invokestatic)
     func = build_known_method_ref (method, method_type, self_type,
Index: gcc/java/java-tree.h
===================================================================
--- gcc/java/java-tree.h	(revision 113252)
+++ gcc/java/java-tree.h	(working copy)
@@ -1241,6 +1241,7 @@
 extern void initialize_builtins (void);
 
 extern tree lookup_name (tree);
+extern void maybe_rewrite_invocation (tree *, tree *, tree *);
 extern tree build_known_method_ref (tree, tree, tree, tree, tree);
 extern tree build_class_init (tree, tree);
 extern int attach_init_test_initialization_flags (void **, void *);
2006-05-03  Andrew Haley  <aph@redhat.com>

	* java/lang/Class.java (getClassLoader(Class)): New.
	forName(String, Class): New.
	* java/lang/natClass.cc (getClassLoader(Class)): New.
	
Index: libjava/java/lang/natClass.cc
===================================================================
--- libjava/java/lang/natClass.cc	(revision 113252)
+++ libjava/java/lang/natClass.cc	(working copy)
@@ -115,9 +115,19 @@
   if (s != NULL)
     {
       jclass caller = _Jv_StackTrace::GetCallingClass (&Class::class$);
-      ClassLoader *caller_loader = NULL;
-      if (caller)
-	caller_loader = caller->getClassLoaderInternal();
+      return getClassLoader (caller);
+   }
+
+  return loader;
+}
+
+java::lang::ClassLoader *
+java::lang::Class::getClassLoader (jclass caller)
+{
+  java::lang::SecurityManager *s = java::lang::System::getSecurityManager();
+  if (s != NULL)
+    {
+      ClassLoader *caller_loader = caller->getClassLoaderInternal();
 
       // If the caller has a non-null class loader, and that loader
       // is not this class' loader or an ancestor thereof, then do a
Index: libjava/java/lang/Class.java
===================================================================
--- libjava/java/lang/Class.java	(revision 113252)
+++ libjava/java/lang/Class.java	(working copy)
@@ -111,6 +111,14 @@
   public static native Class forName (String className)
     throws ClassNotFoundException;
 
+  // A private internal method that is called by compiler-generated code.
+  private static Class forName (String className, Class caller)
+    throws ClassNotFoundException
+  {
+    return forName(className, true, caller.getClassLoader());
+  }
+
+
   /**
    * Use the specified classloader to load and link a class. If the loader
    * is null, this uses the bootstrap class loader (provide the security
@@ -185,6 +193,9 @@
    */
   public native ClassLoader getClassLoader ();
   
+  // A private internal method that is called by compiler-generated code.
+  private final native ClassLoader getClassLoader (Class caller);
+  
   /**
    * If this is an array, get the Class representing the type of array.
    * Examples: "[[Ljava.lang.String;" would return "[Ljava.lang.String;", and
Index: libjava/java/lang/Class.h
===================================================================
--- libjava/java/lang/Class.h	(revision 113252)
+++ libjava/java/lang/Class.h	(working copy)
@@ -289,6 +289,8 @@
   JArray<jclass> *getClasses (void);
 
   java::lang::ClassLoader *getClassLoader (void);
+private:
+  java::lang::ClassLoader *getClassLoader (jclass caller);
 public:
   // This is an internal method that circumvents the usual security
   // checks when getting the class loader.


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