[BC] Patch: FYI: verifier -vs- uninitialized types

[Tom Tromey <tromey at redhat dot com>]

I'm checking this in on the BC branch.

The JVM Spec says that it is invalid to have an uninitialized class
instance on the stack or in a local variable at a backwards branch or
in a region protected by an exception handler.

However, this constraint is not actually needed for type safety.  See
the Coglio paper for a full treatment.  The short form is that, since
we check at `return' type during <init> to make sure that `this' was
initialized, and since we don't allow operations (merges, assignments,
calls) on uninitialized types, the fact that they might be moved
around the stack and locals in strange ways is of no concern.

As a practical matter this is important since the Eclipse java
compiler generates code that violates this constraint.  This is filed
in Eclipse bugzilla as:

  https://bugs.eclipse.org/bugs/show_bug.cgi?id=77219

This patch fixes the bug in parallel in both verifiers.

Tom

Index: gcc/java/ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	* verify-impl.c (push_jump): Removed check for uninitialized
	objects.
	(push_exception_jump): Likewise.
	(handle_ret_insn): Likewise.
	(handle_jsr_insn): Likewise.
	(state_check_no_uninitialized_objects): Removed.

Index: gcc/java/verify-impl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/Attic/verify-impl.c,v
retrieving revision 1.1.2.15
diff -u -r1.1.2.15 verify-impl.c
--- gcc/java/verify-impl.c 26 Oct 2004 19:07:42 -0000 1.1.2.15
+++ gcc/java/verify-impl.c 1 Nov 2004 18:27:23 -0000
@@ -103,13 +103,15 @@
    subroutine is exited via `goto' or `athrow' and not `ret'.
 
    In some other areas the JVM specification is (mildly) incorrect,
-   but we still implement what is specified.  For instance, you cannot
+   so we diverge.  For instance, you cannot
    violate type safety by allocating an object with `new' and then
    failing to initialize it, no matter how one branches or where one
    stores the uninitialized reference.  See "Improving the official
    specification of Java bytecode verification" by Alessandro Coglio.
-   Similarly, there's no real point in enforcing that padding bytes or
-   the mystery byte of invokeinterface must be 0, but we do that too.
+
+   Note that there's no real point in enforcing that padding bytes or
+   the mystery byte of invokeinterface must be 0, but we do that
+   regardless.
 
    The verifier is currently neither completely lazy nor eager when it
    comes to loading classes.  It tries to represent types by name when
@@ -1203,31 +1205,6 @@
     verify_fail ("`this' is uninitialized");
 }
 
-/* Throw an exception if there is an uninitialized object on the
-   stack or in a local variable.  EXCEPTION_SEMANTICS controls
-   whether we're using backwards-branch or exception-handing
-   semantics.  */
-static void 
-state_check_no_uninitialized_objects (state *s, int max_locals,
-				      bool exception_semantics)
-{
-  int i;
-  if (! exception_semantics)
-    {
-      for (i = 0; i < s->stacktop; ++i)
-	if (type_isreference (&s->stack[i]) 
-	    && ! type_initialized (&s->stack[i]))
-	  verify_fail ("uninitialized object on stack");
-    }
-
-  for (i = 0; i < max_locals; ++i)
-    if (type_isreference (&s->locals[i])
-	&& ! type_initialized (&s->locals[i]))
-      verify_fail ("uninitialized object in local variable");
-
-  state_check_this_initialized (s);
-}
-
 /* Set type of `this'.  */
 static void
 state_set_this_type (state *s, type *k)
@@ -1619,9 +1596,10 @@
 push_jump (int offset)
 {
   int npc = compute_jump (offset);
-  if (npc < vfr->PC)
-    state_check_no_uninitialized_objects (vfr->current_state, 
-				    vfr->current_method->max_locals, false);
+  /* According to the JVM Spec, we need to check for uninitialized
+     objects here.  However, this does not actually affect type
+     safety, and the Eclipse java compiler generates code that
+     violates this constraint.  */
   merge_into (npc, vfr->current_state);
 }
 
@@ -1629,8 +1607,10 @@
 push_exception_jump (type t, int pc)
 {
   state s;
-  state_check_no_uninitialized_objects (vfr->current_state,
-					vfr->current_method->max_locals, true);
+  /* According to the JVM Spec, we need to check for uninitialized
+     objects here.  However, this does not actually affect type
+     safety, and the Eclipse java compiler generates code that
+     violates this constraint.  */
   copy_state_with_stack (&s, vfr->current_state, 
 			 vfr->current_method->max_stack,
 			 vfr->current_method->max_locals);
@@ -1700,9 +1680,10 @@
   if (npc >= vfr->current_method->code_length)
     verify_fail ("fell off end");
 
-  if (npc < vfr->PC)
-    state_check_no_uninitialized_objects (vfr->current_state, 
-      vfr->current_method->max_locals, false);
+  /* According to the JVM Spec, we need to check for uninitialized
+     objects here.  However, this does not actually affect type
+     safety, and the Eclipse java compiler generates code that
+     violates this constraint.  */
   merge_into (npc, vfr->current_state);
   invalidate_pc ();
 }
@@ -1712,10 +1693,10 @@
   type ret_addr;
   int npc = compute_jump (offset);
 
-  if (npc < vfr->PC)
-    state_check_no_uninitialized_objects (vfr->current_state,
-					  vfr->current_method->max_locals,
-					  false);
+  /* According to the JVM Spec, we need to check for uninitialized
+     objects here.  However, this does not actually affect type
+     safety, and the Eclipse java compiler generates code that
+     violates this constraint.  */
 
   /* Modify our state as appropriate for entry into a subroutine.  */
   ret_addr = make_type (return_address_type);
Index: libjava/ChangeLog
from  Tom Tromey  <tromey@redhat.com>

	* verify.cc (state::check_no_uninitialized_objects): Removed.
	(push_jump): Updated.
	(push_exception_jump): Likewise.
	(handle_ret_insn): Likewise.
	(handle_jsr_insn): Likewise.

Index: libjava/verify.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/verify.cc,v
retrieving revision 1.62.2.2
diff -u -r1.62.2.2 verify.cc
--- libjava/verify.cc 12 Oct 2004 12:32:29 -0000 1.62.2.2
+++ libjava/verify.cc 1 Nov 2004 18:27:27 -0000
@@ -100,13 +100,15 @@
 // subroutine is exited via `goto' or `athrow' and not `ret'.
 //
 // In some other areas the JVM specification is (mildly) incorrect,
-// but we still implement what is specified.  For instance, you cannot
+// so we diverge.  For instance, you cannot
 // violate type safety by allocating an object with `new' and then
 // failing to initialize it, no matter how one branches or where one
 // stores the uninitialized reference.  See "Improving the official
 // specification of Java bytecode verification" by Alessandro Coglio.
-// Similarly, there's no real point in enforcing that padding bytes or
-// the mystery byte of invokeinterface must be 0, but we do that too.
+//
+// Note that there's no real point in enforcing that padding bytes or
+// the mystery byte of invokeinterface must be 0, but we do that
+// regardless.
 //
 // The verifier is currently neither completely lazy nor eager when it
 // comes to loading classes.  It tries to represent types by name when
@@ -1098,28 +1100,6 @@
       return changed;
     }
 
-    // Throw an exception if there is an uninitialized object on the
-    // stack or in a local variable.  EXCEPTION_SEMANTICS controls
-    // whether we're using backwards-branch or exception-handing
-    // semantics.
-    void check_no_uninitialized_objects (int max_locals,
-					 _Jv_BytecodeVerifier *verifier,
-					 bool exception_semantics = false)
-    {
-      if (! exception_semantics)
-	{
-	  for (int i = 0; i < stacktop; ++i)
-	    if (stack[i].isreference () && ! stack[i].isinitialized ())
-	      verifier->verify_fail ("uninitialized object on stack");
-	}
-
-      for (int i = 0; i < max_locals; ++i)
-	if (locals[i].isreference () && ! locals[i].isinitialized ())
-	  verifier->verify_fail ("uninitialized object in local variable");
-
-      check_this_initialized (verifier);
-    }
-
     // Ensure that `this' has been initialized.
     void check_this_initialized (_Jv_BytecodeVerifier *verifier)
     {
@@ -1434,15 +1414,19 @@
   void push_jump (int offset)
   {
     int npc = compute_jump (offset);
-    if (npc < PC)
-      current_state->check_no_uninitialized_objects (current_method->max_locals, this);
+    // According to the JVM Spec, we need to check for uninitialized
+    // objects here.  However, this does not actually affect type
+    // safety, and the Eclipse java compiler generates code that
+    // violates this constraint.
     merge_into (npc, current_state);
   }
 
   void push_exception_jump (type t, int pc)
   {
-    current_state->check_no_uninitialized_objects (current_method->max_locals,
-						   this, true);
+    // According to the JVM Spec, we need to check for uninitialized
+    // objects here.  However, this does not actually affect type
+    // safety, and the Eclipse java compiler generates code that
+    // violates this constraint.
     state s (current_state, current_method->max_stack,
 	     current_method->max_locals);
     if (current_method->max_stack < 1)
@@ -1504,9 +1488,10 @@
     if (npc >= current_method->code_length)
       verify_fail ("fell off end");
 
-    if (npc < PC)
-      current_state->check_no_uninitialized_objects (current_method->max_locals,
-						     this);
+    // According to the JVM Spec, we need to check for uninitialized
+    // objects here.  However, this does not actually affect type
+    // safety, and the Eclipse java compiler generates code that
+    // violates this constraint.
     merge_into (npc, current_state);
     invalidate_pc ();
   }
@@ -1515,8 +1500,10 @@
   {
     int npc = compute_jump (offset);
 
-    if (npc < PC)
-      current_state->check_no_uninitialized_objects (current_method->max_locals, this);
+    // According to the JVM Spec, we need to check for uninitialized
+    // objects here.  However, this does not actually affect type
+    // safety, and the Eclipse java compiler generates code that
+    // violates this constraint.
 
     // Modify our state as appropriate for entry into a subroutine.
     type ret_addr (return_address_type);