Symbol visibility for gcj

Symbol visibility for gcj

[Andrew Haley]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 4218 bytes --]

This changes the symbol export rules for gcj-built DSOs.  We no longer
output symbols for:

private static methods
private static fields
private classes
vtables (except java.lang.Class)
class$$ fields

This reduces the number of symbols in libgcj.so from 87509 to 66209, a
saving of 20%.

I've attached the list of symbols that are no longer emitted.  

I don't believe it should cause any problems, but it is possible that
this will break some code that was using CNI to access private symbols
inside a DSO.  I don't think we care about that.

Andrew.



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

	* jvgenmain.c (main): Change main to use class$, not class$$.
	(do_mangle_classname): Likewise.
	* class.c (hide): New function.
	(add_field): Hide everything that shouldn't be visible outside a
	DSO.
	(build_static_class_ref): Likewise.
	(build_classdollar_field): Likewise.
	(make_class_data): Likewise.
	(layout_class_method): Likewise.
	
Index: class.c
===================================================================
--- class.c	(revision 123084)
+++ class.c	(working copy)
@@ -691,6 +691,13 @@
   return fntype;
 }
 
+static void
+hide (tree decl)
+{
+  DECL_VISIBILITY (decl) = VISIBILITY_HIDDEN;
+  DECL_VISIBILITY_SPECIFIED (decl) = 1;
+}
+
 tree
 add_method_1 (tree this_class, int access_flags, tree name, tree function_type)
 {
@@ -801,6 +808,10 @@
       /* Always make field externally visible.  This is required so
 	 that native methods can always access the field.  */
       TREE_PUBLIC (field) = 1;
+      /* Hide everything that shouldn't be visible outside a DSO.  */
+      if (flag_indirect_classes
+	  || (FIELD_PRIVATE (field)))
+	hide (field);
       /* Considered external unless we are compiling it into this
 	 object file.  */
       DECL_EXTERNAL (field) = (is_compiled_class (class) != 2);
@@ -958,7 +969,11 @@
       decl = build_decl (VAR_DECL, decl_name, class_type_node);
       TREE_STATIC (decl) = 1;
       if (! flag_indirect_classes)
-	TREE_PUBLIC (decl) = 1;
+	{
+	  TREE_PUBLIC (decl) = 1;
+	  if (CLASS_PRIVATE (TYPE_NAME (type)))
+	    hide (decl);
+	}
       DECL_IGNORED_P (decl) = 1;
       DECL_ARTIFICIAL (decl) = 1;
       if (is_compiled_class (type) == 1)
@@ -997,6 +1012,7 @@
       TREE_CONSTANT (decl) = 1;
       TREE_READONLY (decl) = 1;
       TREE_PUBLIC (decl) = 1;
+      hide (decl);
       DECL_IGNORED_P (decl) = 1;
       DECL_ARTIFICIAL (decl) = 1;
       MAYBE_CREATE_VAR_LANG_DECL_SPECIFIC (decl);
@@ -1684,6 +1700,10 @@
 
       TREE_PUBLIC (dtable_decl) = 1;
       DECL_INITIAL (dtable_decl) = dtable;
+      /* The only dispatch table exported from a DSO is the dispatch
+	 table for java.lang.Class.  */
+      if (DECL_NAME (type_decl) != id_class)
+	hide (dtable_decl);
       if (! flag_indirect_classes)
 	rest_of_decl_compilation (dtable_decl, 1, 0);
       /* Maybe we're compiling Class as the first class.  If so, set
@@ -2553,6 +2573,10 @@
 
   TREE_PUBLIC (method_decl) = 1;
 
+  if (flag_indirect_classes
+      || (METHOD_PRIVATE (method_decl) && METHOD_STATIC (method_decl)))
+    hide (method_decl);
+
   /* Considered external unless it is being compiled into this object
      file, or it was already flagged as external.  */
   if (!DECL_EXTERNAL (method_decl))
Index: jvgenmain.c
===================================================================
--- jvgenmain.c	(revision 123084)
+++ jvgenmain.c	(working copy)
@@ -143,8 +143,8 @@
     fprintf (stream, "   JvRunMainName (\"%s\", argc, argv);\n", classname);
   else
     {
-      fprintf (stream, "   extern void *%s;\n", mangled_classname);
-      fprintf (stream, "   JvRunMain (%s, argc, argv);\n", mangled_classname);
+      fprintf (stream, "   extern char %s;\n", mangled_classname);
+      fprintf (stream, "   JvRunMain (&%s, argc, argv);\n", mangled_classname);
     }
   fprintf (stream, "}\n");
   if (stream != stdout && fclose (stream) != 0)
@@ -176,7 +176,7 @@
 	count++;
     }
   append_gpp_mangled_name (&ptr [-count], count);
-  obstack_grow (mangle_obstack, "7class$$E", strlen ("7class$$E"));
+  obstack_grow (mangle_obstack, "6class$E", strlen ("6class$E"));
   obstack_1grow (mangle_obstack, '\0');
   return obstack_finish (mangle_obstack);
 }



[-- Attachment #2: nosyms.gz --]
[-- Type: application/octet-stream, Size: 102151 bytes --]

Re: Symbol visibility for gcj

[David Daney]

Andrew Haley wrote:
> This changes the symbol export rules for gcj-built DSOs.  We no longer
> output symbols for:
> 
> private static methods
> private static fields
> private classes
> vtables (except java.lang.Class)
> class$$ fields
> 
> This reduces the number of symbols in libgcj.so from 87509 to 66209, a
> saving of 20%.

Does that really matter?  I suppose it might reduce the number of pages 
that are faulted in when the dynamic linker is working because the 
interesting symbols would be spread across fewer pages.

How does your change affect the number of relocations in libgcj.so or 
the values returned by size?

David Daney

Re: Symbol visibility for gcj

[Andrew Haley]

David Daney writes:
 > Andrew Haley wrote:
 > > This changes the symbol export rules for gcj-built DSOs.  We no longer
 > > output symbols for:
 > > 
 > > private static methods
 > > private static fields
 > > private classes
 > > vtables (except java.lang.Class)
 > > class$$ fields
 > > 
 > > This reduces the number of symbols in libgcj.so from 87509 to 66209, a
 > > saving of 20%.
 > 
 > Does that really matter?  I suppose it might reduce the number of pages 
 > that are faulted in when the dynamic linker is working because the 
 > interesting symbols would be spread across fewer pages.

Right, and the size of the file, etc, etc.

 > How does your change affect the number of relocations in libgcj.so

There's a small reduction, less than 1%.

 > or the values returned by size?

Sorry?

Andrew.

Re: Symbol visibility for gcj

[Mark Wielaard]

On Fri, 2007-03-23 at 18:18 +0000, Andrew Haley wrote:
> I don't believe it should cause any problems, but it is possible that
> this will break some code that was using CNI to access private symbols
> inside a DSO.  I don't think we care about that.

For CNI we seem to treat package private members as visible. Is using
package private members from CNI still supported/will it be kept
supported, or should such usage be turned into java protected members?

Cheers,

Mark

Re: Symbol visibility for gcj

[David Daney]

Mark Wielaard wrote:
> On Fri, 2007-03-23 at 18:18 +0000, Andrew Haley wrote:
>> I don't believe it should cause any problems, but it is possible that
>> this will break some code that was using CNI to access private symbols
>> inside a DSO.  I don't think we care about that.
> 
> For CNI we seem to treat package private members as visible. Is using
> package private members from CNI still supported/will it be kept
> supported, or should such usage be turned into java protected members?
> 

There is no C++ equivalent to java's package private IIRC, so there 
seems to be no alternative to making them public in CNI.

Trying to access private members from CNI raises C++ protection errors, 
so it should be safe to make them invisible as you would get a compile 
time error if you tried to use them.

David Daney

Re: Symbol visibility for gcj

[Andrew Haley]

Mark Wielaard writes:
 > On Fri, 2007-03-23 at 18:18 +0000, Andrew Haley wrote:
 > > I don't believe it should cause any problems, but it is possible that
 > > this will break some code that was using CNI to access private symbols
 > > inside a DSO.  I don't think we care about that.
 > 
 > For CNI we seem to treat package private members as visible. Is using
 > package private members from CNI still supported/will it be kept
 > supported, or should such usage be turned into java protected members?

I haven't done anything about package private, so accessing package
private members from outside a DSO will still work after this patch.
Whether this *should* be supported, I don't know...

Andrew.

Re: Symbol visibility for gcj

[Andrew Haley]

Andrew Haley writes:
 > This changes the symbol export rules for gcj-built DSOs.  We no longer
 > output symbols for:
 > 
 > private static methods
 > private static fields
 > private classes
 > vtables (except java.lang.Class)
 > class$$ fields
 > 
 > This reduces the number of symbols in libgcj.so from 87509 to 66209, a
 > saving of 20%.
 > 
 > I've attached the list of symbols that are no longer emitted.  
 > 
 > I don't believe it should cause any problems, but it is possible that
 > this will break some code that was using CNI to access private symbols
 > inside a DSO.  I don't think we care about that.

Well, it did cause problems.  Here is a better, more subtle patch.

Andrew.


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

        * jvgenmain.c (main): Change main to use class$, not class$$.
        (do_mangle_classname): Likewise.
        * class.c (hide): New function.
        (add_field): Hide everything that shouldn't be visible outside a
        DSO.
        (build_static_class_ref): Likewise.
        (build_classdollar_field): Likewise.
        (make_class_data): Likewise.
        (layout_class_method): Likewise.
        * expr.c (special_method_p): New function.

Index: gcc/java/class.c
===================================================================
--- gcc/java/class.c	(revision 122746)
+++ gcc/java/class.c	(working copy)
@@ -689,6 +688,13 @@
   return fntype;
 }
 
+static void
+hide (tree decl)
+{
+  DECL_VISIBILITY (decl) = VISIBILITY_HIDDEN;
+  DECL_VISIBILITY_SPECIFIED (decl) = 1;
+}
+
 tree
 add_method_1 (tree this_class, int access_flags, tree name, tree function_type)
 {
@@ -799,6 +805,10 @@
       /* Always make field externally visible.  This is required so
 	 that native methods can always access the field.  */
       TREE_PUBLIC (field) = 1;
+      /* Hide everything that shouldn't be visible outside a DSO.  */
+      if (flag_indirect_classes
+	  || (FIELD_PRIVATE (field)))
+	hide (field);
       /* Considered external unless we are compiling it into this
 	 object file.  */
       DECL_EXTERNAL (field) = (is_compiled_class (class) != 2);
@@ -956,7 +966,11 @@
       decl = build_decl (VAR_DECL, decl_name, class_type_node);
       TREE_STATIC (decl) = 1;
       if (! flag_indirect_classes)
-	TREE_PUBLIC (decl) = 1;
+	{
+	  TREE_PUBLIC (decl) = 1;
+	  if (CLASS_PRIVATE (TYPE_NAME (type)))
+	    hide (decl);
+	}
       DECL_IGNORED_P (decl) = 1;
       DECL_ARTIFICIAL (decl) = 1;
       if (is_compiled_class (type) == 1)
@@ -995,6 +1009,7 @@
       TREE_CONSTANT (decl) = 1;
       TREE_READONLY (decl) = 1;
       TREE_PUBLIC (decl) = 1;
+      hide (decl);
       DECL_IGNORED_P (decl) = 1;
       DECL_ARTIFICIAL (decl) = 1;
       MAYBE_CREATE_VAR_LANG_DECL_SPECIFIC (decl);
@@ -1640,6 +1655,10 @@
 
       TREE_PUBLIC (dtable_decl) = 1;
       DECL_INITIAL (dtable_decl) = dtable;
+      /* The only dispatch table exported from a DSO is the dispatch
+	 table for java.lang.Class.  */
+      if (DECL_NAME (type_decl) != id_class)
+	hide (dtable_decl);
       if (! flag_indirect_classes)
 	rest_of_decl_compilation (dtable_decl, 1, 0);
       /* Maybe we're compiling Class as the first class.  If so, set
@@ -2509,6 +2528,12 @@
 
   TREE_PUBLIC (method_decl) = 1;
 
+  if (flag_indirect_classes
+      || (METHOD_PRIVATE (method_decl) && METHOD_STATIC (method_decl)
+	  && ! METHOD_NATIVE (method_decl)
+	  && ! special_method_p (method_decl)))
+    hide (method_decl);
+
   /* Considered external unless it is being compiled into this object
      file, or it was already flagged as external.  */
   if (!DECL_EXTERNAL (method_decl))
Index: gcc/java/jvgenmain.c
===================================================================
--- gcc/java/jvgenmain.c	(revision 122746)
+++ gcc/java/jvgenmain.c	(working copy)
@@ -143,8 +143,8 @@
     fprintf (stream, "   JvRunMainName (\"%s\", argc, argv);\n", classname);
   else
     {
-      fprintf (stream, "   extern void *%s;\n", mangled_classname);
-      fprintf (stream, "   JvRunMain (%s, argc, argv);\n", mangled_classname);
+      fprintf (stream, "   extern char %s;\n", mangled_classname);
+      fprintf (stream, "   JvRunMain (&%s, argc, argv);\n", mangled_classname);
     }
   fprintf (stream, "}\n");
   if (stream != stdout && fclose (stream) != 0)
@@ -176,7 +176,7 @@
 	count++;
     }
   append_gpp_mangled_name (&ptr [-count], count);
-  obstack_grow (mangle_obstack, "7class$$E", strlen ("7class$$E"));
+  obstack_grow (mangle_obstack, "6class$E", strlen ("6class$E"));
   obstack_1grow (mangle_obstack, '\0');
   return obstack_finish (mangle_obstack);
 }
Index: gcc/java/expr.c
===================================================================
--- gcc/java/expr.c	(revision 122746)
+++ gcc/java/expr.c	(working copy)
@@ -2121,6 +2121,25 @@
 
    {NULL, NULL, NULL, NULL, 0, NULL}};
 
+/* True if this method is special, i.e. it's a private method that
+   should be exported fro a DSO.  */
+
+bool
+special_method_p (tree candidate_method)
+{
+  tree context = DECL_NAME (TYPE_NAME (DECL_CONTEXT (candidate_method)));
+  tree method = DECL_NAME (candidate_method);
+  rewrite_rule *p;
+
+  for (p = rules; p->classname; p++)
+    {
+      if (get_identifier (p->classname) == context
+	  && get_identifier (p->method) == method)
+	return true;
+    }
+  return false;
+}
+
 /* Scan the rules list for replacements for *METHOD_P and replace the
    args accordingly.  If the rewrite results in an access to a private
    method, update SPECIAL.*/
Index: gcc/java/java-tree.h
===================================================================
--- gcc/java/java-tree.h	(revision 122746)
+++ gcc/java/java-tree.h	(working copy)
@@ -1171,6 +1171,7 @@
 extern void initialize_builtins (void);
 
 extern tree lookup_name (tree);
+extern bool special_method_p (tree);
 extern void maybe_rewrite_invocation (tree *, tree *, tree *, tree *);
 extern tree build_known_method_ref (tree, tree, tree, tree, tree, tree);
 extern tree build_class_init (tree, tree);

-- 
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903