Patch: Java: null `this' -vs- final methods

[Tom Tromey <tromey at cygnus dot com>]

This patch partially fixes PR gcj/2.  It only works when the target
has an MMU and signal handling support in libgcj.  This is no worse
than what we do now, and is better for users of native platforms.

This works by inserting a piece of code to dereference the method's
`this' before making a method call on a final or private method.  We
skip the check if the called method's `this' is the same as the
caller's `this'.

I rebuilt libgcj from scratch with this patch.  `make check' seems in
line.  The `Final' test does work with this patch, both with the
native compiler and from bytecodes.  I also tried it from source with
-O3 and it worked.

I think we could do better than this.  AG suggested doing something
like what we do for class initialization -- introduce a new variable
per object (that requires this treatment) and track whether we've
dereferenced it.  I'm not going to do this.  (I could submit a PR if
you think that is worthwhile.)

Ok to check in?

2000-03-30  Tom Tromey  <tromey@cygnus.com>

	* expr.c (expand_invoke): Generate a "volatile" `this' dereference
	if the call is to a final method.
	* parse.y (invocation_mode): Return INVOKE_NONVIRTUAL for `final'
	or `private' methods.
	(patch_invoke): Handle INVOKE_NONVIRTUAL.

Tom

Index: expr.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/java/expr.c,v
retrieving revision 1.70
diff -u -r1.70 expr.c
--- expr.c	2000/03/30 23:41:57	1.70
+++ expr.c	2000/03/31 18:24:19
@@ -1719,6 +1719,7 @@
     (current_jcf, COMPONENT_REF_CLASS_INDEX(&current_jcf->cpool, method_ref_index));
   const char *self_name = IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (self_type)));
   tree call, func, method, arg_list, method_type;
+  tree deref = NULL_TREE;
 
   if (! CLASS_LOADED_P (self_type))
     {
@@ -1781,13 +1782,30 @@
   flush_quick_stack ();
 
   func = NULL_TREE;
-  if (opcode == OPCODE_invokestatic || opcode == OPCODE_invokespecial
-      || (opcode == OPCODE_invokevirtual
-	  && (METHOD_PRIVATE (method)
-	      || METHOD_FINAL (method) 
-	      || CLASS_FINAL (TYPE_NAME (self_type)))))
+  if (opcode == OPCODE_invokestatic)
     func = build_known_method_ref (method, method_type, self_type,
 				   method_signature, arg_list);
+  else if (opcode == OPCODE_invokespecial
+	   || (opcode == OPCODE_invokevirtual
+	       && (METHOD_PRIVATE (method)
+		   || METHOD_FINAL (method) 
+		   || CLASS_FINAL (TYPE_NAME (self_type)))))
+    {
+      /* We dereference the new `this' pointer in order to provoke a
+	 trap, and thus a NullPointerException, when invoking a method
+	 which doesn't require an actual lookup via the vtable.
+	 However, we don't have to do this if the `this' operand to
+	 the call is the same as the current `this'.  FIXME:
+	 unfortunately, the byte compiler doesn't seem to keep track
+	 of this information and so we can't tell whether this test is
+	 optional.  */
+      deref = build1 (INDIRECT_REF,
+		      ptr_type_node,
+		      TREE_VALUE (arg_list));
+      TREE_SIDE_EFFECTS (deref) = TREE_THIS_VOLATILE (deref) = 1;
+      func = build_known_method_ref (method, method_type, self_type,
+				     method_signature, arg_list);
+    }
   else
     {
       tree dtable = invoke_build_dtable (opcode == OPCODE_invokeinterface, 
@@ -1800,6 +1818,12 @@
   func = build1 (NOP_EXPR, build_pointer_type (method_type), func);
   call = build (CALL_EXPR, TREE_TYPE (method_type), func, arg_list, NULL_TREE);
   TREE_SIDE_EFFECTS (call) = 1;
+
+  if (deref != NULL_TREE)
+    {
+      call = build (COMPOUND_EXPR, TREE_TYPE (call), deref, call);
+      TREE_SIDE_EFFECTS (call) = 1;
+    }
 
   if (TREE_CODE (TREE_TYPE (method_type)) == VOID_TYPE)
     expand_expr_stmt (call);
Index: parse.y
===================================================================
RCS file: /cvs/gcc/egcs/gcc/java/parse.y,v
retrieving revision 1.146
diff -u -r1.146 parse.y
--- parse.y	2000/03/23 07:01:24	1.146
+++ parse.y	2000/03/31 18:25:01
@@ -9625,6 +9625,7 @@
 {
   tree dtable, func;
   tree original_call, t, ta;
+  tree deref = NULL_TREE;
 
   /* Last step for args: convert build-in types. If we're dealing with
      a new TYPE() type call, the first argument to the constructor
@@ -9656,6 +9657,21 @@
 	  func = build_invokevirtual (dtable, method);
 	  break;
 
+	case INVOKE_NONVIRTUAL:
+	  /* We dereference the new `this' pointer in order to provoke
+	     a trap, and thus a NullPointerException, when invoking a
+	     method which doesn't require an actual lookup via the
+	     vtable.  However, we don't have to do this if the `this'
+	     operand to the call is the same as the current `this'.  */
+	  if (TREE_VALUE (args) != current_this)
+	    {
+	      deref = build1 (INDIRECT_REF,
+			      ptr_type_node,
+			      TREE_VALUE (args));
+	      TREE_SIDE_EFFECTS (deref) = TREE_THIS_VOLATILE (deref) = 1;
+	    }
+	  /* Fall through.  */
+
 	case INVOKE_SUPER:
 	case INVOKE_STATIC:
 	  func = build_known_method_ref (method, TREE_TYPE (method),
@@ -9711,6 +9727,13 @@
       TREE_SET_CODE (original_call, CALL_EXPR);
       patch = build (COMPOUND_EXPR, TREE_TYPE (new), patch, saved_new);
     }
+
+  if (deref != NULL_TREE)
+    {
+      patch = build (COMPOUND_EXPR, TREE_TYPE (patch), deref, patch);
+      TREE_SIDE_EFFECTS (patch) = 1;
+    }
+
   return patch;
 }
 
@@ -9724,17 +9747,22 @@
   if (super)
     return INVOKE_SUPER;
 
-  if (access & ACC_STATIC || access & ACC_FINAL || access & ACC_PRIVATE)
+  if (access & ACC_STATIC)
     return INVOKE_STATIC;
 
-  if (CLASS_FINAL (TYPE_NAME (DECL_CONTEXT (method))))
+  /* We need to check for a constructor before checking for a
+     nonvirtual method invocation.  */
+  if (DECL_CONSTRUCTOR_P (method))
     return INVOKE_STATIC;
-  
+
+  if (access & ACC_FINAL || access & ACC_PRIVATE)
+    return INVOKE_NONVIRTUAL;
+
+  if (CLASS_FINAL (TYPE_NAME (DECL_CONTEXT (method))))
+    return INVOKE_NONVIRTUAL;
+
   if (CLASS_INTERFACE (TYPE_NAME (DECL_CONTEXT (method))))
     return INVOKE_INTERFACE;
-  
-  if (DECL_CONSTRUCTOR_P (method))
-    return INVOKE_STATIC;
 
   return INVOKE_VIRTUAL;
 }