Re: namespace namespace

Re: namespace namespace

[Andrew Haley]

It turns out that the mangling of C++ keywords used by gcj is completely
broken:

In particular, when a member name is a C++ keyword, "$" is appended to the
name.  However, this is results in an illegal mangled name so it is not
possible to refer to the member from CNI code.

Also, the set of C++ keywords in gjavah is different from the set in gcj.

Also, types are not checked for C++ keywords, so javax.xml.namespace.* cannot
be referenced from CNI code.

This patch fixes the problem.

This is an ABI change.  However, the ABI was almost completely broken
anyway.

Andrew.


2009-06-29  Andrew Haley  <aph@redhat.com>

	PR java/40590	
	* tools/gnu/classpath/tools/javah/FieldHelper.java (print):
	Use printName().
	* tools/gnu/classpath/tools/javah/MethodHelper.java (print):
	Use printName().
	* tools/gnu/classpath/tools/javah/CniStubPrinter.java (printDecl):
	Use printName().
	* tools/gnu/classpath/tools/javah/Keywords.java (words): Replace
	with keywords list from  gcc/java/mangle.c.
	* tools/gnu/classpath/tools/javah/ClassWrapper.java (printMethods):
	Don't pre-convert a C++ keyword.
	(print(CniPrintStream)): Call CniPrintStream.printName().
	(printContents): Likewise.
	* tools/gnu/classpath/tools/javah/CniPrintStream.java
	(getClassName): Don't call replaceAll("/", "::").
	(print(Type)): Add ""::" befor name, " *" after.  Use printName(), not
	print.
	(printName(PrintStream, String), printName(String), printlnName):
	New methods.
	(moveToPackage): Use printName().

2009-06-29  Andrew Haley  <aph@redhat.com>

	PR java/40590
	* java-tree.h (cxx_keyword_p): New declaration.
	* mangle_name.c (utf8_cmp): Move here from mangle.c.
	(cxx_keywords): Likewise.
	(cxx_keyword_p): Likewise.
	(MANGLE_CXX_KEYWORDS): New macro.
	(append_gpp_mangled_name): Use MANGLE_CXX_KEYWORDS.
	(append_gpp_mangled_name): Likewise.
	* mangle.c: Move code to mangle_name.c.
	(mangle_member_name): Don't call cxx_keyword_p.

Index: gcc/java/mangle_name.c
===================================================================
--- gcc/java/mangle_name.c	(revision 149053)
+++ gcc/java/mangle_name.c	(working copy)
@@ -41,6 +41,183 @@

 extern struct obstack *mangle_obstack;

+static int
+utf8_cmp (const unsigned char *str, int length, const char *name)
+{
+  const unsigned char *limit = str + length;
+  int i;
+
+  for (i = 0; name[i]; ++i)
+    {
+      int ch = UTF8_GET (str, limit);
+      if (ch != name[i])
+	return ch - name[i];
+    }
+
+  return str == limit ? 0 : 1;
+}
+
+/* A sorted list of all C++ keywords.  */
+static const char *const cxx_keywords[] =
+{
+  "_Complex",
+  "__alignof",
+  "__alignof__",
+  "__asm",
+  "__asm__",
+  "__attribute",
+  "__attribute__",
+  "__builtin_va_arg",
+  "__complex",
+  "__complex__",
+  "__const",
+  "__const__",
+  "__extension__",
+  "__imag",
+  "__imag__",
+  "__inline",
+  "__inline__",
+  "__label__",
+  "__null",
+  "__real",
+  "__real__",
+  "__restrict",
+  "__restrict__",
+  "__signed",
+  "__signed__",
+  "__typeof",
+  "__typeof__",
+  "__volatile",
+  "__volatile__",
+  "and",
+  "and_eq",
+  "asm",
+  "auto",
+  "bitand",
+  "bitor",
+  "bool",
+  "break",
+  "case",
+  "catch",
+  "char",
+  "class",
+  "compl",
+  "const",
+  "const_cast",
+  "continue",
+  "default",
+  "delete",
+  "do",
+  "double",
+  "dynamic_cast",
+  "else",
+  "enum",
+  "explicit",
+  "export",
+  "extern",
+  "false",
+  "float",
+  "for",
+  "friend",
+  "goto",
+  "if",
+  "inline",
+  "int",
+  "long",
+  "mutable",
+  "namespace",
+  "new",
+  "not",
+  "not_eq",
+  "operator",
+  "or",
+  "or_eq",
+  "private",
+  "protected",
+  "public",
+  "register",
+  "reinterpret_cast",
+  "return",
+  "short",
+  "signed",
+  "sizeof",
+  "static",
+  "static_cast",
+  "struct",
+  "switch",
+  "template",
+  "this",
+  "throw",
+  "true",
+  "try",
+  "typedef",
+  "typeid",
+  "typename",
+  "typeof",
+  "union",
+  "unsigned",
+  "using",
+  "virtual",
+  "void",
+  "volatile",
+  "wchar_t",
+  "while",
+  "xor",
+  "xor_eq"
+};
+
+/* Return true if NAME is a C++ keyword.  */
+int
+cxx_keyword_p (const char *name, int length)
+{
+  int last = ARRAY_SIZE (cxx_keywords);
+  int first = 0;
+  int mid = (last + first) / 2;
+  int old = -1;
+
+  for (mid = (last + first) / 2;
+       mid != old;
+       old = mid, mid = (last + first) / 2)
+    {
+      int kwl = strlen (cxx_keywords[mid]);
+      int min_length = kwl > length ? length : kwl;
+      int r = utf8_cmp ((const unsigned char *) name, min_length, cxx_keywords[mid]);
+
+      if (r == 0)
+	{
+	  int i;
+	  /* We've found a match if all the remaining characters are `$'.  */
+	  for (i = min_length; i < length && name[i] == '$'; ++i)
+	    ;
+	  if (i == length)
+	    return 1;
+	  r = 1;
+	}
+
+      if (r < 0)
+	last = mid;
+      else
+	first = mid;
+    }
+  return 0;
+}
+
+/* If NAME happens to be a C++ keyword, add `$'.  */
+#define MANGLE_CXX_KEYWORDS(NAME, LEN)			\
+do							\
+  {							\
+    if (cxx_keyword_p ((NAME), (LEN)))			\
+      {							\
+	char *tmp_buf = (char *)alloca ((LEN)+1);	\
+	memcpy (tmp_buf, (NAME), (LEN));		\
+	tmp_buf[LEN]= '$';				\
+	(NAME) = tmp_buf;				\
+	(LEN)++;					\
+      }							\
+  }							\
+while (0)
+
+
 /* If the assembler doesn't support UTF8 in symbol names, some
    characters might need to be escaped.  */

@@ -54,10 +231,14 @@
 void
 append_gpp_mangled_name (const char *name, int len)
 {
-  int encoded_len = unicode_mangling_length (name, len);
-  int needs_escapes = encoded_len > 0;
+  int encoded_len, needs_escapes;
   char buf[6];

+  MANGLE_CXX_KEYWORDS (name, len);
+
+  encoded_len = unicode_mangling_length (name, len);
+  needs_escapes = encoded_len > 0;
+
   sprintf (buf, "%d", (needs_escapes ? encoded_len : len));
   obstack_grow (mangle_obstack, buf, strlen (buf));

@@ -195,10 +376,14 @@
 append_gpp_mangled_name (const char *name, int len)
 {
   const unsigned char *ptr;
-  const unsigned char *limit = (const unsigned char *)name + len;
+  const unsigned char *limit;
   int encoded_len;
   char buf [6];

+  MANGLE_CXX_KEYWORDS (name, len);
+
+  limit = (const unsigned char *)name + len;
+
   /* Compute the length of the string we wish to mangle. */
   for (encoded_len =  0, ptr = (const unsigned char *) name;
        ptr < limit; encoded_len++)
Index: gcc/java/mangle.c
===================================================================
--- gcc/java/mangle.c	(revision 149053)
+++ gcc/java/mangle.c	(working copy)
@@ -72,167 +72,6 @@
 /* atms: array template mangled string. */
 static GTY(()) tree atms;

-static int
-utf8_cmp (const unsigned char *str, int length, const char *name)
-{
-  const unsigned char *limit = str + length;
-  int i;
-
-  for (i = 0; name[i]; ++i)
-    {
-      int ch = UTF8_GET (str, limit);
-      if (ch != name[i])
-	return ch - name[i];
-    }
-
-  return str == limit ? 0 : 1;
-}
-
-/* A sorted list of all C++ keywords.  */
-static const char *const cxx_keywords[] =
-{
-  "_Complex",
-  "__alignof",
-  "__alignof__",
-  "__asm",
-  "__asm__",
-  "__attribute",
-  "__attribute__",
-  "__builtin_va_arg",
-  "__complex",
-  "__complex__",
-  "__const",
-  "__const__",
-  "__extension__",
-  "__imag",
-  "__imag__",
-  "__inline",
-  "__inline__",
-  "__label__",
-  "__null",
-  "__real",
-  "__real__",
-  "__restrict",
-  "__restrict__",
-  "__signed",
-  "__signed__",
-  "__typeof",
-  "__typeof__",
-  "__volatile",
-  "__volatile__",
-  "and",
-  "and_eq",
-  "asm",
-  "auto",
-  "bitand",
-  "bitor",
-  "bool",
-  "break",
-  "case",
-  "catch",
-  "char",
-  "class",
-  "compl",
-  "const",
-  "const_cast",
-  "continue",
-  "default",
-  "delete",
-  "do",
-  "double",
-  "dynamic_cast",
-  "else",
-  "enum",
-  "explicit",
-  "export",
-  "extern",
-  "false",
-  "float",
-  "for",
-  "friend",
-  "goto",
-  "if",
-  "inline",
-  "int",
-  "long",
-  "mutable",
-  "namespace",
-  "new",
-  "not",
-  "not_eq",
-  "operator",
-  "or",
-  "or_eq",
-  "private",
-  "protected",
-  "public",
-  "register",
-  "reinterpret_cast",
-  "return",
-  "short",
-  "signed",
-  "sizeof",
-  "static",
-  "static_cast",
-  "struct",
-  "switch",
-  "template",
-  "this",
-  "throw",
-  "true",
-  "try",
-  "typedef",
-  "typeid",
-  "typename",
-  "typeof",
-  "union",
-  "unsigned",
-  "using",
-  "virtual",
-  "void",
-  "volatile",
-  "wchar_t",
-  "while",
-  "xor",
-  "xor_eq"
-};
-
-/* Return true if NAME is a C++ keyword.  */
-static int
-cxx_keyword_p (const char *name, int length)
-{
-  int last = ARRAY_SIZE (cxx_keywords);
-  int first = 0;
-  int mid = (last + first) / 2;
-  int old = -1;
-
-  for (mid = (last + first) / 2;
-       mid != old;
-       old = mid, mid = (last + first) / 2)
-    {
-      int kwl = strlen (cxx_keywords[mid]);
-      int min_length = kwl > length ? length : kwl;
-      int r = utf8_cmp ((const unsigned char *) name, min_length, cxx_keywords[mid]);
-
-      if (r == 0)
-	{
-	  int i;
-	  /* We've found a match if all the remaining characters are `$'.  */
-	  for (i = min_length; i < length && name[i] == '$'; ++i)
-	    ;
-	  if (i == length)
-	    return 1;
-	  r = 1;
-	}
-
-      if (r < 0)
-	last = mid;
-      else
-	first = mid;
-    }
-  return 0;
-}
-
 /* This is the mangling interface: a decl, a class field (.class) and
    the vtable. */

@@ -392,10 +231,6 @@
 {
   append_gpp_mangled_name (IDENTIFIER_POINTER (name),
 			   IDENTIFIER_LENGTH (name));
-
-  /* If NAME happens to be a C++ keyword, add `$'.  */
-  if (cxx_keyword_p (IDENTIFIER_POINTER (name), IDENTIFIER_LENGTH (name)))
-    obstack_1grow (mangle_obstack, '$');
 }

 /* Append the mangled name of TYPE onto OBSTACK.  */
Index: gcc/java/java-tree.h
===================================================================
--- gcc/java/java-tree.h	(revision 149053)
+++ gcc/java/java-tree.h	(working copy)
@@ -1224,6 +1224,8 @@

 extern void rewrite_reflection_indexes (void *);

+int cxx_keyword_p (const char *name, int length);
+
 #define DECL_FINAL(DECL) DECL_LANG_FLAG_3 (DECL)

 /* Access flags etc for a method (a FUNCTION_DECL): */
Index: libjava/classpath/tools/gnu/classpath/tools/javah/CniPrintStream.java
===================================================================
--- libjava/classpath/tools/gnu/classpath/tools/javah/CniPrintStream.java	(revision 148903)
+++ libjava/classpath/tools/gnu/classpath/tools/javah/CniPrintStream.java	(working copy)
@@ -125,9 +125,10 @@
     // Add the plain class name; we'll handle it when
     // we process namespaces.
     allClasses.add(name);
-    return "::" + name.replaceAll("/", "::") + " *";
+    return name;
   }

+  // Print the C++ form of TYPE, mangling C++ keywords.
   public void print(Type type)
   {
     int arrayCount = 0;
@@ -141,7 +142,9 @@
       }
     if (type.getSort() == Type.OBJECT)
       {
-        print(getClassName(type));
+	print("::");
+        printName(getClassName(type));
+	print(" *");
       }
     else
       {
@@ -156,6 +159,34 @@
       }
   }

+  // Print NAME, converting into C++ syntax and mangling C++ keywords
+  // as we go.
+  public final static void printName(PrintStream out, String name)
+  {
+    String[] parts = name.split("::|/");
+    for (int i = 0; i < parts.length; i++)
+      {
+	if (i != 0)
+	  out.print("::");
+	out.print(Keywords.getCxxName(parts[i]));
+      }
+  }
+
+  // Println NAME, converting into C++ syntax and mangling C++
+  // keywords as we go.
+  public final static void printlnName(PrintStream out, String name)
+  {
+    printName(out, name);
+    out.println();
+  }
+
+  // Print NAME, converting into C++ syntax and mangling C++ keywords
+  // as we go.
+  final void printName(String name)
+  {
+    printName(this, name);
+  }
+
   private void indent(PrintStream out, int n)
   {
     for (int i = 0; i < n; ++i)
@@ -186,7 +217,7 @@
       {
         indent(out, j + 1);
         out.print("namespace ");
-        out.println(pkgParts[j]);
+        printlnName(out, pkgParts[j]);
         indent(out, j + 1);
         out.println("{");
       }
@@ -202,7 +233,7 @@
     moveToPackage(out, pkgParts);
     indent(out, pkgParts.length + 2);
     out.print("class ");
-    out.print(className);
+    printName(out, className);
     out.println(";");
   }

Index: libjava/classpath/tools/gnu/classpath/tools/javah/ClassWrapper.java
===================================================================
--- libjava/classpath/tools/gnu/classpath/tools/javah/ClassWrapper.java	(revision 148903)
+++ libjava/classpath/tools/gnu/classpath/tools/javah/ClassWrapper.java	(working copy)
@@ -260,7 +260,7 @@
 	if (bridgeTargets.contains(sum))
 	  nameToUse = (String) methodNameMap.get(sum);
 	else
-	  nameToUse = Keywords.getCxxName(m.name);
+	  nameToUse = m.name;
 	methodNameMap.put(sum, nameToUse);
         MethodHelper.print(out, m, this, nameToUse);
       }
@@ -291,7 +291,8 @@

   public void print(CniPrintStream out)
   {
-    out.print("::" + name.replaceAll("/", "::"));
+    out.print("::");
+    out.printName(name);
   }

   // This prints the body of a class to a CxxPrintStream.
@@ -303,7 +304,7 @@

     out.print("class ");
     // Don't use our print() -- we don't want the leading "::".
-    out.print(name.replaceAll("/", "::"));
+    out.printName(name);
     if (superClass != null)
       {
         out.print(" : public ");
Index: libjava/classpath/tools/gnu/classpath/tools/javah/Keywords.java
===================================================================
--- libjava/classpath/tools/gnu/classpath/tools/javah/Keywords.java	(revision 148903)
+++ libjava/classpath/tools/gnu/classpath/tools/javah/Keywords.java	(working copy)
@@ -42,28 +42,115 @@

 public class Keywords
 {
-  private static final String[] words = { "and", "and_eq", "asm", "auto",
-                                         "bitand", "bitor", "bool", "break",
-                                         "case", "catch", "char", "class",
-                                         "compl", "const", "const_cast",
-                                         "continue", "default", "delete", "do",
-                                         "double", "dynamic_cast", "else",
-                                         "enum", "explicit", "export",
-                                         "extern", "false", "float", "for",
-                                         "friend", "goto", "if", "inline",
-                                         "int", "long", "mutable", "namespace",
-                                         "new", "not", "not_eq", "operator",
-                                         "or", "or_eq", "private", "protected",
-                                         "public", "register",
-                                         "reinterpret_cast", "return", "short",
-                                         "signed", "sizeof", "static",
-                                         "static_cast", "struct", "switch",
-                                         "template", "this", "throw", "true",
-                                         "try", "typedef", "typeid",
-                                         "typename", "typeof", "union",
-                                         "unsigned", "using", "virtual",
-                                         "void", "volatile", "wchar_t",
-                                         "while", "xor", "xor_eq" };
+/* A sorted list of all C++ keywords.  This is identical to the list
+   in gcc/java/mangle.c.  */
+  private static final String[] words =
+    {
+      "_Complex",
+      "__alignof",
+      "__alignof__",
+      "__asm",
+      "__asm__",
+      "__attribute",
+      "__attribute__",
+      "__builtin_va_arg",
+      "__complex",
+      "__complex__",
+      "__const",
+      "__const__",
+      "__extension__",
+      "__imag",
+      "__imag__",
+      "__inline",
+      "__inline__",
+      "__label__",
+      "__null",
+      "__real",
+      "__real__",
+      "__restrict",
+      "__restrict__",
+      "__signed",
+      "__signed__",
+      "__typeof",
+      "__typeof__",
+      "__volatile",
+      "__volatile__",
+      "and",
+      "and_eq",
+      "asm",
+      "auto",
+      "bitand",
+      "bitor",
+      "bool",
+      "break",
+      "case",
+      "catch",
+      "char",
+      "class",
+      "compl",
+      "const",
+      "const_cast",
+      "continue",
+      "default",
+      "delete",
+      "do",
+      "double",
+      "dynamic_cast",
+      "else",
+      "enum",
+      "explicit",
+      "export",
+      "extern",
+      "false",
+      "float",
+      "for",
+      "friend",
+      "goto",
+      "if",
+      "inline",
+      "int",
+      "long",
+      "mutable",
+      "namespace",
+      "new",
+      "not",
+      "not_eq",
+      "operator",
+      "or",
+      "or_eq",
+      "private",
+      "protected",
+      "public",
+      "register",
+      "reinterpret_cast",
+      "return",
+      "short",
+      "signed",
+      "sizeof",
+      "static",
+      "static_cast",
+      "struct",
+      "switch",
+      "template",
+      "this",
+      "throw",
+      "true",
+      "try",
+      "typedef",
+      "typeid",
+      "typename",
+      "typeof",
+      "union",
+      "unsigned",
+      "using",
+      "virtual",
+      "void",
+      "volatile",
+      "wchar_t",
+      "while",
+      "xor",
+      "xor_eq"
+    };

   private static final HashSet keywords;
   static
Index: libjava/classpath/tools/gnu/classpath/tools/javah/CniStubPrinter.java
===================================================================
--- libjava/classpath/tools/gnu/classpath/tools/javah/CniStubPrinter.java	(revision 148903)
+++ libjava/classpath/tools/gnu/classpath/tools/javah/CniStubPrinter.java	(working copy)
@@ -59,9 +59,9 @@

   private void printDecl(CniPrintStream out, String className, MethodNode method)
   {
-    out.print(className);
+    out.printName(className);
     out.print("::");
-    out.print(method.name);
+    out.printName(method.name);
     out.print("(");
     Type[] argTypes = Type.getArgumentTypes(method.desc);
     for (int j = 0; j < argTypes.length; ++j)
Index: libjava/classpath/tools/gnu/classpath/tools/javah/MethodHelper.java
===================================================================
--- libjava/classpath/tools/gnu/classpath/tools/javah/MethodHelper.java	(revision 148903)
+++ libjava/classpath/tools/gnu/classpath/tools/javah/MethodHelper.java	(working copy)
@@ -97,14 +97,14 @@
       {
         out.print(Type.getReturnType(meth.desc));
         out.print(" ");
-	out.print(realMethodName);
+	out.printName(realMethodName);
       }
     else
       {
         String name = declarer.name;
         int index = name.lastIndexOf('/');
         name = name.substring(index + 1);
-        out.print(name);
+        out.printName(name);
       }
     out.print("(");
     Type[] argTypes = Type.getArgumentTypes(meth.desc);
Index: libjava/classpath/tools/gnu/classpath/tools/javah/FieldHelper.java
===================================================================
--- libjava/classpath/tools/gnu/classpath/tools/javah/FieldHelper.java	(revision 148903)
+++ libjava/classpath/tools/gnu/classpath/tools/javah/FieldHelper.java	(working copy)
@@ -66,7 +66,7 @@
         out.print(")))) ");
         result = true;
       }
-    out.print(Keywords.getCxxName(field.name));
+    out.printName(field.name);
     if (hasMethodName)
       out.print("__");
     if (Modifier.isStatic(field.access))

Re: namespace namespace

[Joseph S. Myers]

On Mon, 29 Jun 2009, Andrew Haley wrote:

> Also, the set of C++ keywords in gjavah is different from the set in gcj.

Sounds like you should put a comment where the G++ list of keywords is, 
saying that if you change the list of keywords for C++ you need to update 
<list of other places in gcj, gjavah etc.> to keep those other lists in 
sync - logically any addition of a C++ keyword should start with adding it 
for the C++ front end.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: namespace namespace

[Andrew Haley]

Joseph S. Myers wrote:
> On Mon, 29 Jun 2009, Andrew Haley wrote:
> 
>> Also, the set of C++ keywords in gjavah is different from the set in gcj.
> 
> Sounds like you should put a comment where the G++ list of keywords is, 
> saying that if you change the list of keywords for C++ you need to update 
> <list of other places in gcj, gjavah etc.> to keep those other lists in 
> sync - logically any addition of a C++ keyword should start with adding it 
> for the C++ front end.

Will do.

Andrew.

Re: namespace namespace

[Dave Korn]

Andrew Haley wrote:

> This is an ABI change.  However, the ABI was almost completely broken
> anyway.

  Tangentially-related: Would it ever be possible to change the use of periods
as separators in java mangled names too, or is that too deeply entrenched or
part of a standard?

    cheers,
      DaveK

Re: namespace namespace

[Andrew Haley]

Dave Korn wrote:
> Andrew Haley wrote:
> 
>> This is an ABI change.  However, the ABI was almost completely broken
>> anyway.
> 
>   Tangentially-related: Would it ever be possible to change the use of periods
> as separators in java mangled names too, or is that too deeply entrenched or
> part of a standard?

I suppose it could be done.  Is there some big problem with periods that
I don't know about?

For example, java.lang.Object.class is mangled as _ZN4java4lang6Object6class$E.
When do you see a problem with the use of periods as separators?

Andrew.

Re: namespace namespace

[Dave Korn]

Andrew Haley wrote:
> Dave Korn wrote:
>> Andrew Haley wrote:
>>
>>> This is an ABI change.  However, the ABI was almost completely broken
>>> anyway.
>>   Tangentially-related: Would it ever be possible to change the use of periods
>> as separators in java mangled names too, or is that too deeply entrenched or
>> part of a standard?
> 
> I suppose it could be done.  Is there some big problem with periods that
> I don't know about?
> 
> For example, java.lang.Object.class is mangled as _ZN4java4lang6Object6class$E.

  The kind of symbols I'm talking about are those with real actual periods in
them, things I've seen in libjava, such as these:

gnu/.libs/classpath.o:

00000030 r _java.io.IOException_ref.1567
00000034 r _java.io.IOException_ref.3036
0000002c r _java.lang.Exception_ref.1841

or these (from unidentified .o files):

000139c0 t
__ZN3gnu5javax3net3ssl8provider11CipherSuiteC1EPNS3_15CipherAlgorithmEPNS3_20KeyExchangeAlgorithmEPNS3_18SignatureAlgorithmEPNS3_12MacAlgorithmEiiiPN4java4lang6StringE.clone.1
0002a9b0 t
__ZN3gnu5javax3net3ssl8provider4Util11toHexStringEJPN4java4lang6StringEP6JArrayIcEw.clone.0
000139c0 t
__ZN3gnu5javax3net3ssl8provider11CipherSuiteC1EPNS3_15CipherAlgorithmEPNS3_20KeyExchangeAlgorithmEPNS3_18SignatureAlgorithmEPNS3_12MacAlgorithmEiiiPN4java4lang6StringE.clone.1
0002a9b0 t
__ZN3gnu5javax3net3ssl8provider4Util11toHexStringEJPN4java4lang6StringEP6JArrayIcEw.clone.0
00000d10 t __ZN3gnu5javax5swing4text4html6parser7support3low4nodeC1Eib.clone.0
00000f72 r __Utf106.2066
00000fe0 r __Utf117.2121
000002f4 r __Utf17.1621
00003b54 r __Utf1042.35506
0000c696 r __Utf1757.124553
000001d4 r _CSWTCH.18597
000001d4 r _CSWTCH.18597
00000000 d ___emutls_v._ZL12method_cache
00000020 t __Z17JvNewStringLatin1PKc.clone.0


> When do you see a problem with the use of periods as separators?

  Actually, there may not be a problem at all.  There is a /potential/ problem
relating to the export of symbols from DLLS, specified using DEF files.  The
syntax of DEF files uses periods as a separator in entries that forward an
export to another DLL, so having periods in the symbol names would cause
ambiguity in the grammar.

  Like I said I've seen these mysterious symbols with literal periods in them,
but I don't know what they actually are.  On closer inspection while writing
this mail, I suddenly notice that they all appear to be local symbols; if this
is always the case, then there's no problem at all because we'll never want to
export them from a DLL.  And noticing the '.clone.' in there makes me wonder
if this is in fact an entirely C++ issue and not Java at all?  (In which case
sorry for bothering you with something that's not your department!)

    cheers,
      DaveK

Re: namespace namespace

[Dave Korn]

Dave Korn wrote:
>  And noticing the '.clone.' in there makes me wonder
> if this is in fact an entirely C++ issue and not Java at all?  (In which case
> sorry for bothering you with something that's not your department!)

  Sorry for bothering you with something that's not your department.  I guess
windows platforms should probably define NO_DOT_IN_LABEL.

    cheers,
      DaveK

Re: namespace namespace

[Andrew Haley]

Dave Korn wrote:
> Andrew Haley wrote:
>> Dave Korn wrote:
>>> Andrew Haley wrote:
>>>
>>>> This is an ABI change.  However, the ABI was almost completely broken
>>>> anyway.
>>>   Tangentially-related: Would it ever be possible to change the use of periods
>>> as separators in java mangled names too, or is that too deeply entrenched or
>>> part of a standard?
>> I suppose it could be done.  Is there some big problem with periods that
>> I don't know about?
>>
>> For example, java.lang.Object.class is mangled as _ZN4java4lang6Object6class$E.
> 
>   The kind of symbols I'm talking about are those with real actual periods in
> them, things I've seen in libjava, such as these:
> 
> gnu/.libs/classpath.o:
> 
> 00000030 r _java.io.IOException_ref.1567
> 00000034 r _java.io.IOException_ref.3036
> 0000002c r _java.lang.Exception_ref.1841

OK, I could fix those, but they're internal anyway.

> or these (from unidentified .o files):
> 
> 000139c0 t
> __ZN3gnu5javax3net3ssl8provider11CipherSuiteC1EPNS3_15CipherAlgorithmEPNS3_20KeyExchangeAlgorithmEPNS3_18SignatureAlgorithmEPNS3_12MacAlgorithmEiiiPN4java4lang6StringE.clone.1

All this .clone stuff is to do with template instantiation in the C++ compiler.

>   Like I said I've seen these mysterious symbols with literal periods in them,
> but I don't know what they actually are.  On closer inspection while writing
> this mail, I suddenly notice that they all appear to be local symbols; if this
> is always the case, then there's no problem at all because we'll never want to
> export them from a DLL.  And noticing the '.clone.' in there makes me wonder
> if this is in fact an entirely C++ issue and not Java at all?  (In which case
> sorry for bothering you with something that's not your department!)

Blame C++, that's what I say.

Andrew.

Re: namespace namespace

[Jakub Jelinek]

On Tue, Jun 30, 2009 at 02:30:18PM +0100, Andrew Haley wrote:
> > 000139c0 t
> > __ZN3gnu5javax3net3ssl8provider11CipherSuiteC1EPNS3_15CipherAlgorithmEPNS3_20KeyExchangeAlgorithmEPNS3_18SignatureAlgorithmEPNS3_12MacAlgorithmEiiiPN4java4lang6StringE.clone.1
> 
> All this .clone stuff is to do with template instantiation in the C++ compiler.

No, C++ FE is innocent.  *.clone.NNN are created by function versioning, see
clone_function_name, and are never exported.
If . doesn't work even for non-exported symbols on your target, the target
is misconfigured (look at NO_DOT_IN_LABEL and NO_DOLLAR_IN_LABEL), if it
works, why do you have problems with it?

	Jakub

Re: namespace namespace

[Dave Korn]

Jakub Jelinek wrote:
> On Tue, Jun 30, 2009 at 02:30:18PM +0100, Andrew Haley wrote:
>>> 000139c0 t
>>> __ZN3gnu5javax3net3ssl8provider11CipherSuiteC1EPNS3_15CipherAlgorithmEPNS3_20KeyExchangeAlgorithmEPNS3_18SignatureAlgorithmEPNS3_12MacAlgorithmEiiiPN4java4lang6StringE.clone.1
>> All this .clone stuff is to do with template instantiation in the C++ compiler.
> 
> No, C++ FE is innocent.  *.clone.NNN are created by function versioning, see
> clone_function_name, and are never exported.
                       ^^^^^^^^^^^^^^^^^^^^^^^

> If . doesn't work even for non-exported symbols on your target, the target
> is misconfigured (look at NO_DOT_IN_LABEL and NO_DOLLAR_IN_LABEL), if it
> works, why do you have problems with it?

  I don't, but I didn't know that for sure until you said above that it would
definitely never be exported.  Thanks for the advice.  From what Andrew says,
it seems the java symbols will also never be exported, so there's not going to
be a problem.

    cheers,
      DaveK

Re: namespace namespace

[Dave Korn]

Dave Korn wrote:
> Jakub Jelinek wrote:
>>>> 000139c0 t
>>>> __ZN3gnu5javax3net3ssl8provider11CipherSuiteC1EPNS3_15CipherAlgorithmEPNS3_20KeyExchangeAlgorithmEPNS3_18SignatureAlgorithmEPNS3_12MacAlgorithmEiiiPN4java4lang6StringE.clone.1

>> No, C++ FE is innocent.  *.clone.NNN are created by function versioning, see
>> clone_function_name, and are never exported.
>                        ^^^^^^^^^^^^^^^^^^^^^^^
> 
>> If . doesn't work even for non-exported symbols on your target, the target
>> is misconfigured (look at NO_DOT_IN_LABEL and NO_DOLLAR_IN_LABEL), if it
>> works, why do you have problems with it?
> 
>   I don't, but I didn't know that for sure until you said above that it would
> definitely never be exported.  

  Ok, now I do, and they are(*).

  I take it that this is a bug in the dllexport attribute getting propagated
unduly to clones when it should only be applied to the original (non-cloned)
version, and not that these clones are meant to be externally visible for some
reason I don't understand?

  There's presumably an issue with the handling in the backend hooks, which I
should be able to track down without too much difficulty, but you could help
me with one bit of advice: how do I detect when I've been handed the tree for
a decl of a cloned function rather than an original?

    cheers,
      DaveK
-- 
(*) - http://sourceware.org/ml/binutils/2009-07/msg00076.html

Re: namespace namespace

[Jakub Jelinek]

On Tue, Jul 07, 2009 at 10:51:27PM +0100, Dave Korn wrote:
> >   I don't, but I didn't know that for sure until you said above that it would
> > definitely never be exported.  
> 
>   Ok, now I do, and they are(*).

That would be a target bug then.

>   I take it that this is a bug in the dllexport attribute getting propagated
> unduly to clones when it should only be applied to the original (non-cloned)
> version, and not that these clones are meant to be externally visible for some
> reason I don't understand?
> 
>   There's presumably an issue with the handling in the backend hooks, which I
> should be able to track down without too much difficulty, but you could help
> me with one bit of advice: how do I detect when I've been handed the tree for
> a decl of a cloned function rather than an original?

  /* Update the properties.
     Make clone visible only within this translation unit.  Make sure
     that is not weak also.
     ??? We cannot use COMDAT linkage because there is no
     ABI support for this.  */
  DECL_EXTERNAL (new_node->decl) = 0;
  DECL_COMDAT_GROUP (new_node->decl) = 0;
  TREE_PUBLIC (new_node->decl) = 0;
  DECL_COMDAT (new_node->decl) = 0;
  DECL_WEAK (new_node->decl) = 0;

Certainly !TREE_PUBLIC functions shouldn't be exported from the current
assembly file in any way...

	Jakub

Re: namespace namespace

[Dave Korn]

Jakub Jelinek wrote:
> On Tue, Jul 07, 2009 at 10:51:27PM +0100, Dave Korn wrote:
>>>   I don't, but I didn't know that for sure until you said above that it would
>>> definitely never be exported.  
>>   Ok, now I do, and they are(*).
> 
> That would be a target bug then.

  Yep, that's what I meant by target hooks.

>>   I take it that this is a bug in the dllexport attribute getting propagated
>> unduly to clones when it should only be applied to the original (non-cloned)
>> version, and not that these clones are meant to be externally visible for some
>> reason I don't understand?
>>
>>   There's presumably an issue with the handling in the backend hooks, which I
>> should be able to track down without too much difficulty, but you could help
>> me with one bit of advice: how do I detect when I've been handed the tree for
>> a decl of a cloned function rather than an original?
> 
>   /* Update the properties.
>      Make clone visible only within this translation unit.  Make sure
>      that is not weak also.
>      ??? We cannot use COMDAT linkage because there is no
>      ABI support for this.  */
>   DECL_EXTERNAL (new_node->decl) = 0;
>   DECL_COMDAT_GROUP (new_node->decl) = 0;
>   TREE_PUBLIC (new_node->decl) = 0;
>   DECL_COMDAT (new_node->decl) = 0;
>   DECL_WEAK (new_node->decl) = 0;
> 
> Certainly !TREE_PUBLIC functions shouldn't be exported from the current
> assembly file in any way...

  That should do for me.  Thanks!

    cheers,
      DaveK

Re: namespace namespace

[Dave Korn]

Jakub Jelinek wrote:

>   /* Update the properties.
>      Make clone visible only within this translation unit.  Make sure
>      that is not weak also.
>      ??? We cannot use COMDAT linkage because there is no
>      ABI support for this.  */
>   DECL_EXTERNAL (new_node->decl) = 0;
>   DECL_COMDAT_GROUP (new_node->decl) = 0;
>   TREE_PUBLIC (new_node->decl) = 0;
>   DECL_COMDAT (new_node->decl) = 0;
>   DECL_WEAK (new_node->decl) = 0;
> 
> Certainly !TREE_PUBLIC functions shouldn't be exported from the current
> assembly file in any way...

  Is there a reason why not to set DECL_ARTIFICIAL on these clones?

    cheers,
      DaveK

Re: namespace namespace

[Ian Lance Taylor]

Andrew Haley <aph@redhat.com> writes:

> It turns out that the mangling of C++ keywords used by gcj is completely
> broken:
>
> In particular, when a member name is a C++ keyword, "$" is appended to the
> name.  However, this is results in an illegal mangled name so it is not
> possible to refer to the member from CNI code.
>
> Also, the set of C++ keywords in gjavah is different from the set in gcj.
>
> Also, types are not checked for C++ keywords, so javax.xml.namespace.* cannot
> be referenced from CNI code.
>
> This patch fixes the problem.
>
> This is an ABI change.  However, the ABI was almost completely broken
> anyway.

Does this require any changes to the demangler?  Note that the demangler
currently has a special case for this:

  /* A Java mangled name may have a trailing '$' if it is a C++
     keyword.  This '$' is not included in the length count.  We just
     ignore the '$'.  */
  if ((di->options & DMGL_JAVA) != 0
      && d_peek_char (di) == '$')
    d_advance (di, 1);

Ian

Re: namespace namespace

[Andrew Haley]

Ian Lance Taylor wrote:
> Andrew Haley <aph@redhat.com> writes:
> 
>> It turns out that the mangling of C++ keywords used by gcj is completely
>> broken:
>>
>> In particular, when a member name is a C++ keyword, "$" is appended to the
>> name.  However, this is results in an illegal mangled name so it is not
>> possible to refer to the member from CNI code.
>>
>> Also, the set of C++ keywords in gjavah is different from the set in gcj.
>>
>> Also, types are not checked for C++ keywords, so javax.xml.namespace.* cannot
>> be referenced from CNI code.
>>
>> This patch fixes the problem.
>>
>> This is an ABI change.  However, the ABI was almost completely broken
>> anyway.
> 
> Does this require any changes to the demangler? 

Good point.  I suppose that for really pedantic accuracy there should be, yes.

> Note that the demangler currently has a special case for this:
> 
>   /* A Java mangled name may have a trailing '$' if it is a C++
>      keyword.  This '$' is not included in the length count.  We just
>      ignore the '$'.  */
>   if ((di->options & DMGL_JAVA) != 0
>       && d_peek_char (di) == '$')
>     d_advance (di, 1);

How interesting.  I didn't know that.

I'm pretty sure that it can't possibly have worked, since there would have been
no way for CNI code to refer to methods or objects that had name components
that were C++ keywords.  Unless, perhaps, there was a similar hack in the C++
compiler, but what would have been the point?  Baffling.

Thanks,
Andrew.

Re: namespace namespace

[Tom Tromey]

>>>>> "Andrew" == Andrew Haley <aph@redhat.com> writes:

>> Note that the demangler currently has a special case for this:
>> /* A Java mangled name may have a trailing '$' if it is a C++
>> keyword.  This '$' is not included in the length count.  We just
>> ignore the '$'.  */
>> if ((di->options & DMGL_JAVA) != 0
>> && d_peek_char (di) == '$')
>> d_advance (di, 1);

Andrew> How interesting.  I didn't know that.

Andrew> I'm pretty sure that it can't possibly have worked, since
Andrew> there would have been no way for CNI code to refer to methods
Andrew> or objects that had name components that were C++ keywords.
Andrew> Unless, perhaps, there was a similar hack in the C++ compiler,
Andrew> but what would have been the point?  Baffling.

Baffling and gcj ... must be my doing!

I don't think I considered namespace components back in the day, only
virtual methods and instance fields.  These were needed to compile
libjava itself (I've long forgotten the examples) and could work
purely by changing gcjh's output.

Tom