Bug in Method.invoke() / Patch

[enno at convergence.de]

Hi Godmar,

thanks for your coment. Sounds convincing not to mess with m->idx
afterwards, even if I don't know of what use this field is in the case
of methods of an interface: there is no fixed index into any
class' dispatching table we could put in there, or is my understanding
of the internal structures wrong here?

Nevertheless, I'd appreciate your comment on a second attempt, because
I'd really like to have this bug fixed (maybe you find the time to
glance over it and comment it.  Hopefully, my suggestion it not too
dumb - I'm still trying to get a deeper insight into the internal
structures of the kaffe vm: So, changing m->idx in Method.invoke() was
error-prone, but couldn't we just locate the right method (out of the
target object's class/superclasses, in the same fashion as
getInheritedMethodIndex() finds the right index) to pass down the
calling machinery instead of the one we get out of the method table of
the interface (which is not associated to any code anyway).  

To do that I used the following code snippet (patch attached):

	        if( CLASS_IS_INTERFACE( clazz ) ) {
		  Hjava_lang_Class* objClazz = (*env)->GetObjectClass(env, obj);
		  errorInfo info;
		  Method* realmeth;
		  if( ! instanceof( clazz, objClazz ) ) {
		    SignalError("java.lang.IllegalArgumentException", "object is not an instance of declaring class");
		  }

		  realmeth = findMethod(objClazz, meth->name, METHOD_SIG(meth), &info);
		  if (realmeth == 0) {
		    throwError(&info);
		  }
		  meth = realmeth;
		}

This doesn't change any method->idx ... and in addition it also checks
if the object actually implements the interface (this didn't happen at
all before). But even if this works, I think it's not the fix that's
really appropriate: in soft.c there's a function
soft_lookupinterfacemethod(...), which more or less does what we want
(including the instanceof-check). this function uses what i guess is
a class internal mapping from interface method indices to
(implementing) class method (member itable2dtable in struct
Hjava_lang_Class), which should be faster, but I don't know how to
exploit this function here, because it doesn't return the method, but
the associated entry in the dispatching table. so, my patch might not
be the most efficient one, but do you think it does what it's supposed
to do?

regards

Enno




On Wed, Jul 25, 2001 at 11:21:34AM -0600, Godmar Back wrote:
> 
> 
>  Hi Enno,
> 
> thanks for your report.
> 
> > 
> > Attached is a patch, which to my opinion should fix this (using
> > getInheritedMethodIndex() in classMethod.c), but I would appreciate
> > if somebody more familiar with this code could check my explanation/fix.
> > 
> 
> I agree with your explanation, but I have doubts about the fix.  
> classMethod.c:getInheritedMethodIndex() should probably be static 
> anyway, and we mustn't tweak m->idx after a class was loaded.
> 
> I think we're screwing ourselves when we set meth->slot
> in Class.c:makeMethod in the case where CLASS_IS_INTERFACE(meth->class).
> I haven't thought through what the best fix might be (probably handle
> the case where CLASS_IS_INTERFACE(meth->class) separately in 
> Method.c:invoke0(), but I don't have the time right now to do that.
> 
> 	- Godmar
> 

--
Enno Brehm
enno at convergence.de
( convergence           )
( integrated media gmbh )
-------------- next part --------------
Index: Method.c
===================================================================
RCS file: /cvs/kaffe/kaffe/libraries/clib/native/Method.c,v
retrieving revision 1.30
diff -u -r1.30 Method.c
--- Method.c	2000/12/03 03:25:09	1.30
+++ Method.c	2001/07/27 12:39:35
@@ -33,7 +33,6 @@
 #include <native.h>
 #include "jni.h"
 #include "defs.h"
-
 jint
 java_lang_reflect_Method_getModifiers(struct Hjava_lang_reflect_Method* this)
 {
@@ -281,6 +280,21 @@
 		rettype = 'L';
 	}
 	else {			/* nonstatic method */
+	        if( CLASS_IS_INTERFACE( clazz ) ) {
+		  Hjava_lang_Class* objClazz = (*env)->GetObjectClass(env, obj);
+		  errorInfo info;
+		  Method* realmeth;
+		  if( ! instanceof( clazz, objClazz ) ) {
+		    SignalError("java.lang.IllegalArgumentException", "object is not an instance of declaring class!");
+		  }
+
+		  realmeth = findMethod(objClazz, meth->name, METHOD_SIG(meth), &info);
+		  if (realmeth == 0) {
+		    throwError(&info);
+		  }
+		  meth = realmeth;
+		}
+
 		switch (rettype) {
 
 		/* Why Call<Type>MethodA and not CallNonvirtual<Type>MethodA?