Patch: RFC: PR 7482

[Tom Tromey <tromey at redhat dot com>]

This patch fixes PR 7482.

It turns out the verifier missed an odd case, where a method returns
an interface type but the actual types on the stack have been merged
to Object.

The simplest fix, implemented here, is to let this case slide by and
then check at runtime.

The long term fix is to change how the verifier represents types.
While possible (in theory I built the verifier so that this change can
be made at some later date without too many problems), it is still a
lot of work and I'm not planning to do it.

Before checking this in, though, I wanted to ask what we should do
about _Jv_InterpMethod::run modifying the _Jv_InterpMethod object.
Right now we already do this in a thread-unsafe way (see PR 7587) for
the `prepared' field.  And this patch introduces a similar problem for
the `return_type' field.

Obviously this isn't acceptable.  But how to fix it?  I'd prefer not
to put a lock in the run method.  I don't see how we could do these
modifications in defineclass.cc, since that might force new classes to
be loaded and initialized.

Perhaps the lock is the only answer.  Or, more hopefully, I'm missing
something obvious.  Any suggestions?

Tom

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

	For PR java/7482.
	* defineclass.cc (handleCodeAttribute): Initialize `return_type'.
	* include/java-interp.h (class _Jv_InterpMethod) [return_type]:
	New field.
	* interpret.cc (run): Initalize return_type.
	[insn_areturn]: Check return value if return type is an
	interface.
	* verify.cc (_Jv_BytecodeVerifier::check_return_type): Add special
	case for interfaces merged to Object.

Index: defineclass.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/defineclass.cc,v
retrieving revision 1.29
diff -u -r1.29 defineclass.cc
--- defineclass.cc 24 Jun 2002 20:38:45 -0000 1.29
+++ defineclass.cc 13 Aug 2002 17:11:50 -0000
@@ -1259,8 +1259,9 @@
   method->defining_class = def;
   method->self           = &def->methods[method_index];
   method->prepared       = NULL;
+  method->return_type    = NULL;
 
-  // grab the byte code!
+  // Grab the byte code!
   memcpy ((void*) method->bytecode (),
 	  (void*) (bytes+code_start),
 	  code_length);
Index: interpret.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/interpret.cc,v
retrieving revision 1.33
diff -u -r1.33 interpret.cc
--- interpret.cc 24 Jun 2002 20:38:45 -0000 1.33
+++ interpret.cc 13 Aug 2002 17:11:51 -0000
@@ -36,6 +36,7 @@
 #include <java/lang/NullPointerException.h>
 #include <java/lang/ArithmeticException.h>
 #include <java/lang/IncompatibleClassChangeError.h>
+#include <java/lang/VerifyError.h>
 #include <java-insns.h>
 #include <java-signal.h>
 
@@ -1025,6 +1026,17 @@
 
 #endif /* DIRECT_THREADED */
 
+  // FIXME: thread safety.
+  if (return_type == NULL)
+    {
+      unsigned char *ptr = (unsigned char *) self->signature->data;
+      unsigned char *limit = ptr + self->signature->length;
+      while (UTF8_GET (ptr, limit) != ')')
+	;
+      // FIXME: class loader.
+      return_type = _Jv_FindClassFromSignature ((char *) ptr, NULL);
+    }
+
 #define TAKE_GOTO pc = GOTO_VAL ()
 
   try
@@ -2334,7 +2346,17 @@
       NEXT_INSN;
 
     insn_areturn:
-      *(jobject *) retp = POPA ();
+      jobject val = POPA ();
+      // Due to the type representation of our verifier, we can't
+      // always correctly detect type violations in areturn ahead of
+      // time.  So here we detect them in the one case that matters:
+      // when the method's declared return type is an interface.  (We
+      // could mark each `areturn' needing this special case, if we
+      // cared to.)
+      if (return_type->isInterface ()
+	  && ! return_type->isInstance (val))
+	throw new java::lang::VerifyError (JvNewStringLatin1 ("invalid actual return type"));
+      *(jobject *) retp = val;
       return;
 
     insn_lreturn:
Index: verify.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/verify.cc,v
retrieving revision 1.42
diff -u -r1.42 verify.cc
--- verify.cc 24 Jun 2002 20:38:45 -0000 1.42
+++ verify.cc 13 Aug 2002 17:11:53 -0000
@@ -239,7 +239,7 @@
 		// We use a recursive call because we also need to
 		// check superinterfaces.
 		if (is_assignable_from_slow (target, source->interfaces[i]))
-		    return true;
+		  return true;
 	      }
 	    source = source->getSuperclass ();
 	    if (source == NULL)
@@ -2054,7 +2054,19 @@
   void check_return_type (type onstack)
   {
     type rt = compute_return_type (current_method->self->signature);
-    if (! rt.compatible (onstack, this))
+
+    bool ok = rt.compatible (onstack, this);
+    if (! ok && rt.isinterface (this))
+      {
+	// After merging, the type on the stack might be Object even
+	// though the actual value implements our interface in both
+	// original code paths.  In this case we accept Object and do
+	// more checking at runtime.
+	type obj (&java::lang::Object::class$);
+	ok = onstack.compatible (obj, this);
+      }
+
+    if (! ok)
       verify_fail ("incompatible return type");
   }
 
Index: include/java-interp.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/java-interp.h,v
retrieving revision 1.17
diff -u -r1.17 java-interp.h
--- include/java-interp.h 24 Jun 2002 20:38:46 -0000 1.17
+++ include/java-interp.h 13 Aug 2002 17:11:53 -0000
@@ -100,6 +100,11 @@
 
   _Jv_ushort       exc_count;
 
+  // The return type of this method.
+  jclass           return_type;
+
+  // If the interpreter is direct-threaded, this holds the "compiled"
+  // bytecode.
   void *prepared;
 
   unsigned char* bytecode ()