Patch: FYI: CNI -vs- bridge methods

Patch: FYI: CNI -vs- bridge methods

[Tom Tromey]

I'm checking this in on the trunk and in to Classpath.

Jakub, I did not put this on the RH 4.1 branch yet.  While it does not
affect binary compatibility, it might affect source compatibility if
there is a program that uses CNI and tries to call a "bridge target
method".  I don't know of any such program but I thought perhaps I was
forgetting something.  Are there CNI users other than frysk?  (I
looked at frysk and did not see any suspect calls.)

The bug here is that with covariant returns we can end up with
multiple bridge methods, with a single "ultimate" target method, in a
single class.  So our old naive approach of renaming these to
"target$method" will not work.

Our fix is to use the name "DeclaringClass$method" instead.  This
works well enough for all the cases in libgcj.  It is possible that a
future addition to Java could break this.  Note that our whole
approach here is suspect though... if the compiler ever gets smarter,
it could, perhaps, devirtualize such a call and then we would fail to
link.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>

	* Regenerated headers with new gjavah.

Index: classpath/ChangeLog
from  Tom Tromey  <tromey@redhat.com>

	* tools/gnu/classpath/tools/javah/MethodHelper.java (print):
	Changed arguments.  Directly print method name.
	* tools/gnu/classpath/tools/javah/ClassWrapper.java
	(methodNameMap): New field.
	(makeVtable): Initialize it.
	(printMethods): Compute name for bridge targets.

Index: classpath/tools/gnu/classpath/tools/javah/ClassWrapper.java
===================================================================
--- classpath/tools/gnu/classpath/tools/javah/ClassWrapper.java	(revision 123959)
+++ classpath/tools/gnu/classpath/tools/javah/ClassWrapper.java	(working copy)
@@ -1,5 +1,5 @@
 /* ClassWrapper.java - wrap ASM class objects
- Copyright (C) 2006 Free Software Foundation, Inc.
+ Copyright (C) 2006, 2007 Free Software Foundation, Inc.
 
  This file is part of GNU Classpath.
 
@@ -43,6 +43,7 @@
 import java.io.PrintStream;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 
@@ -68,6 +69,11 @@
   // A set of all the method names in this class.
   HashSet methodNames = new HashSet();
 
+  // This maps a method name + descriptor, e.g. "method()V", to the
+  // name chosen for the method.  This is used when computing the
+  // names of bridge method targets.
+  HashMap methodNameMap = new HashMap();
+
   public ClassWrapper(Main classpath)
   {
     this.classpath = classpath;
@@ -187,18 +193,24 @@
         superClass.makeVtable();
         vtable = new ArrayList(superClass.vtable);
         bridgeTargets = new HashSet(superClass.bridgeTargets);
+	methodNameMap = new HashMap(superClass.methodNameMap);
       }
     else
       {
         // Object.
         vtable = new ArrayList();
         bridgeTargets = new HashSet();
+	methodNameMap = new HashMap();
       }
     addLocalMethods();
     addInterfaces(this);
 
-    // Make a set of all the targets of bridge methods.
-    // We rename bridge methods to avoid problems with C++.
+    // Make a set of all the targets of bridge methods.  We rename
+    // bridge target methods to avoid problems with C++.  You might
+    // think we could rename the bridge methods themselves, but bridge
+    // methods by definition override a method from the superclass --
+    // and we have to consider the superclass' header as an
+    // unchangeable entity.
     Iterator i = methods.iterator();
     while (i.hasNext())
       {
@@ -232,8 +244,25 @@
     while (i.hasNext())
       {
         MethodNode m = (MethodNode) i.next();
-        boolean isTarget = bridgeTargets.contains(m.name + m.desc);
-        MethodHelper.print(out, m, this, isTarget);
+	String nameToUse;
+	String sum = m.name + m.desc;
+	if (bridgeTargets.contains(sum))
+	  {
+	    if (methodNameMap.containsKey(sum))
+	      nameToUse = (String) methodNameMap.get(sum);
+	    else
+	      {
+		// Bridge target that is new in this class.
+		String cname = this.name;
+		int index = cname.lastIndexOf('/');
+		cname = cname.substring(index + 1);
+		nameToUse = cname + "$" + m.name;
+	      }
+	  }
+	else
+	  nameToUse = Keywords.getCxxName(m.name);
+	methodNameMap.put(sum, nameToUse);
+        MethodHelper.print(out, m, this, nameToUse);
       }
   }
 
Index: classpath/tools/gnu/classpath/tools/javah/MethodHelper.java
===================================================================
--- classpath/tools/gnu/classpath/tools/javah/MethodHelper.java	(revision 123959)
+++ classpath/tools/gnu/classpath/tools/javah/MethodHelper.java	(working copy)
@@ -1,5 +1,5 @@
 /* MethodHelper.java - helper class for manipulating methods
- Copyright (C) 2006 Free Software Foundation, Inc.
+ Copyright (C) 2006, 2007 Free Software Foundation, Inc.
 
  This file is part of GNU Classpath.
 
@@ -76,7 +76,7 @@
   }
 
   public static void print(CniPrintStream out, MethodNode meth,
-                           ClassWrapper declarer, boolean isBridgeTarget)
+                           ClassWrapper declarer, String realMethodName)
   {
     if ("<clinit>".equals(meth.name))
       return;
@@ -97,15 +97,7 @@
       {
         out.print(Type.getReturnType(meth.desc));
         out.print(" ");
-        if (isBridgeTarget)
-          {
-            out.print("target$");
-            out.print(meth.name);
-          }
-        else
-          {
-            out.print(Keywords.getCxxName(meth.name));
-          }
+	out.print(realMethodName);
       }
     else
       {