Patch to fix PR9861

Patch to fix PR9861

[TJ Laurenzo]

[-- Attachment #1: Type: text/plain, Size: 527 bytes --]

The attached patch corrects PR9861 by including the return type in
mangled names in the C++ and Java compilers for Java class methods. 
This changes the ABI for Java types but does not make any changes to
C++ programs that do not access Java types.

This patch has been tested against CVS HEAD from 9/22/2005 on
i686-pc-linux-gnu (with Jacks added to the source tree) and introduces
no new testsuite failures.

This issue has been discussed on the java mailing list:
http://gcc.gnu.org/ml/java/2005-08/msg00061.html

[-- Attachment #2: pr9861.diff --]
[-- Type: application/octet-stream, Size: 6120 bytes --]

Index: gcc/cp/mangle.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/cp/mangle.c,v
retrieving revision 1.128
diff -c -3 -p -r1.128 mangle.c
*** gcc/cp/mangle.c	13 Sep 2005 15:15:31 -0000	1.128
--- gcc/cp/mangle.c	22 Sep 2005 21:05:30 -0000
*************** write_encoding (const tree decl)
*** 742,747 ****
--- 742,748 ----
      {
        tree fn_type;
        tree d;
+       int mangle_ret_type;
  
        if (decl_is_template_id (decl, NULL))
  	{
*************** write_encoding (const tree decl)
*** 760,770 ****
  	  d = decl;
  	}
  
        write_bare_function_type (fn_type,
! 				(!DECL_CONSTRUCTOR_P (decl)
! 				 && !DECL_DESTRUCTOR_P (decl)
! 				 && !DECL_CONV_FN_P (decl)
! 				 && decl_is_template_id (decl, NULL)),
  				d);
      }
  }
--- 761,780 ----
  	  d = decl;
  	}
  
+       mangle_ret_type =
+  	!DECL_CONSTRUCTOR_P (decl)
+  	 && !DECL_DESTRUCTOR_P (decl)
+  	 && !DECL_CONV_FN_P (decl)
+  	 && (
+  		decl_is_template_id (decl, NULL) ||
+  		(	/* Detect java methods and include the return type */
+  			DECL_FUNCTION_MEMBER_P (decl)
+  			&& TYPE_FOR_JAVA ( DECL_CONTEXT(decl) ) 
+  		)
+  	    );
+ 
        write_bare_function_type (fn_type,
!                                 mangle_ret_type,
  				d);
      }
  }
Index: gcc/java/mangle.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/java/mangle.c,v
retrieving revision 1.34
diff -c -3 -p -r1.34 mangle.c
*** gcc/java/mangle.c	25 Jun 2005 00:33:04 -0000	1.34
--- gcc/java/mangle.c	22 Sep 2005 21:05:30 -0000
*************** mangle_method_decl (tree mdecl)
*** 188,193 ****
--- 188,197 ----
    if (TREE_CODE (TREE_TYPE (mdecl)) == METHOD_TYPE)
      arglist = TREE_CHAIN (arglist);
    
+   /* Mangle the return type IF not a constructor */
+   if (!ID_INIT_P(method_name)) 
+     mangle_type(TREE_TYPE(TREE_TYPE(mdecl)));
+   
    /* No arguments is easy. We shortcut it. */
    if (arglist == end_params_node)
      obstack_1grow (mangle_obstack, 'v');
Index: gcc/java/builtins.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/java/builtins.c,v
retrieving revision 1.34
diff -c -3 -p -r1.34 builtins.c
*** gcc/java/builtins.c	18 Sep 2005 19:10:10 -0000	1.34
--- gcc/java/builtins.c	22 Sep 2005 21:05:30 -0000
*************** initialize_builtins (void)
*** 194,236 ****
  		  float_ftype_float_float, "fmodf", BUILTIN_CONST);
  
    define_builtin (BUILT_IN_ACOS, "__builtin_acos",
! 		  double_ftype_double, "_ZN4java4lang4Math4acosEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ASIN, "__builtin_asin",
! 		  double_ftype_double, "_ZN4java4lang4Math4asinEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN, "__builtin_atan",
! 		  double_ftype_double, "_ZN4java4lang4Math4atanEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN2, "__builtin_atan2",
! 		  double_ftype_double_double, "_ZN4java4lang4Math5atan2Edd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_CEIL, "__builtin_ceil",
! 		  double_ftype_double, "_ZN4java4lang4Math4ceilEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_COS, "__builtin_cos",
! 		  double_ftype_double, "_ZN4java4lang4Math3cosEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_EXP, "__builtin_exp",
! 		  double_ftype_double, "_ZN4java4lang4Math3expEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_FLOOR, "__builtin_floor",
! 		  double_ftype_double, "_ZN4java4lang4Math5floorEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_LOG, "__builtin_log",
! 		  double_ftype_double, "_ZN4java4lang4Math3logEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_POW, "__builtin_pow",
! 		  double_ftype_double_double, "_ZN4java4lang4Math3powEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SIN, "__builtin_sin",
! 		  double_ftype_double, "_ZN4java4lang4Math3sinEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SQRT, "__builtin_sqrt",
! 		  double_ftype_double, "_ZN4java4lang4Math4sqrtEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_TAN, "__builtin_tan",
! 		  double_ftype_double, "_ZN4java4lang4Math3tanEd",
  		  BUILTIN_CONST);
    
    t = tree_cons (NULL_TREE, boolean_type_node, end_params_node);
--- 194,236 ----
  		  float_ftype_float_float, "fmodf", BUILTIN_CONST);
  
    define_builtin (BUILT_IN_ACOS, "__builtin_acos",
! 		  double_ftype_double, "_ZN4java4lang4Math4acosEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ASIN, "__builtin_asin",
! 		  double_ftype_double, "_ZN4java4lang4Math4asinEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN, "__builtin_atan",
! 		  double_ftype_double, "_ZN4java4lang4Math4atanEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN2, "__builtin_atan2",
! 		  double_ftype_double_double, "_ZN4java4lang4Math5atan2Eddd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_CEIL, "__builtin_ceil",
! 		  double_ftype_double, "_ZN4java4lang4Math4ceilEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_COS, "__builtin_cos",
! 		  double_ftype_double, "_ZN4java4lang4Math3cosEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_EXP, "__builtin_exp",
! 		  double_ftype_double, "_ZN4java4lang4Math3expEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_FLOOR, "__builtin_floor",
! 		  double_ftype_double, "_ZN4java4lang4Math5floorEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_LOG, "__builtin_log",
! 		  double_ftype_double, "_ZN4java4lang4Math3logEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_POW, "__builtin_pow",
! 		  double_ftype_double_double, "_ZN4java4lang4Math3powEddd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SIN, "__builtin_sin",
! 		  double_ftype_double, "_ZN4java4lang4Math3sinEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SQRT, "__builtin_sqrt",
! 		  double_ftype_double, "_ZN4java4lang4Math4sqrtEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_TAN, "__builtin_tan",
! 		  double_ftype_double, "_ZN4java4lang4Math3tanEdd",
  		  BUILTIN_CONST);
    
    t = tree_cons (NULL_TREE, boolean_type_node, end_params_node);

[-- Attachment #3: ChangeLog-Java.txt --]
[-- Type: text/plain, Size: 225 bytes --]

2005-09-22   Terry Laurenzo   <tlaurenzo@gmail.com>

	* mangle.c (mangle_method_decl): Mangle return type of normal methods
	* builtins.c (initialize_builtins): Change builtin mangled name 
	constants to include return types

[-- Attachment #4: ChangeLog-cp.txt --]
[-- Type: text/plain, Size: 131 bytes --]

2005-09-22   Terry Laurenzo   <tlaurenzo@gmail.com>

	* mangle.c (write_encoding): Mangle return type for methods of Java
	classes

Re: Patch to fix PR9861

[Ian Lance Taylor]

TJ Laurenzo <tlaurenzo@gmail.com> writes:

> The attached patch corrects PR9861 by including the return type in
> mangled names in the C++ and Java compilers for Java class methods. 
> This changes the ABI for Java types but does not make any changes to
> C++ programs that do not access Java types.

How is the demangler going to work with this patch?  Is there any way
for the demangler to determine which names are Java names and which
are C++ names?  I don't see any obvious way, though I haven't looked
that hard.

If there is no way at present, then I recommend adding a 'J' at the
start of bare-function-type if the return type is encoded.  That is,
do this:
    <bare-function-type> ::= [J] <type>+
That should be unambiguous.

(I note in passing that your patch is not formatted correctly
according to the GNU standards.  The correct formatting would be
something like:
  mangle_ret_type = (!DECL_CONSTRUCTOR_P (decl)
		     && !DECL_DESTRUCTOR_P (decl)
		     && !DECL_CONV_FN_P (decl)
		     && (decl_is_template_id (decl, NULL)
			 || (DECL_FUNCTION_MEMBER_P (decl)
			     && TYPE_FOR_JAVA (DECL_CONTEXT (decl)))));
Thanks.)

Ian

Re: Patch to fix PR9861

[Ian Lance Taylor]

Ian Lance Taylor <ian@airs.com> writes:

> TJ Laurenzo <tlaurenzo@gmail.com> writes:
> 
> > The attached patch corrects PR9861 by including the return type in
> > mangled names in the C++ and Java compilers for Java class methods. 
> > This changes the ABI for Java types but does not make any changes to
> > C++ programs that do not access Java types.
> 
> How is the demangler going to work with this patch?  Is there any way
> for the demangler to determine which names are Java names and which
> are C++ names?  I don't see any obvious way, though I haven't looked
> that hard.

By the way, considering the demangler is not an unimportant point as
any ambiguity in demangling is an ambiguity that can be happen when
linking C++ and Java code together.  For example, your proposed
mangling for
    java.lang.Math.acos(double)
appears to be
    _ZN4java4lang4Math4acosEdd
which is the mangled name which will be used for the C++ function
    java::lang::Math::acos(double, double)

Ian

Re: Patch to fix PR9861

[David Daney]

Ian Lance Taylor wrote:
> Ian Lance Taylor <ian@airs.com> writes:
> 
> 
>>TJ Laurenzo <tlaurenzo@gmail.com> writes:
>>
>>
>>>The attached patch corrects PR9861 by including the return type in
>>>mangled names in the C++ and Java compilers for Java class methods. 
>>>This changes the ABI for Java types but does not make any changes to
>>>C++ programs that do not access Java types.
>>
>>How is the demangler going to work with this patch?  Is there any way
>>for the demangler to determine which names are Java names and which
>>are C++ names?  I don't see any obvious way, though I haven't looked
>>that hard.
> 
> 
> By the way, considering the demangler is not an unimportant point as
> any ambiguity in demangling is an ambiguity that can be happen when
> linking C++ and Java code together.  For example, your proposed
> mangling for
>     java.lang.Math.acos(double)
> appears to be
>     _ZN4java4lang4Math4acosEdd
> which is the mangled name which will be used for the C++ function
>     java::lang::Math::acos(double, double)
> 

Along tenuously similar vein, it is sometimes nice to be able to trick 
GDB into thinking it *is* C++ (using set lang c++) and referring to 
things by their C++ names.

I think it would be nice if this were still possible.  I think gdb uses 
the demangler as part of its name completion system, so breaking the 
demangler will make debugging harder.

David Daney

Re: Patch to fix PR9861

[TJ Laurenzo]

> By the way, considering the demangler is not an unimportant point as
> any ambiguity in demangling is an ambiguity that can be happen when
> linking C++ and Java code together.  For example, your proposed
> mangling for
>     java.lang.Math.acos(double)
> appears to be
>     _ZN4java4lang4Math4acosEdd
> which is the mangled name which will be used for the C++ function
>     java::lang::Math::acos(double, double)

I think your point is valid but this situation actually wouldn't
happen because the C++ mangler would recognize that java::lang::Math
is a Java type and would apply the Java mangling rules.    I think
that with this change, Java has reached the point where it needs to
diverge from the C++ ABI and it would probably be a good idea to have
a way to distinguish the Java form from the C++ form.  My only concern
is that introducing a new divergent form would break existing tools
that do demangling until they could catchup.

TJ

Re: Patch to fix PR9861

[Ian Lance Taylor]

TJ Laurenzo <tlaurenzo@gmail.com> writes:

> > By the way, considering the demangler is not an unimportant point as
> > any ambiguity in demangling is an ambiguity that can be happen when
> > linking C++ and Java code together.  For example, your proposed
> > mangling for
> >     java.lang.Math.acos(double)
> > appears to be
> >     _ZN4java4lang4Math4acosEdd
> > which is the mangled name which will be used for the C++ function
> >     java::lang::Math::acos(double, double)
> 
> I think your point is valid but this situation actually wouldn't
> happen because the C++ mangler would recognize that java::lang::Math
> is a Java type and would apply the Java mangling rules.    I think
> that with this change, Java has reached the point where it needs to
> diverge from the C++ ABI and it would probably be a good idea to have
> a way to distinguish the Java form from the C++ form.  My only concern
> is that introducing a new divergent form would break existing tools
> that do demangling until they could catchup.

I may have chosen a poor example.

My point really is that if we permit mangling ambiguity, then a pure
C++ function may get the same mangled name as a Java function, even
though the functions will have different signatures.  That then
becomes a problem if you want to link C++ code with Java code in the
same executable.  It's not particularly likely, but I think it's worth
taking the extra step to make it impossible.

Ian

Re: Patch to fix PR9861

[TJ Laurenzo]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

I am attaching a revised patch that corrects the ambiguity problem in
accordance with Ian's suggestion.  It includes an update to the
demangler and test cases to support it.

It should be noted that with this patch applied, mangled names
produced by the Java and C++ compilers for methods of Java classes
cannot be demangled by old tools until they are patched with the
libiberty cp-demangle.c updates included in this patch.

Of course, this patch also breaks old-style Java binary compatibility
(such as exists to begin with).

I have built and successfully run the testsuite with this patch
against i686-pc-linux-gnu.

TJ

[-- Attachment #2: pr9861-a.diff --]
[-- Type: application/octet-stream, Size: 11663 bytes --]

Index: libiberty/cp-demangle.c
===================================================================
RCS file: /cvsroot/gcc/gcc/libiberty/cp-demangle.c,v
retrieving revision 1.83
diff -c -3 -p -r1.83 cp-demangle.c
*** libiberty/cp-demangle.c	1 Jul 2005 16:39:35 -0000	1.83
--- libiberty/cp-demangle.c	23 Sep 2005 18:25:22 -0000
*************** d_function_type (struct d_info *di)
*** 1939,1945 ****
    return ret;
  }
  
! /* <bare-function-type> ::= <type>+  */
  
  static struct demangle_component *
  d_bare_function_type (struct d_info *di, int has_return_type)
--- 1939,1945 ----
    return ret;
  }
  
! /* <bare-function-type> ::= [J]<type>+  */
  
  static struct demangle_component *
  d_bare_function_type (struct d_info *di, int has_return_type)
*************** d_bare_function_type (struct d_info *di,
*** 1947,1959 ****
    struct demangle_component *return_type;
    struct demangle_component *tl;
    struct demangle_component **ptl;
  
    return_type = NULL;
    tl = NULL;
    ptl = &tl;
    while (1)
      {
-       char peek;
        struct demangle_component *type;
  
        peek = d_peek_char (di);
--- 1947,1968 ----
    struct demangle_component *return_type;
    struct demangle_component *tl;
    struct demangle_component **ptl;
+   char peek;
+ 
+   /* Detect special qualifier indicating that the first argument
+      is the return type.  */
+   peek = d_peek_char (di);
+   if (peek == 'J')
+     {
+       d_advance (di, 1);
+       has_return_type = 1;
+     }
  
    return_type = NULL;
    tl = NULL;
    ptl = &tl;
    while (1)
      {
        struct demangle_component *type;
  
        peek = d_peek_char (di);
Index: libiberty/ChangeLog
===================================================================
RCS file: /cvsroot/gcc/gcc/libiberty/ChangeLog,v
retrieving revision 1.601
diff -c -3 -p -r1.601 ChangeLog
*** libiberty/ChangeLog	15 Sep 2005 00:46:19 -0000	1.601
--- libiberty/ChangeLog	23 Sep 2005 18:25:24 -0000
***************
*** 1,3 ****
--- 1,10 ----
+ 2005-09-23  Terry Laurenzo  <tlaurenzo@gmail.com>
+ 
+ 	PR java/9861
+ 	* cp-demangle.c (d_bare_function_type): Recognize new 'J' qualifier
+ 	and include return type when found.
+ 	* testsuite/demangle-expected: Test cases to verify extended encoding
+ 
  2005-09-14  Christopher Faylor  <cgf@timesys.com>
  
  	* pex-win32.c: Include "windows.h".
Index: libiberty/testsuite/demangle-expected
===================================================================
RCS file: /cvsroot/gcc/gcc/libiberty/testsuite/demangle-expected,v
retrieving revision 1.33
diff -c -3 -p -r1.33 demangle-expected
*** libiberty/testsuite/demangle-expected	1 Jul 2005 16:39:36 -0000	1.33
--- libiberty/testsuite/demangle-expected	23 Sep 2005 18:25:24 -0000
*************** _test_array__L_1__B23b___clean.6
*** 3781,3783 ****
--- 3781,3797 ----
  --format=java
  _ZGAN4java4lang5Class7forNameEPNS0_6StringE
  hidden alias for java.lang.Class.forName(java.lang.String)
+ #
+ # Test cases to verify encoding that determines if a return type is present
+ # Related to PR9861
+ --format=java
+ _ZN4java4lang4Math4acosEJdd
+ double java.lang.Math.acos(double)
+ #
+ --format=auto
+ _ZN4java4lang4Math4acosEJdd
+ double java::lang::Math::acos(double)
+ #
+ --format=auto
+ _ZN4java4lang4Math4acosEJvd
+ void java::lang::Math::acos(double)
\ No newline at end of file
Index: gcc/java/mangle.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/java/mangle.c,v
retrieving revision 1.34
diff -c -3 -p -r1.34 mangle.c
*** gcc/java/mangle.c	25 Jun 2005 00:33:04 -0000	1.34
--- gcc/java/mangle.c	23 Sep 2005 18:25:24 -0000
*************** mangle_method_decl (tree mdecl)
*** 188,193 ****
--- 188,201 ----
    if (TREE_CODE (TREE_TYPE (mdecl)) == METHOD_TYPE)
      arglist = TREE_CHAIN (arglist);
    
+   /* Output literal 'J' and mangle the return type IF not a 
+      constructor.  */
+   if (!ID_INIT_P (method_name))
+     {
+       obstack_1grow (mangle_obstack, 'J');
+       mangle_type(TREE_TYPE(TREE_TYPE(mdecl)));
+     }
+   
    /* No arguments is easy. We shortcut it. */
    if (arglist == end_params_node)
      obstack_1grow (mangle_obstack, 'v');
Index: gcc/java/builtins.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/java/builtins.c,v
retrieving revision 1.34
diff -c -3 -p -r1.34 builtins.c
*** gcc/java/builtins.c	18 Sep 2005 19:10:10 -0000	1.34
--- gcc/java/builtins.c	23 Sep 2005 18:25:24 -0000
*************** initialize_builtins (void)
*** 194,236 ****
  		  float_ftype_float_float, "fmodf", BUILTIN_CONST);
  
    define_builtin (BUILT_IN_ACOS, "__builtin_acos",
! 		  double_ftype_double, "_ZN4java4lang4Math4acosEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ASIN, "__builtin_asin",
! 		  double_ftype_double, "_ZN4java4lang4Math4asinEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN, "__builtin_atan",
! 		  double_ftype_double, "_ZN4java4lang4Math4atanEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN2, "__builtin_atan2",
! 		  double_ftype_double_double, "_ZN4java4lang4Math5atan2Edd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_CEIL, "__builtin_ceil",
! 		  double_ftype_double, "_ZN4java4lang4Math4ceilEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_COS, "__builtin_cos",
! 		  double_ftype_double, "_ZN4java4lang4Math3cosEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_EXP, "__builtin_exp",
! 		  double_ftype_double, "_ZN4java4lang4Math3expEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_FLOOR, "__builtin_floor",
! 		  double_ftype_double, "_ZN4java4lang4Math5floorEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_LOG, "__builtin_log",
! 		  double_ftype_double, "_ZN4java4lang4Math3logEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_POW, "__builtin_pow",
! 		  double_ftype_double_double, "_ZN4java4lang4Math3powEdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SIN, "__builtin_sin",
! 		  double_ftype_double, "_ZN4java4lang4Math3sinEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SQRT, "__builtin_sqrt",
! 		  double_ftype_double, "_ZN4java4lang4Math4sqrtEd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_TAN, "__builtin_tan",
! 		  double_ftype_double, "_ZN4java4lang4Math3tanEd",
  		  BUILTIN_CONST);
    
    t = tree_cons (NULL_TREE, boolean_type_node, end_params_node);
--- 194,236 ----
  		  float_ftype_float_float, "fmodf", BUILTIN_CONST);
  
    define_builtin (BUILT_IN_ACOS, "__builtin_acos",
! 		  double_ftype_double, "_ZN4java4lang4Math4acosEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ASIN, "__builtin_asin",
! 		  double_ftype_double, "_ZN4java4lang4Math4asinEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN, "__builtin_atan",
! 		  double_ftype_double, "_ZN4java4lang4Math4atanEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_ATAN2, "__builtin_atan2",
! 		  double_ftype_double_double, "_ZN4java4lang4Math5atan2EJddd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_CEIL, "__builtin_ceil",
! 		  double_ftype_double, "_ZN4java4lang4Math4ceilEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_COS, "__builtin_cos",
! 		  double_ftype_double, "_ZN4java4lang4Math3cosEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_EXP, "__builtin_exp",
! 		  double_ftype_double, "_ZN4java4lang4Math3expEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_FLOOR, "__builtin_floor",
! 		  double_ftype_double, "_ZN4java4lang4Math5floorEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_LOG, "__builtin_log",
! 		  double_ftype_double, "_ZN4java4lang4Math3logEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_POW, "__builtin_pow",
! 		  double_ftype_double_double, "_ZN4java4lang4Math3powEJddd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SIN, "__builtin_sin",
! 		  double_ftype_double, "_ZN4java4lang4Math3sinEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_SQRT, "__builtin_sqrt",
! 		  double_ftype_double, "_ZN4java4lang4Math4sqrtEJdd",
  		  BUILTIN_CONST);
    define_builtin (BUILT_IN_TAN, "__builtin_tan",
! 		  double_ftype_double, "_ZN4java4lang4Math3tanEJdd",
  		  BUILTIN_CONST);
    
    t = tree_cons (NULL_TREE, boolean_type_node, end_params_node);
Index: gcc/java/ChangeLog
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/java/ChangeLog,v
retrieving revision 1.1666
diff -c -3 -p -r1.1666 ChangeLog
*** gcc/java/ChangeLog	21 Sep 2005 13:34:27 -0000	1.1666
--- gcc/java/ChangeLog	23 Sep 2005 18:25:28 -0000
***************
*** 1,3 ****
--- 1,11 ----
+ 2005-09-23   Terry Laurenzo   <tlaurenzo@gmail.com>
+ 
+ 	PR java/9861
+ 	* mangle.c (mangle_method_decl): Mangle Java methods by prepending 'J'
+ 	to bare_function_type and including the return type
+ 	* builtins.c (initialize_builtins): Change builtin mangled name 
+ 	constants to conform to new mangling scheme
+ 
  2005-09-21  Ranjit Mathew  <rmathew@gcc.gnu.org>
  
  	PR java/21418
Index: gcc/cp/mangle.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/cp/mangle.c,v
retrieving revision 1.128
diff -c -3 -p -r1.128 mangle.c
*** gcc/cp/mangle.c	13 Sep 2005 15:15:31 -0000	1.128
--- gcc/cp/mangle.c	23 Sep 2005 18:25:29 -0000
*************** write_function_type (const tree type)
*** 1858,1873 ****
     is mangled before the parameter types.  If non-NULL, DECL is
     FUNCTION_DECL for the function whose type is being emitted.
  
!      <bare-function-type> ::= </signature/ type>+  */
  
  static void
  write_bare_function_type (const tree type, const int include_return_type_p,
  			  const tree decl)
  {
    MANGLE_TRACE_TREE ("bare-function-type", type);
  
    /* Mangle the return type, if requested.  */
!   if (include_return_type_p)
      write_type (TREE_TYPE (type));
  
    /* Now mangle the types of the arguments.  */
--- 1858,1895 ----
     is mangled before the parameter types.  If non-NULL, DECL is
     FUNCTION_DECL for the function whose type is being emitted.
  
!    If DECL is a member of a Java type, then a literal 'J'
!    is output and the return type is mangled as if INCLUDE_RETURN_TYPE
!    were nonzero.
! 
!      <bare-function-type> ::= [J]</signature/ type>+  */
  
  static void
  write_bare_function_type (const tree type, const int include_return_type_p,
  			  const tree decl)
  {
+   int java_method_p;
+ 
    MANGLE_TRACE_TREE ("bare-function-type", type);
  
+   /* Detect Java methods and emit special encoding.  */
+   if (decl != NULL
+       && DECL_FUNCTION_MEMBER_P (decl)
+       && TYPE_FOR_JAVA (DECL_CONTEXT (decl))
+       && !DECL_CONSTRUCTOR_P (decl)
+       && !DECL_DESTRUCTOR_P (decl)
+       && !DECL_CONV_FN_P (decl))
+     {
+       java_method_p = 1;
+       write_char ('J');
+     }
+   else
+     {
+       java_method_p = 0;
+     }
+ 
    /* Mangle the return type, if requested.  */
!   if (include_return_type_p || java_method_p)
      write_type (TREE_TYPE (type));
  
    /* Now mangle the types of the arguments.  */
Index: gcc/cp/ChangeLog
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/cp/ChangeLog,v
retrieving revision 1.4900
diff -c -3 -p -r1.4900 ChangeLog
*** gcc/cp/ChangeLog	22 Sep 2005 00:11:18 -0000	1.4900
--- gcc/cp/ChangeLog	23 Sep 2005 18:25:30 -0000
***************
*** 1,3 ****
--- 1,9 ----
+ 2005-09-23   Terry Laurenzo   <tlaurenzo@gmail.com>
+ 
+ 	PR java/9861
+ 	* mangle.c (write_bare_function_type): Mangle return type for 
+ 	methods of Java classes
+ 
  2005-09-21  Mark Mitchell  <mark@codesourcery.com>
  
  	PR c++/23993

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > I am attaching a revised patch that corrects the ambiguity problem in
 > accordance with Ian's suggestion.  It includes an update to the
 > demangler and test cases to support it.
 > 
 > It should be noted that with this patch applied, mangled names
 > produced by the Java and C++ compilers for methods of Java classes
 > cannot be demangled by old tools until they are patched with the
 > libiberty cp-demangle.c updates included in this patch.
 > 
 > Of course, this patch also breaks old-style Java binary compatibility
 > (such as exists to begin with).
 > 
 > I have built and successfully run the testsuite with this patch
 > against i686-pc-linux-gnu.

I wonder what this does to debugging.  I'm going to try it now.

Andrew.

Re: Patch to fix PR9861

[TJ Laurenzo]

Andrew Haley wrote:
> I wonder what this does to debugging.  I'm going to try it now.


If you build gdb with the patched cp-demangle.c the only difference
*should* be that demangled names will include the return type (without
cp-demangle.c patched, you will only see mangled names).  I have not
tested this against gdb so I am interested to know how it goes.

I have tested this against binutils by applying the cp-demangle.c
patch and verifying that demangling names in object files works
properly (ie.  nm -C).  I have also added a couple of test cases to
libiberty that exercise this change.

TJ

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > Andrew Haley wrote:
 > > I wonder what this does to debugging.  I'm going to try it now.
 > 
 > If you build gdb with the patched cp-demangle.c the only difference
 > *should* be that demangled names will include the return type (without
 > cp-demangle.c patched, you will only see mangled names).  I have not
 > tested this against gdb so I am interested to know how it goes.
 > 
 > I have tested this against binutils by applying the cp-demangle.c
 > patch and verifying that demangling names in object files works
 > properly (ie.  nm -C).  I have also added a couple of test cases to
 > libiberty that exercise this change.

We're going to need to be able to run with an unpatched gdb and an
unpatched binutils for a little while.  I understand that there may be
some odd behaviour, but basic debugging must still be possible.

Andrew.

Re: Patch to fix PR9861

[TJ Laurenzo]

On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
> TJ Laurenzo writes:
>  > Andrew Haley wrote:
>  > > I wonder what this does to debugging.  I'm going to try it now.
>  >
>  > If you build gdb with the patched cp-demangle.c the only difference
>  > *should* be that demangled names will include the return type (without
>  > cp-demangle.c patched, you will only see mangled names).  I have not
>  > tested this against gdb so I am interested to know how it goes.
>  >
>  > I have tested this against binutils by applying the cp-demangle.c
>  > patch and verifying that demangling names in object files works
>  > properly (ie.  nm -C).  I have also added a couple of test cases to
>  > libiberty that exercise this change.
>
> We're going to need to be able to run with an unpatched gdb and an
> unpatched binutils for a little while.  I understand that there may be
> some odd behaviour, but basic debugging must still be possible.
>
> Andrew.
>
>
The current "odd" behavior in an unpatched binutils is that anytime it
tries to demangle the name, it will fail and only print the mangled
name.  I would expect similar behavior from GDB (except that in GDB,
the lack of demangling would really complicate things more than the
same lack in binutils).  So, my opinion is that GDB is the main
problem here, since this patch effectively makes Java debugging very
difficult with an unpatched GDB.

I will note that with my original patch, both tools would print
demangled names but the return type would be listed as the first
parameter.  In addition to being odd, this also has other problems
that Ian pointed out.

If we could find an encoding format that was recognized by the current
demangler and provided uniqueness guarantees consistent with the new
patch, this could work.... perhaps a flag character somewhere that the
current demangler would ignore instead of abort on.

TJ

Re: Patch to fix PR9861

[TJ Laurenzo]

> If we could find an encoding format that was recognized by the current
> demangler and provided uniqueness guarantees consistent with the new
> patch, this could work.... perhaps a flag character somewhere that the
> current demangler would ignore instead of abort on.

For example, if instead of encoding the return type prior to all of
the other types, we could encode it after an implied "void" at the end
of the signature.  This would be unambiguos and recognizable for
patched toolsets but would provide some degree of backwards
compatibility.  For example, the encoding for Math.pow would be:
_ZN4java4lang4Math3powEddvd
Under an unpatched toolset, this would demangle to:
   java.lang.Math.pow(double, double, void, double)
A patched toolset could recognize the pattern and instead print:
   double java.lang.Math.pow(double, double)

This is the only way I could see to preserve any semblance of
backwards compatibility.  I really don't like it, though.  It would be
providing backwards compatibility at the expense of clarity and
simplicity.  In particular, the function d_bare_function_type in
cp-demangle.c would have to maintain a two type look-ahead.  Perhaps,
keeping the return type encoded as the first argument and separating
it from the rest of the argument list by a "v" could keep the
look-ahead logic isolated to the front of the function.

TJ

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
 > > TJ Laurenzo writes:
 > >  > Andrew Haley wrote:
 > >  > > I wonder what this does to debugging.  I'm going to try it now.
 > >  >
 > >  > If you build gdb with the patched cp-demangle.c the only difference
 > >  > *should* be that demangled names will include the return type (without
 > >  > cp-demangle.c patched, you will only see mangled names).  I have not
 > >  > tested this against gdb so I am interested to know how it goes.
 > >  >
 > >  > I have tested this against binutils by applying the cp-demangle.c
 > >  > patch and verifying that demangling names in object files works
 > >  > properly (ie.  nm -C).  I have also added a couple of test cases to
 > >  > libiberty that exercise this change.
 > >
 > > We're going to need to be able to run with an unpatched gdb and an
 > > unpatched binutils for a little while.  I understand that there may be
 > > some odd behaviour, but basic debugging must still be possible.
 > >
 > The current "odd" behavior in an unpatched binutils is that anytime it
 > tries to demangle the name, it will fail and only print the mangled
 > name.

Yes.  This means that you can't set a breakpoint on
'main(java.lang.String[])'.  The only way you can set a breakpoint is
if you know the mangled string.

 > I would expect similar behavior from GDB (except that in GDB, the
 > lack of demangling would really complicate things more than the
 > same lack in binutils).  So, my opinion is that GDB is the main
 > problem here, since this patch effectively makes Java debugging
 > very difficult with an unpatched GDB.

It does.  However, it might be possible to persuade people to ship
updated bnutils on free operating systems.  Unfree systems are going
to be much more problematic, though.

 > I will note that with my original patch, both tools would print
 > demangled names but the return type would be listed as the first
 > parameter.  In addition to being odd, this also has other problems
 > that Ian pointed out.

I don't think we can live with that!  :-)

 > If we could find an encoding format that was recognized by the
 > current demangler and provided uniqueness guarantees consistent
 > with the new patch, this could work.... perhaps a flag character
 > somewhere that the current demangler would ignore instead of abort
 > on.

Andrew.

Re: Patch to fix PR9861

[Ian Lance Taylor]

Andrew Haley <aph@redhat.com> writes:

>  > I would expect similar behavior from GDB (except that in GDB, the
>  > lack of demangling would really complicate things more than the
>  > same lack in binutils).  So, my opinion is that GDB is the main
>  > problem here, since this patch effectively makes Java debugging
>  > very difficult with an unpatched GDB.
> 
> It does.  However, it might be possible to persuade people to ship
> updated bnutils on free operating systems.  Unfree systems are going
> to be much more problematic, though.

How much do we care?  Presumably we are talking about people who are
downloading their own gcc and building it themselves.  Otherwise,
keeping gcc and gdb in synch is the responsibility of the distributor.
People who download their own gcc can always download their own gdb.

Ian

Re: Patch to fix PR9861

[Andrew Haley]

Ian Lance Taylor writes:
 > Andrew Haley <aph@redhat.com> writes:
 > 
 > >  > I would expect similar behavior from GDB (except that in GDB, the
 > >  > lack of demangling would really complicate things more than the
 > >  > same lack in binutils).  So, my opinion is that GDB is the main
 > >  > problem here, since this patch effectively makes Java debugging
 > >  > very difficult with an unpatched GDB.
 > > 
 > > It does.  However, it might be possible to persuade people to ship
 > > updated bnutils on free operating systems.  Unfree systems are going
 > > to be much more problematic, though.
 > 
 > How much do we care?

That's a good question.  I don't know, really: what I do know is that
I don't want to be fielding many bug reports about this.  

 > Presumably we are talking about people who are downloading their
 > own gcc and building it themselves.  Otherwise, keeping gcc and gdb
 > in synch is the responsibility of the distributor.  People who
 > download their own gcc can always download their own gdb.

I agree.  However, I think we should be fairly careful about the
timing for this change.  One thought: if we make this change just
before 4.1 branches, we'll have the branch stabilization period to get
the changes pushed through to gdb and binutils.

Andrew.

Re: Patch to fix PR9861

[TJ Laurenzo]

>  > I would expect similar behavior from GDB (except that in GDB, the
>  > lack of demangling would really complicate things more than the
>  > same lack in binutils).  So, my opinion is that GDB is the main
>  > problem here, since this patch effectively makes Java debugging
>  > very difficult with an unpatched GDB.
>
> It does.  However, it might be possible to persuade people to ship
> updated bnutils on free operating systems.  Unfree systems are going
> to be much more problematic, though.
I'm not sure we are talking about a hard requirement to update
binutils.  The old version works.  There are a few instances that
could cause confusion but which could be solved with a FAQ entry that
says to upgrade binutils to some version to get more specific error
messages (ie.  Unresolved symbols in the output from ld would not
pretty-print but would show raw mangled form).

As for GDB, I think it does become non-negotiable to upgrade that in
order to get anything but the most rudimentary debugging support.  I
have to ask, though, how many people who are using a bleeding-edge
version of gcj will be unwilling to update to a new version of GDB?  I
would think that it will be some years before non-free OS's (I am
specifically thinking of things like the "companion cd" for Solaris)
will include GCC 4.1 (or whichever version this patch makes its way
into) anyway.

>  > I will note that with my original patch, both tools would print
>  > demangled names but the return type would be listed as the first
>  > parameter.  In addition to being odd, this also has other problems
>  > that Ian pointed out.
>
> I don't think we can live with that!  :-)
That's why there is a "-a" version of the patch which doesn't do that!  :)

TJ

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > >  > I would expect similar behavior from GDB (except that in GDB, the
 > >  > lack of demangling would really complicate things more than the
 > >  > same lack in binutils).  So, my opinion is that GDB is the main
 > >  > problem here, since this patch effectively makes Java debugging
 > >  > very difficult with an unpatched GDB.
 > >
 > > It does.  However, it might be possible to persuade people to ship
 > > updated bnutils on free operating systems.  Unfree systems are going
 > > to be much more problematic, though.

 > I'm not sure we are talking about a hard requirement to update
 > binutils.  The old version works.  There are a few instances that
 > could cause confusion but which could be solved with a FAQ entry
 > that says to upgrade binutils to some version to get more specific
 > error messages (ie.  Unresolved symbols in the output from ld would
 > not pretty-print but would show raw mangled form).

Yeah, I got that.

 > As for GDB, I think it does become non-negotiable to upgrade that
 > in order to get anything but the most rudimentary debugging
 > support.  I have to ask, though, how many people who are using a
 > bleeding-edge version of gcj will be unwilling to update to a new
 > version of GDB?

That's not the case I'm worried about.  I'm worried about making the
change long before an appropriate gdb is released.

 > I would think that it will be some years before non-free OS's (I am
 > specifically thinking of things like the "companion cd" for
 > Solaris) will include GCC 4.1 (or whichever version this patch
 > makes its way into) anyway.

Probably.  But what I don't want to happen is for them to include new
gcc and old gdb.  It's a matter of release cycles.

Andrew.

Re: Patch to fix PR9861

[Andrew Haley]

I tried your patch, and while it correctly demangles, it doesn't allow
me to put a breakpoint on 'f.main(java.lang.String[])' However,
putting a breakpoint on 'void f.main(java.lang.String[])' does work,
as you might expect.

This has the rather nasty effect of breaking tab completion in gdb.
For example, you can't type "break 'f<tab>" and get a nice list of the
methods in f to choose from.

It ought to be possible to fix this in gdb, though.  I'll investigate
some more.

Andrew.

Re: Patch to fix PR9861

[Daniel Jacobowitz]

On Mon, Sep 26, 2005 at 07:04:22PM +0100, Andrew Haley wrote:
> I tried your patch, and while it correctly demangles, it doesn't allow
> me to put a breakpoint on 'f.main(java.lang.String[])' However,
> putting a breakpoint on 'void f.main(java.lang.String[])' does work,
> as you might expect.
> 
> This has the rather nasty effect of breaking tab completion in gdb.
> For example, you can't type "break 'f<tab>" and get a nice list of the
> methods in f to choose from.
> 
> It ought to be possible to fix this in gdb, though.  I'll investigate
> some more.

It may be hard; I tried doing this for C++ templates once and got
nowhere.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: Patch to fix PR9861

[TJ Laurenzo]

On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
> I tried your patch, and while it correctly demangles, it doesn't allow
> me to put a breakpoint on 'f.main(java.lang.String[])' However,
> putting a breakpoint on 'void f.main(java.lang.String[])' does work,
> as you might expect.
>
> This has the rather nasty effect of breaking tab completion in gdb.
> For example, you can't type "break 'f<tab>" and get a nice list of the
> methods in f to choose from.
>
> It ought to be possible to fix this in gdb, though.  I'll investigate
> some more.
>
> Andrew.

That just occurred to me about 10 seconds before your email hit my
mailbox.  There are some C++ cases where this can happen too (function
templates and operators, if I recall).  They are probably corner cases
that GDB doesn't handle specially, though.

What if the return type demangling didn't print the return type first
(the return type could be in parentheses or something after the
argument list or it could end with " returns void")?  That would also
be a way to solve the problem.

TJ

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
 > > I tried your patch, and while it correctly demangles, it doesn't allow
 > > me to put a breakpoint on 'f.main(java.lang.String[])' However,
 > > putting a breakpoint on 'void f.main(java.lang.String[])' does work,
 > > as you might expect.
 > >
 > > This has the rather nasty effect of breaking tab completion in gdb.
 > > For example, you can't type "break 'f<tab>" and get a nice list of the
 > > methods in f to choose from.
 > >
 > > It ought to be possible to fix this in gdb, though.  I'll investigate
 > > some more.
 > 
 > That just occurred to me about 10 seconds before your email hit my
 > mailbox.  There are some C++ cases where this can happen too (function
 > templates and operators, if I recall).  They are probably corner cases
 > that GDB doesn't handle specially, though.
 > 
 > What if the return type demangling didn't print the return type first
 > (the return type could be in parentheses or something after the
 > argument list or it could end with " returns void")?  That would also
 > be a way to solve the problem.

It would.  Nice idea.

Andrew.

Re: Patch to fix PR9861

[TJ Laurenzo]

On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
> TJ Laurenzo writes:
>  > On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
>  > > I tried your patch, and while it correctly demangles, it doesn't allow
>  > > me to put a breakpoint on 'f.main(java.lang.String[])' However,
>  > > putting a breakpoint on 'void f.main(java.lang.String[])' does work,
>  > > as you might expect.
>  > >
>  > > This has the rather nasty effect of breaking tab completion in gdb.
>  > > For example, you can't type "break 'f<tab>" and get a nice list of the
>  > > methods in f to choose from.
>  > >
>  > > It ought to be possible to fix this in gdb, though.  I'll investigate
>  > > some more.
>  >
>  > That just occurred to me about 10 seconds before your email hit my
>  > mailbox.  There are some C++ cases where this can happen too (function
>  > templates and operators, if I recall).  They are probably corner cases
>  > that GDB doesn't handle specially, though.
>  >
>  > What if the return type demangling didn't print the return type first
>  > (the return type could be in parentheses or something after the
>  > argument list or it could end with " returns void")?  That would also
>  > be a way to solve the problem.
>
> It would.  Nice idea.
>
Would you agree with changing the display for Java formatted names to
print it more like a Pascal declaration:
    f.main(String[]):void
I don't think we would need to touch the existing C++ format, even
though it sounds like it could benefit from the same things for C++
templates, et al.

TJ

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
 > > TJ Laurenzo writes:
 > >  > On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
 > >  > > I tried your patch, and while it correctly demangles, it doesn't allow
 > >  > > me to put a breakpoint on 'f.main(java.lang.String[])' However,
 > >  > > putting a breakpoint on 'void f.main(java.lang.String[])' does work,
 > >  > > as you might expect.
 > >  > >
 > >  > > This has the rather nasty effect of breaking tab completion in gdb.
 > >  > > For example, you can't type "break 'f<tab>" and get a nice list of the
 > >  > > methods in f to choose from.
 > >  > >
 > >  > > It ought to be possible to fix this in gdb, though.  I'll investigate
 > >  > > some more.
 > >  >
 > >  > That just occurred to me about 10 seconds before your email hit my
 > >  > mailbox.  There are some C++ cases where this can happen too (function
 > >  > templates and operators, if I recall).  They are probably corner cases
 > >  > that GDB doesn't handle specially, though.
 > >  >
 > >  > What if the return type demangling didn't print the return type first
 > >  > (the return type could be in parentheses or something after the
 > >  > argument list or it could end with " returns void")?  That would also
 > >  > be a way to solve the problem.
 > >
 > > It would.  Nice idea.
 > >
 > Would you agree with changing the display for Java formatted names to
 > print it more like a Pascal declaration:
 >     f.main(String[]):void

At first glance that looks OK.  It would be interesting to see if that
works with gdb.

 > I don't think we would need to touch the existing C++ format, even
 > though it sounds like it could benefit from the same things for C++
 > templates, et al.

Could be.

Andrew.

Re: Patch to fix PR9861

[Andrew Haley]

Andrew Haley writes:
 > TJ Laurenzo writes:
 >  > On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
 >  > > TJ Laurenzo writes:
 >  > >  > On 9/26/05, Andrew Haley <aph@redhat.com> wrote:
 >  > >  > > I tried your patch, and while it correctly demangles, it doesn't allow
 >  > >  > > me to put a breakpoint on 'f.main(java.lang.String[])' However,
 >  > >  > > putting a breakpoint on 'void f.main(java.lang.String[])' does work,
 >  > >  > > as you might expect.
 >  > >  > >
 >  > >  > > This has the rather nasty effect of breaking tab completion in gdb.
 >  > >  > > For example, you can't type "break 'f<tab>" and get a nice list of the
 >  > >  > > methods in f to choose from.
 >  > >  > >
 >  > >  > > It ought to be possible to fix this in gdb, though.  I'll investigate
 >  > >  > > some more.
 >  > >  >
 >  > >  > That just occurred to me about 10 seconds before your email hit my
 >  > >  > mailbox.  There are some C++ cases where this can happen too (function
 >  > >  > templates and operators, if I recall).  They are probably corner cases
 >  > >  > that GDB doesn't handle specially, though.
 >  > >  >
 >  > >  > What if the return type demangling didn't print the return type first
 >  > >  > (the return type could be in parentheses or something after the
 >  > >  > argument list or it could end with " returns void")?  That would also
 >  > >  > be a way to solve the problem.
 >  > >
 >  > > It would.  Nice idea.
 >  > >
 >  > Would you agree with changing the display for Java formatted names to
 >  > print it more like a Pascal declaration:
 >  >     f.main(String[]):void
 > 
 > At first glance that looks OK.  It would be interesting to see if that
 > works with gdb.

How about simply

 f.main(String[])void

That's the nearest to the Java signature form.

Andrew.

Re: Patch to fix PR9861

[Ian Lance Taylor]

Andrew Haley <aph@redhat.com> writes:

> How about simply
> 
>  f.main(String[])void
> 
> That's the nearest to the Java signature form.

I don't know, it's kind of ugly and confusing.

Are there any problems with the a more normal demangling other than
gdb tab-completion?

Ian

Re: Patch to fix PR9861

[Andrew Haley]

Ian Lance Taylor writes:
 > Andrew Haley <aph@redhat.com> writes:
 > 
 > > How about simply
 > > 
 > >  f.main(String[])void
 > > 
 > > That's the nearest to the Java signature form.
 > 
 > I don't know, it's kind of ugly and confusing.

Looks OK to me, but maybe that's a Java thing.  Or maybe I'm weird.

 > Are there any problems with the a more normal demangling other than
 > gdb tab-completion?

Not AFAIK.  Maybe we can just lose the return type altogether.
Although that's perhaps even more confusing.

Andrew.

Re: Patch to fix PR9861

[Bryce McKinlay]

Andrew Haley wrote:

>Ian Lance Taylor writes:
> > Andrew Haley <aph@redhat.com> writes:
> > 
> > > How about simply
> > > 
> > >  f.main(String[])void
> > > 
> > > That's the nearest to the Java signature form.
> > 
> > I don't know, it's kind of ugly and confusing.
>
>Looks OK to me, but maybe that's a Java thing.  Or maybe I'm weird.
>  
>
I'll also cast a vote for this form - it is the simplest and most Java 
like. I don't like the ":".

Bryce

Re: Patch to fix PR9861

[TJ Laurenzo]

[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]

On 9/26/05, Bryce McKinlay <mckinlay@redhat.com> wrote:
> Andrew Haley wrote:
>
> >Ian Lance Taylor writes:
> > > Andrew Haley <aph@redhat.com> writes:
> > >
> > > > How about simply
> > > >
> > > >  f.main(String[])void
> > > >
> > > > That's the nearest to the Java signature form.
> > >
> > > I don't know, it's kind of ugly and confusing.
> >
> >Looks OK to me, but maybe that's a Java thing.  Or maybe I'm weird.
> >
> >
> I'll also cast a vote for this form - it is the simplest and most Java
> like. I don't like the ":".
>
> Bryce

The patch for this is attached (fix-java-demangle.diff).  I added a
flag DMGL_RET_POSTFIX for the demangler.  When this flag is present,
the return type is output after the function signature with a space
separating the two.  This option is enabled by default for the Java
demangler.  I modified the test harness and added test cases for this
as well.  This patch applies against cvs head, so it replaces the
libiberty parts of my previous patch.

I am also attaching a patch to gdb that uses this new flag to put
return types after the function signature for normal C++ symbols
stored in the lookup table too.  This helps with CNI debugging and as
a consequence, works around the issue that was pointed out earlier
with selecting template functions.  I'm not terribly familiar with gdb
internals, so I am not certain if this second patch causes any other
problems or whether the maintainers want to change the behavior for
template functions.

If this approach is acceptable, I will role this up into an overall
patch and resubmit.

TJ

[-- Attachment #2: fix-java-demangle.diff --]
[-- Type: application/octet-stream, Size: 8146 bytes --]

Index: libiberty/cp-demangle.c
===================================================================
RCS file: /cvsroot/gcc/gcc/libiberty/cp-demangle.c,v
retrieving revision 1.83
diff -c -3 -p -r1.83 cp-demangle.c
*** libiberty/cp-demangle.c	1 Jul 2005 16:39:35 -0000	1.83
--- libiberty/cp-demangle.c	26 Sep 2005 23:17:03 -0000
*************** d_function_type (struct d_info *di)
*** 1939,1945 ****
    return ret;
  }
  
! /* <bare-function-type> ::= <type>+  */
  
  static struct demangle_component *
  d_bare_function_type (struct d_info *di, int has_return_type)
--- 1939,1945 ----
    return ret;
  }
  
! /* <bare-function-type> ::= [J]<type>+  */
  
  static struct demangle_component *
  d_bare_function_type (struct d_info *di, int has_return_type)
*************** d_bare_function_type (struct d_info *di,
*** 1947,1959 ****
    struct demangle_component *return_type;
    struct demangle_component *tl;
    struct demangle_component **ptl;
  
    return_type = NULL;
    tl = NULL;
    ptl = &tl;
    while (1)
      {
-       char peek;
        struct demangle_component *type;
  
        peek = d_peek_char (di);
--- 1947,1968 ----
    struct demangle_component *return_type;
    struct demangle_component *tl;
    struct demangle_component **ptl;
+   char peek;
+ 
+   /* Detect special qualifier indicating that the first argument
+      is the return type.  */
+   peek = d_peek_char (di);
+   if (peek == 'J')
+     {
+       d_advance (di, 1);
+       has_return_type = 1;
+     }
  
    return_type = NULL;
    tl = NULL;
    ptl = &tl;
    while (1)
      {
        struct demangle_component *type;
  
        peek = d_peek_char (di);
*************** d_print_comp (struct d_print_info *dpi,
*** 3025,3037 ****
  
      case DEMANGLE_COMPONENT_FUNCTION_TYPE:
        {
  	if (d_left (dc) != NULL)
  	  {
  	    struct d_print_mod dpm;
  
  	    /* We must pass this type down as a modifier in order to
  	       print it in the right location.  */
! 
  	    dpm.next = dpi->modifiers;
  	    dpi->modifiers = &dpm;
  	    dpm.mod = dc;
--- 3034,3053 ----
  
      case DEMANGLE_COMPONENT_FUNCTION_TYPE:
        {
+ 	if ((dpi->options & DMGL_RET_POSTFIX) != 0)
+ 	  d_print_function_type (dpi, dc, dpi->modifiers);
+ 
+ 	/* Print return type if present */
  	if (d_left (dc) != NULL)
  	  {
  	    struct d_print_mod dpm;
  
  	    /* We must pass this type down as a modifier in order to
  	       print it in the right location.  */
! 	    
! 	    if ((dpi->options & DMGL_RET_POSTFIX) != 0)
! 	      d_append_char (dpi, ' ');
! 	    
  	    dpm.next = dpi->modifiers;
  	    dpi->modifiers = &dpm;
  	    dpm.mod = dc;
*************** d_print_comp (struct d_print_info *dpi,
*** 3045,3054 ****
  	    if (dpm.printed)
  	      return;
  
! 	    d_append_char (dpi, ' ');
  	  }
  
! 	d_print_function_type (dpi, dc, dpi->modifiers);
  
  	return;
        }
--- 3061,3072 ----
  	    if (dpm.printed)
  	      return;
  
! 	    if ((dpi->options & DMGL_RET_POSTFIX) == 0)
! 	      d_append_char (dpi, ' ');
  	  }
  
! 	if ((dpi->options & DMGL_RET_POSTFIX) == 0) 
! 	  d_print_function_type (dpi, dc, dpi->modifiers);
  
  	return;
        }
*************** java_demangle_v3 (const char* mangled)
*** 4003,4009 ****
    char *from;
    char *to;
  
!   demangled = d_demangle (mangled, DMGL_JAVA | DMGL_PARAMS, &alc);
  
    if (demangled == NULL)
      return NULL;
--- 4021,4028 ----
    char *from;
    char *to;
  
!   demangled = d_demangle (mangled, DMGL_JAVA | DMGL_PARAMS | DMGL_RET_POSTFIX, 
! 			  &alc);
  
    if (demangled == NULL)
      return NULL;
Index: include/demangle.h
===================================================================
RCS file: /cvsroot/gcc/gcc/include/demangle.h,v
retrieving revision 1.29
diff -c -3 -p -r1.29 demangle.h
*** include/demangle.h	25 May 2005 23:29:53 -0000	1.29
--- include/demangle.h	26 Sep 2005 23:17:03 -0000
*************** extern "C" {
*** 35,40 ****
--- 35,42 ----
  #define DMGL_JAVA	 (1 << 2)	/* Demangle as Java rather than C++. */
  #define DMGL_VERBOSE	 (1 << 3)	/* Include implementation details.  */
  #define DMGL_TYPES	 (1 << 4)	/* Also try to demangle type encodings.  */
+ #define DMGL_RET_POSTFIX (1 << 5)       /* Print function return types (when
+                                            present) after function signature */
  
  #define DMGL_AUTO	 (1 << 8)
  #define DMGL_GNU	 (1 << 9)
Index: libiberty/testsuite/demangle-expected
===================================================================
RCS file: /cvsroot/gcc/gcc/libiberty/testsuite/demangle-expected,v
retrieving revision 1.33
diff -c -3 -p -r1.33 demangle-expected
*** libiberty/testsuite/demangle-expected	1 Jul 2005 16:39:36 -0000	1.33
--- libiberty/testsuite/demangle-expected	26 Sep 2005 23:17:04 -0000
***************
*** 11,16 ****
--- 11,17 ----
  #    --is-v3-ctor        Calls is_gnu_v3_mangled_ctor on input; expected
  #                        output is an integer representing ctor_kind.
  #    --is-v3-dtor        Likewise, but for dtors.
+ #    --ret-postfix       Passes the DMGL_RET_POSTFIX option
  #
  #  For compatibility, just in case it matters, the options line may be
  #  empty, to mean --format=auto.  If it doesn't start with --, then it
*************** _test_array__L_1__B23b___clean.6
*** 3781,3783 ****
--- 3782,3807 ----
  --format=java
  _ZGAN4java4lang5Class7forNameEPNS0_6StringE
  hidden alias for java.lang.Class.forName(java.lang.String)
+ #
+ # Test cases to verify encoding that determines if a return type is present
+ # Related to PR9861
+ --format=java
+ _ZN4java4lang4Math4acosEJdd
+ java.lang.Math.acos(double) double
+ #
+ --format=auto
+ _ZN4java4lang4Math4acosEJdd
+ double java::lang::Math::acos(double)
+ #
+ --format=auto
+ _ZN4java4lang4Math4acosEJvd
+ void java::lang::Math::acos(double)
+ #
+ --format=auto --ret-postfix
+ _ZN4java4lang4Math4acosEJdd
+ java::lang::Math::acos(double) double
+ #
+ --format=gnu-v3 --no-params --ret-postfix
+ _Z4makeI7FactoryiET_IT0_Ev
+ make<Factory, int>() Factory<int>
+ make<Factory, int>
Index: libiberty/testsuite/test-demangle.c
===================================================================
RCS file: /cvsroot/gcc/gcc/libiberty/testsuite/test-demangle.c,v
retrieving revision 1.6
diff -c -3 -p -r1.6 test-demangle.c
*** libiberty/testsuite/test-demangle.c	17 Aug 2005 03:31:04 -0000	1.6
--- libiberty/testsuite/test-demangle.c	26 Sep 2005 23:17:04 -0000
*************** exp: %s\n",
*** 114,119 ****
--- 114,120 ----
       --is-v3-ctor        Calls is_gnu_v3_mangled_ctor on input; expected
                           output is an integer representing ctor_kind.
       --is-v3-dtor        Likewise, but for dtors.
+      --ret-postfix       Passes the DMGL_RET_POSTFIX option
  
     For compatibility, just in case it matters, the options line may be
     empty, to mean --format=auto.  If it doesn't start with --, then it
*************** main(argc, argv)
*** 129,134 ****
--- 130,136 ----
    int no_params;
    int is_v3_ctor;
    int is_v3_dtor;
+   int ret_postfix;
    struct line format;
    struct line input;
    struct line expect;
*************** main(argc, argv)
*** 158,163 ****
--- 160,166 ----
        tests++;
  
        no_params = 0;
+       ret_postfix = 0;
        is_v3_ctor = 0;
        is_v3_dtor = 0;
        if (format.data[0] == '\0')
*************** main(argc, argv)
*** 212,217 ****
--- 215,222 ----
  		is_v3_ctor = 1;
  	      else if (strcmp (opt, "--is-v3-dtor") == 0)
  		is_v3_dtor = 1;
+ 	      else if (strcmp (opt, "--ret-postfix") == 0)
+ 		ret_postfix = 1;
  	      else
  		{
  		  printf ("FAIL at line %d: unrecognized option %s\n",
*************** main(argc, argv)
*** 255,261 ****
        cplus_demangle_set_style (style);
  
        result = cplus_demangle (input.data,
! 			       DMGL_PARAMS|DMGL_ANSI|DMGL_TYPES);
  
        if (result
  	  ? strcmp (result, expect.data)
--- 260,267 ----
        cplus_demangle_set_style (style);
  
        result = cplus_demangle (input.data,
! 			       DMGL_PARAMS|DMGL_ANSI|DMGL_TYPES|
! 			       (ret_postfix ? DMGL_RET_POSTFIX : 0));
  
        if (result
  	  ? strcmp (result, expect.data)

[-- Attachment #3: gdb.diff --]
[-- Type: application/octet-stream, Size: 1328 bytes --]

Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.145
diff -c -3 -p -r1.145 symtab.c
*** gdb/symtab.c	8 Mar 2005 04:34:44 -0000	1.145
--- gdb/symtab.c	26 Sep 2005 23:21:14 -0000
*************** symbol_find_demangled_name (struct gener
*** 466,472 ****
        || gsymbol->language == language_auto)
      {
        demangled =
!         cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI);
        if (demangled != NULL)
  	{
  	  gsymbol->language = language_cplus;
--- 466,472 ----
        || gsymbol->language == language_auto)
      {
        demangled =
!         cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI | DMGL_RET_POSTFIX);
        if (demangled != NULL)
  	{
  	  gsymbol->language = language_cplus;
*************** symbol_find_demangled_name (struct gener
*** 477,483 ****
      {
        demangled =
          cplus_demangle (mangled,
!                         DMGL_PARAMS | DMGL_ANSI | DMGL_JAVA);
        if (demangled != NULL)
  	{
  	  gsymbol->language = language_java;
--- 477,483 ----
      {
        demangled =
          cplus_demangle (mangled,
!                         DMGL_PARAMS | DMGL_ANSI | DMGL_JAVA | DMGL_RET_POSTFIX);
        if (demangled != NULL)
  	{
  	  gsymbol->language = language_java;

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > On 9/26/05, Bryce McKinlay <mckinlay@redhat.com> wrote:
 > > Andrew Haley wrote:
 > >
 > > >Ian Lance Taylor writes:
 > > > > Andrew Haley <aph@redhat.com> writes:
 > > > >
 > > > > > How about simply
 > > > > >
 > > > > >  f.main(String[])void
 > > > > >
 > > > > > That's the nearest to the Java signature form.
 > > > >
 > > > > I don't know, it's kind of ugly and confusing.
 > > >
 > > >Looks OK to me, but maybe that's a Java thing.  Or maybe I'm weird.
 > > >
 > > >
 > > I'll also cast a vote for this form - it is the simplest and most Java
 > > like. I don't like the ":".
 > 
 > The patch for this is attached (fix-java-demangle.diff).  I added a
 > flag DMGL_RET_POSTFIX for the demangler.  When this flag is present,
 > the return type is output after the function signature with a space
 > separating the two.  This option is enabled by default for the Java
 > demangler.  I modified the test harness and added test cases for this
 > as well.  This patch applies against cvs head, so it replaces the
 > libiberty parts of my previous patch.
 > 
 > I am also attaching a patch to gdb that uses this new flag to put
 > return types after the function signature for normal C++ symbols
 > stored in the lookup table too.

 > This helps with CNI debugging and as a consequence, works around
 > the issue that was pointed out earlier with selecting template
 > functions.  I'm not terribly familiar with gdb internals, so I am
 > not certain if this second patch causes any other problems or
 > whether the maintainers want to change the behavior for template
 > functions.

OK.  We'll see.

I'm not keen on the space, but it's not important enough to reject
your patch.

Andrew.

Re: Patch to fix PR9861

[Andrew Haley]

Andrew Haley writes:
 > TJ Laurenzo writes:
 >  > On 9/26/05, Bryce McKinlay <mckinlay@redhat.com> wrote:
 >  > > Andrew Haley wrote:
 >  > >
 >  > > >Ian Lance Taylor writes:
 >  > > > > Andrew Haley <aph@redhat.com> writes:
 >  > > > >
 >  > > > > > How about simply
 >  > > > > >
 >  > > > > >  f.main(String[])void
 >  > > > > >
 >  > > > > > That's the nearest to the Java signature form.
 >  > > > >
 >  > > > > I don't know, it's kind of ugly and confusing.
 >  > > >
 >  > > >Looks OK to me, but maybe that's a Java thing.  Or maybe I'm weird.
 >  > > >
 >  > > >
 >  > > I'll also cast a vote for this form - it is the simplest and most Java
 >  > > like. I don't like the ":".
 >  > 
 >  > The patch for this is attached (fix-java-demangle.diff).  I added a
 >  > flag DMGL_RET_POSTFIX for the demangler.  When this flag is present,
 >  > the return type is output after the function signature with a space
 >  > separating the two.  This option is enabled by default for the Java
 >  > demangler.  I modified the test harness and added test cases for this
 >  > as well.  This patch applies against cvs head, so it replaces the
 >  > libiberty parts of my previous patch.
 >  > 
 >  > I am also attaching a patch to gdb that uses this new flag to put
 >  > return types after the function signature for normal C++ symbols
 >  > stored in the lookup table too.
 > 
 >  > This helps with CNI debugging and as a consequence, works around
 >  > the issue that was pointed out earlier with selecting template
 >  > functions.  I'm not terribly familiar with gdb internals, so I am
 >  > not certain if this second patch causes any other problems or
 >  > whether the maintainers want to change the behavior for template
 >  > functions.
 > 
 > OK.  We'll see.
 > 
 > I'm not keen on the space, but it's not important enough to reject
 > your patch.

I changed my mind because the space makes assembly expressions look
really odd.  Like this:

-----------------------------------------------------------------------
.globl Test.x() java.lang.Object
        .type   Test.x() java.lang.Object, @function
Test.x() java.lang.Object:
.LFB3:
.LBB3:
        movl    $0, %eax
.LBE3:
        ret
.LFE3:
        .size   Test.x() java.lang.Object, .-Test.x() java.lang.Object
-----------------------------------------------------------------------

If you remove the space, it's much clearer that
"Test.x()java.lang.Object" is a single symbol:

-----------------------------------------------------------------------
.globl Test.x()java.lang.Object
        .type   Test.x()java.lang.Object, @function
Test.x()java.lang.Object:
.LFB3:
.LBB3:
        movl    $0, %eax
.LBE3:
        ret
.LFE3:
        .size   Test.x()java.lang.Object, .-Test.x()java.lang.Object
-----------------------------------------------------------------------

Also, constructors have no encoded return type.  I guess that's OK,
but I was rather surprised.

Andrew.

Re: Patch to fix PR9861

[TJ Laurenzo]

On 9/27/05, Andrew Haley <aph@redhat.com> wrote:
> Andrew Haley writes:
>  > TJ Laurenzo writes:
>  >  > On 9/26/05, Bryce McKinlay <mckinlay@redhat.com> wrote:
>  >  > > Andrew Haley wrote:
>  >  > >
>  >  > > >Ian Lance Taylor writes:
>  >  > > > > Andrew Haley <aph@redhat.com> writes:
>  >  > > > >
>  >  > > > > > How about simply
>  >  > > > > >
>  >  > > > > >  f.main(String[])void
>  >  > > > > >
>  >  > > > > > That's the nearest to the Java signature form.
>  >  > > > >
>  >  > > > > I don't know, it's kind of ugly and confusing.
>  >  > > >
>  >  > > >Looks OK to me, but maybe that's a Java thing.  Or maybe I'm weird.
>  >  > > >
>  >  > > >
>  >  > > I'll also cast a vote for this form - it is the simplest and most Java
>  >  > > like. I don't like the ":".
>  >  >
>  >  > The patch for this is attached (fix-java-demangle.diff).  I added a
>  >  > flag DMGL_RET_POSTFIX for the demangler.  When this flag is present,
>  >  > the return type is output after the function signature with a space
>  >  > separating the two.  This option is enabled by default for the Java
>  >  > demangler.  I modified the test harness and added test cases for this
>  >  > as well.  This patch applies against cvs head, so it replaces the
>  >  > libiberty parts of my previous patch.
>  >  >
>  >  > I am also attaching a patch to gdb that uses this new flag to put
>  >  > return types after the function signature for normal C++ symbols
>  >  > stored in the lookup table too.
>  >
>  >  > This helps with CNI debugging and as a consequence, works around
>  >  > the issue that was pointed out earlier with selecting template
>  >  > functions.  I'm not terribly familiar with gdb internals, so I am
>  >  > not certain if this second patch causes any other problems or
>  >  > whether the maintainers want to change the behavior for template
>  >  > functions.
>  >
>  > OK.  We'll see.
>  >
>  > I'm not keen on the space, but it's not important enough to reject
>  > your patch.
>
> I changed my mind because the space makes assembly expressions look
> really odd.  Like this:
>
> -----------------------------------------------------------------------
> .globl Test.x() java.lang.Object
>         .type   Test.x() java.lang.Object, @function
> Test.x() java.lang.Object:
> .LFB3:
> .LBB3:
>         movl    $0, %eax
> .LBE3:
>         ret
> .LFE3:
>         .size   Test.x() java.lang.Object, .-Test.x() java.lang.Object
> -----------------------------------------------------------------------
>
> If you remove the space, it's much clearer that
> "Test.x()java.lang.Object" is a single symbol:
>
> -----------------------------------------------------------------------
> .globl Test.x()java.lang.Object
>         .type   Test.x()java.lang.Object, @function
> Test.x()java.lang.Object:
> .LFB3:
> .LBB3:
>         movl    $0, %eax
> .LBE3:
>         ret
> .LFE3:
>         .size   Test.x()java.lang.Object, .-Test.x()java.lang.Object
> -----------------------------------------------------------------------
>
> Also, constructors have no encoded return type.  I guess that's OK,
> but I was rather surprised.

Ok.  The space goes.  A single observation on this point is that
methods with more than one argument have a space between the comma and
beginning of the next argument.  With that said, in light of the the
common case of a method with <=1 arguments I can see how it makes
sense for the demangled name to not include a space.

The decision to exclude return types on constructors was based on a
list of C++  rules for excluding return types for members of template
classes.  It seemed to make since, so I did the same thing with Java. 
The return type for constructors shows up as "void" in the tree, so
including it would put a "v" at the end of every demangled name and
add a "Jv" to each mangled form.  I'm inclined to keep the exclusion
because it sticks with the existing rule that in C++, constructors
never have an encoded return type.

TJ

Re: Patch to fix PR9861

[Tom Tromey]

>>>>> "TJ" == TJ Laurenzo <tlaurenzo@gmail.com> writes:

>> Also, constructors have no encoded return type.  I guess that's OK,
>> but I was rather surprised.

TJ> The decision to exclude return types on constructors was based on a
TJ> list of C++  rules for excluding return types for members of template
TJ> classes.

It is weird, but valid, to have a method with the class' name in java.
Would we get a symbol clash for a class like this?

    public class Weird {
      public Weird (int x) { /* constructor */ }
      public int Weird() { return 5; /* method */ }
    }

Tom

Re: Patch to fix PR9861

[TJ Laurenzo]

On 27 Sep 2005 10:18:05 -0600, Tom Tromey <tromey@redhat.com> wrote:
> >>>>> "TJ" == TJ Laurenzo <tlaurenzo@gmail.com> writes:
>
> >> Also, constructors have no encoded return type.  I guess that's OK,
> >> but I was rather surprised.
>
> TJ> The decision to exclude return types on constructors was based on a
> TJ> list of C++  rules for excluding return types for members of template
> TJ> classes.
>
> It is weird, but valid, to have a method with the class' name in java.
> Would we get a symbol clash for a class like this?
>
>     public class Weird {
>       public Weird (int x) { /* constructor */ }
>       public int Weird() { return 5; /* method */ }
>     }
>
> Tom
>

No.  The mangled forms would be as follows:
 public Weird(int x)  :  _ZN5WeirdC1Ei
 public int Weird()  :  _ZN5Weird5WeirdEJiv

 The actual name of the constructor is not included in the mangled
form and the extra "J" qualifier further distinguishes it from a
regular method.

 You learn something new every day.  I had never even considered that
you could have methods with the same name as the class.  Guess I slept
through that part of the JLS.

 TJ

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > 
 >  You learn something new every day.  I had never even considered that
 > you could have methods with the same name as the class.  Guess I slept
 > through that part of the JLS.

Me too.  This sort of weirdness is why I'm being so careful with this
patch.

Andrew.

Re: Patch to fix PR9861

[Andrew Haley]

Tom Tromey writes:
 > >>>>> "TJ" == TJ Laurenzo <tlaurenzo@gmail.com> writes:
 > 
 > >> Also, constructors have no encoded return type.  I guess that's OK,
 > >> but I was rather surprised.
 > 
 > TJ> The decision to exclude return types on constructors was based on a
 > TJ> list of C++  rules for excluding return types for members of template
 > TJ> classes.
 > 
 > It is weird, but valid, to have a method with the class' name in java.
 > Would we get a symbol clash for a class like this?
 > 
 >     public class Weird {
 >       public Weird (int x) { /* constructor */ }
 >       public int Weird() { return 5; /* method */ }
 >     }

No.  A constructor that has no return type (not even void) doesn't
clash with any method name.

The constructor is

  Weird.Weird(int)  ->  _ZN5WeirdC1Ei

The method is

  Weird.Weird()int  ->  _ZN5Weird5WeirdEJiv

Andrew.

Re: Patch to fix PR9861

[Ian Lance Taylor]

Andrew Haley <aph@redhat.com> writes:

> I changed my mind because the space makes assembly expressions look
> really odd.  Like this:
> 
> -----------------------------------------------------------------------
> .globl Test.x() java.lang.Object
>         .type   Test.x() java.lang.Object, @function
> Test.x() java.lang.Object:
> .LFB3:
> .LBB3:
>         movl    $0, %eax
> .LBE3:
>         ret
> .LFE3:
>         .size   Test.x() java.lang.Object, .-Test.x() java.lang.Object
> -----------------------------------------------------------------------

Wait, what's going on here?  The assembler should only see the mangled
name.  The assembler is not going to like those parentheses, much less
the space.

Ian

Re: Patch to fix PR9861

[Andrew Haley]

Ian Lance Taylor writes:
 > Andrew Haley <aph@redhat.com> writes:
 > 
 > > I changed my mind because the space makes assembly expressions look
 > > really odd.  Like this:
 > > 
 > > -----------------------------------------------------------------------
 > > .globl Test.x() java.lang.Object
 > >         .type   Test.x() java.lang.Object, @function
 > > Test.x() java.lang.Object:
 > > .LFB3:
 > > .LBB3:
 > >         movl    $0, %eax
 > > .LBE3:
 > >         ret
 > > .LFE3:
 > >         .size   Test.x() java.lang.Object, .-Test.x() java.lang.Object
 > > -----------------------------------------------------------------------
 > 
 > Wait, what's going on here?  The assembler should only see the mangled
 > name.  The assembler is not going to like those parentheses, much less
 > the space.

This is the ouput of c++filt.  It's for me to read, not the assembler.

Andrew.

Re: Patch to fix PR9861

[TJ Laurenzo]

>  > I would think that it will be some years before non-free OS's (I am
>  > specifically thinking of things like the "companion cd" for
>  > Solaris) will include GCC 4.1 (or whichever version this patch
>  > makes its way into) anyway.
>
> Probably.  But what I don't want to happen is for them to include new
> gcc and old gdb.  It's a matter of release cycles.

Is it acceptable to get the changes to cp_demangle.c into mainline
prior to the rest?  It might make release planning easier as new
versions of binutils and gdb would already have latent support for
this fix by the time it is released in GCC.  Just a thought.

TJ

Re: Patch to fix PR9861

[Andrew Haley]

TJ Laurenzo writes:
 > >  > I would think that it will be some years before non-free OS's (I am
 > >  > specifically thinking of things like the "companion cd" for
 > >  > Solaris) will include GCC 4.1 (or whichever version this patch
 > >  > makes its way into) anyway.
 > >
 > > Probably.  But what I don't want to happen is for them to include new
 > > gcc and old gdb.  It's a matter of release cycles.
 > 
 > Is it acceptable to get the changes to cp_demangle.c into mainline
 > prior to the rest?

Yes.  If Ian is happy with the new mangling scheme we should commit
this straight away.

Andrew.

Re: Patch to fix PR9861

[Ian Lance Taylor]

Andrew Haley <aph@redhat.com> writes:

> TJ Laurenzo writes:
>  > >  > I would think that it will be some years before non-free OS's (I am
>  > >  > specifically thinking of things like the "companion cd" for
>  > >  > Solaris) will include GCC 4.1 (or whichever version this patch
>  > >  > makes its way into) anyway.
>  > >
>  > > Probably.  But what I don't want to happen is for them to include new
>  > > gcc and old gdb.  It's a matter of release cycles.
>  > 
>  > Is it acceptable to get the changes to cp_demangle.c into mainline
>  > prior to the rest?
> 
> Yes.  If Ian is happy with the new mangling scheme we should commit
> this straight away.

I'm fine with it.  You might want to double check with the C++ folks
to make sure they are fine with it--they probably don't care, but I
think it's worth asking.

Ian

Re: Patch to fix PR9861

[Andrew Haley]

Ian Lance Taylor writes:
 > Andrew Haley <aph@redhat.com> writes:
 > 
 > > TJ Laurenzo writes:
 > >  > >  > I would think that it will be some years before non-free OS's (I am
 > >  > >  > specifically thinking of things like the "companion cd" for
 > >  > >  > Solaris) will include GCC 4.1 (or whichever version this patch
 > >  > >  > makes its way into) anyway.
 > >  > >
 > >  > > Probably.  But what I don't want to happen is for them to include new
 > >  > > gcc and old gdb.  It's a matter of release cycles.
 > >  > 
 > >  > Is it acceptable to get the changes to cp_demangle.c into mainline
 > >  > prior to the rest?
 > > 
 > > Yes.  If Ian is happy with the new mangling scheme we should commit
 > > this straight away.
 > 
 > I'm fine with it.  You might want to double check with the C++ folks
 > to make sure they are fine with it--they probably don't care, but I
 > think it's worth asking.

OK.  Well, there's still some question about the exact form that the
demangling will take, but this is progress.

Andrew.

Re: Patch to fix PR9861

[David Daney]

TJ Laurenzo wrote:
>> > I would expect similar behavior from GDB (except that in GDB, the
>> > lack of demangling would really complicate things more than the
>> > same lack in binutils).  So, my opinion is that GDB is the main
>> > problem here, since this patch effectively makes Java debugging
>> > very difficult with an unpatched GDB.
>>
>>It does.  However, it might be possible to persuade people to ship
>>updated bnutils on free operating systems.  Unfree systems are going
>>to be much more problematic, though.
> 
> I'm not sure we are talking about a hard requirement to update
> binutils.  The old version works.  There are a few instances that
> could cause confusion but which could be solved with a FAQ entry that
> says to upgrade binutils to some version to get more specific error
> messages (ie.  Unresolved symbols in the output from ld would not
> pretty-print but would show raw mangled form).
> 
> As for GDB, I think it does become non-negotiable to upgrade that in
> order to get anything but the most rudimentary debugging support.  I
> have to ask, though, how many people who are using a bleeding-edge
> version of gcj will be unwilling to update to a new version of GDB?

I am not a gdb version expert, but I seem to recall reading somewhere 
that the gdb that ships with FC[34] has some RedHat local patches to 
allow it to work better with their patched kernel/glibc.  I may be being 
a worry wart, but I am concerned that since I use FC3 that I will not be 
able to use gdb's CVS HEAD.

Not that that should be a reason for not applying the patch.  If we need 
to change the mangling to be able to implement according to the JLS then 
sooner may be better than later.

David Daney

Re: Patch to fix PR9861

[Ranjit Mathew]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Lance Taylor wrote:
> Ian Lance Taylor <ian@airs.com> writes:
> 
> 
>>TJ Laurenzo <tlaurenzo@gmail.com> writes:
>>
>>
>>>The attached patch corrects PR9861 by including the return type in
>>>mangled names in the C++ and Java compilers for Java class methods. 
>>>This changes the ABI for Java types but does not make any changes to
>>>C++ programs that do not access Java types.
>>
>>How is the demangler going to work with this patch?  Is there any way
>>for the demangler to determine which names are Java names and which
>>are C++ names?  I don't see any obvious way, though I haven't looked
>>that hard.
> 
> 
> By the way, considering the demangler is not an unimportant point as
> any ambiguity in demangling is an ambiguity that can be happen when
> linking C++ and Java code together.  For example, your proposed
> mangling for
>     java.lang.Math.acos(double)
> appears to be
>     _ZN4java4lang4Math4acosEdd
> which is the mangled name which will be used for the C++ function
>     java::lang::Math::acos(double, double)

If we are changing the mangling for "Java" classes, is it possible
to have a mangling where the return type comes *before* the
class-cum-method name to disambiguate a (say) "void foo::bar(int,int)"
from an "int foo::bar(int)" when the return type is included?

Right now, without this patch, these seem to be encoded as
"ZN3foo3barEii" and "ZN3foo3barEi" respectively.

Thanks,
Ranjit.

- --
Ranjit Mathew       Email: rmathew AT gmail DOT com

Bangalore, INDIA.     Web: http://ranjitmathew.hostingzero.com/




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDM10IYb1hx2wRS48RAgOBAJ9gJIACog8fsSZax84eEJqjpPNV3gCfcdMt
KOBMo/YAbKwv3IDf4FKs9ZE=
=cHTC
-----END PGP SIGNATURE-----

Re: Patch to fix PR9861

[TJ Laurenzo]

I have implemented Ian's suggested encoding.  This disambiguates the
forms by including a 'J' qualifier for java methods when the return
type is included.  I'll get the patch out tomorrow.  I'm getting a
build failure that has something to do with constructors and CNI and
haven't looked into it yet.

TJ

On 9/22/05, Ranjit Mathew <rmathew@gmail.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ian Lance Taylor wrote:
> > Ian Lance Taylor <ian@airs.com> writes:
> >
> >
> >>TJ Laurenzo <tlaurenzo@gmail.com> writes:
> >>
> >>
> >>>The attached patch corrects PR9861 by including the return type in
> >>>mangled names in the C++ and Java compilers for Java class methods.
> >>>This changes the ABI for Java types but does not make any changes to
> >>>C++ programs that do not access Java types.
> >>
> >>How is the demangler going to work with this patch?  Is there any way
> >>for the demangler to determine which names are Java names and which
> >>are C++ names?  I don't see any obvious way, though I haven't looked
> >>that hard.
> >
> >
> > By the way, considering the demangler is not an unimportant point as
> > any ambiguity in demangling is an ambiguity that can be happen when
> > linking C++ and Java code together.  For example, your proposed
> > mangling for
> >     java.lang.Math.acos(double)
> > appears to be
> >     _ZN4java4lang4Math4acosEdd
> > which is the mangled name which will be used for the C++ function
> >     java::lang::Math::acos(double, double)
>
> If we are changing the mangling for "Java" classes, is it possible
> to have a mangling where the return type comes *before* the
> class-cum-method name to disambiguate a (say) "void foo::bar(int,int)"
> from an "int foo::bar(int)" when the return type is included?
>
> Right now, without this patch, these seem to be encoded as
> "ZN3foo3barEii" and "ZN3foo3barEi" respectively.
>
> Thanks,
> Ranjit.
>
> - --
> Ranjit Mathew       Email: rmathew AT gmail DOT com
>
> Bangalore, INDIA.     Web: http://ranjitmathew.hostingzero.com/
>
>
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.2 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
>
> iD8DBQFDM10IYb1hx2wRS48RAgOBAJ9gJIACog8fsSZax84eEJqjpPNV3gCfcdMt
> KOBMo/YAbKwv3IDf4FKs9ZE=
> =cHTC
> -----END PGP SIGNATURE-----
>

Re: Patch to fix PR9861

[TJ Laurenzo]

The demangler currently handles this by printing the return type as
the first argument of the method.  I recognize that this is not ideal,
but it is a lot better than not seeing any decoded form at all.  I
think Tom Tromey mentioned the option of doing something like what you
mentioned to work around the ambiguity.

What files need to be touched to fix the demangler to support this? 
Would I be right in assuming that if we made a syntactic change like
this and fixed the demangler to support it, binutils and gdb would be
unable to decode the new form until the patches made it over to those
projects?

I'll fix the format of the code...  Thanks for the tip.

TJ

On 22 Sep 2005 14:28:08 -0700, Ian Lance Taylor <ian@airs.com> wrote:
> TJ Laurenzo <tlaurenzo@gmail.com> writes:
>
> > The attached patch corrects PR9861 by including the return type in
> > mangled names in the C++ and Java compilers for Java class methods.
> > This changes the ABI for Java types but does not make any changes to
> > C++ programs that do not access Java types.
>
> How is the demangler going to work with this patch?  Is there any way
> for the demangler to determine which names are Java names and which
> are C++ names?  I don't see any obvious way, though I haven't looked
> that hard.
>
> If there is no way at present, then I recommend adding a 'J' at the
> start of bare-function-type if the return type is encoded.  That is,
> do this:
>     <bare-function-type> ::= [J] <type>+
> That should be unambiguous.
>
> (I note in passing that your patch is not formatted correctly
> according to the GNU standards.  The correct formatting would be
> something like:
>   mangle_ret_type = (!DECL_CONSTRUCTOR_P (decl)
>                      && !DECL_DESTRUCTOR_P (decl)
>                      && !DECL_CONV_FN_P (decl)
>                      && (decl_is_template_id (decl, NULL)
>                          || (DECL_FUNCTION_MEMBER_P (decl)
>                              && TYPE_FOR_JAVA (DECL_CONTEXT (decl)))));
> Thanks.)
>
> Ian
>

Re: Patch to fix PR9861

[Ian Lance Taylor]

TJ Laurenzo <tlaurenzo@gmail.com> writes:

> What files need to be touched to fix the demangler to support this? 

libiberty/cp-demangle.c.  For my suggestion, you would need to edit
d_bare_function_type like this:

    peek = d_peek_char (di);
    if (peek == 'J')
      {
        d_advance (di, 1);
        has_return_type = 1;
      }

> Would I be right in assuming that if we made a syntactic change like
> this and fixed the demangler to support it, binutils and gdb would be
> unable to decode the new form until the patches made it over to those
> projects?

Yes.  The file libiberty/cp-demangle.c is shared by all the projects,
so the source code patch would go over right away, but older
binutils/gdb releases would of course not work with the new mangled
names.

Ian

Re: Patch to fix PR9861

[TJ Laurenzo]

> Yes.  The file libiberty/cp-demangle.c is shared by all the projects,
> so the source code patch would go over right away, but older
> binutils/gdb releases would of course not work with the new mangled
> names.
I can live with that.  Barring objections, I will make these changes,
test and resubmit the patch.

TJ