[gcjx] Patch: FYI: cleanup bug in bytecode generation

[Tom Tromey <tromey at redhat dot com>]

I'm checking this in on the gcjx branch.

Dalibor noted that for a certain piece of synchronized code we were
incorrectly calling monitorexit twice.  The problem was that
call_cleanups wouldn't know when to stop when searching for the
statement we were leaving.

This fixes the problem.  It also adds an assert to ensure it doesn't
happen again.

Tom

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

	* bytecode/generate.hh (finally_handler): New constructor.
	* bytecode/generate.cc (call_cleanups): Added assert.  Exit early
	if dummy entry found.
	(visit_break): Pass correct target to call_cleanups.
	(visit_do): Push a dummy finally handler.
	(visit_for_enhanced): Likewise.
	(visit_for): Likewise.
	(visit_label): Likewise.
	(visit_switch): Likewise.
	(visit_while): Likewise.

Index: bytecode/generate.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/bytecode/Attic/generate.cc,v
retrieving revision 1.1.2.20
diff -u -r1.1.2.20 generate.cc
--- bytecode/generate.cc 12 Sep 2005 01:38:23 -0000 1.1.2.20
+++ bytecode/generate.cc 14 Sep 2005 03:53:49 -0000
@@ -381,10 +381,17 @@
   while (! last_was_ours && ! finally_stack.empty ())
     {
       finally_handler h = finally_stack.back ();
+      last_was_ours = h.statement == upto;
+      // Exit early if we found a target statement.
+      if (h.statement == upto && h.variable == -2)
+	{
+	  assert (h.block == NULL);
+	  break;
+	}
       finally_stack.pop_back ();
       emit_saved_cleanup (h);
-      last_was_ours = h.statement == upto;
     }
+  assert (last_was_ours || upto == NULL);
   finally_stack = save;
 }
 
@@ -526,7 +533,7 @@
   if (! real_target)
     real_target = brk->get_target ();
 
-  call_cleanups (real_target);
+  call_cleanups (target.get ());
 
   // Find the target statement.
   target_map_type::iterator iter = target_map.find (real_target);
@@ -586,6 +593,11 @@
   bytecode_block *done (new_bytecode_block ());
   target_map[dstmt] = std::make_pair (test, done);
 
+  // Push a dummy finally handler that tells call_cleanups to exit
+  // early.
+  finally_handler dummy (dstmt);
+  stack_temporary<finally_handler> push (finally_stack, dummy);
+
   bytecode_block *top (new_bytecode_block ());
   define (top);
   body->visit (this);
@@ -631,6 +643,11 @@
 
   vars.push_scope (fstmt);
 
+  // Push a dummy finally handler that tells call_cleanups to exit
+  // early.
+  finally_handler dummy (fstmt);
+  stack_temporary<finally_handler> push (finally_stack, dummy);
+
   // We introduce a new scope so that the temporary locals are killed
   // at the right point.
   {
@@ -790,6 +807,11 @@
 {
   note_line (fstmt);
 
+  // Push a dummy finally handler that tells call_cleanups to exit
+  // early.
+  finally_handler dummy (fstmt);
+  stack_temporary<finally_handler> push (finally_stack, dummy);
+
   vars.push_scope (fstmt);
 
   if (init)
@@ -877,6 +899,11 @@
 {
   note_line (label);
 
+  // Push a dummy finally handler that tells call_cleanups to exit
+  // early.
+  finally_handler dummy (label);
+  stack_temporary<finally_handler> push (finally_stack, dummy);
+
   // We might not know the target of a break statement at semantic
   // analysis time, so we compute it here.
   if (label->get_break_target () == NULL)
@@ -946,6 +973,11 @@
 {
   note_line (swstmt);
 
+  // Push a dummy finally handler that tells call_cleanups to exit
+  // early.
+  finally_handler dummy (swstmt);
+  stack_temporary<finally_handler> push (finally_stack, dummy);
+
   bytecode_block *finish = new_bytecode_block ();
 
   // First create a destination label for each branch of the switch.
@@ -1321,6 +1353,11 @@
 {
   note_line (wstmt);
 
+  // Push a dummy finally handler that tells call_cleanups to exit
+  // early.
+  finally_handler dummy (wstmt);
+  stack_temporary<finally_handler> push (finally_stack, dummy);
+
   bytecode_block *body_bytes = new_bytecode_block ();
   bytecode_block *again = new_bytecode_block ();
   bytecode_block *done = new_bytecode_block ();
Index: bytecode/generate.hh
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/bytecode/Attic/generate.hh,v
retrieving revision 1.1.2.9
diff -u -r1.1.2.9 generate.hh
--- bytecode/generate.hh 12 Sep 2005 01:38:23 -0000 1.1.2.9
+++ bytecode/generate.hh 14 Sep 2005 03:53:49 -0000
@@ -68,6 +68,8 @@
     // finally handler represents the finally part of a 'synchronized'
     // block.  This can be NULL for an ordinary try statement if it
     // has no finally clause; in this case variable will be -1.
+    // If BLOCK is NULL and VARIABLE is -2, then this is a dummy
+    // handler representing some labeled block in the tree.
     model_block *block;
     // Variable used by the 'try'.
     int variable;
@@ -78,6 +80,13 @@
 	variable (v)
     {
     }
+
+    finally_handler (const model_stmt *s)
+      : statement (s),
+	block (NULL),
+	variable (-2)
+    {
+    }
   };
 
   // Our method.