This is the mail archive of the java@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: PR 1208


Tom Tromey <tromey@redhat.com> writes:

> I already have a partial patch for this problem.  Alex is working on
> finishing it.  I believe this is the same problem as PR 2216.
> 
> I've appended the incomplete patch.  This patch gets pretty close, but
> it causes some other failures.

I;ve been working on my own patch (see below).  I've tested it on
the NullPointerTest, plus methods with empty finally clause or
empty try clause.

> +	* parse.y (try_statement): Always create a TRY_BLOCK.

Why?  That seems like the wrong thing to do to me.

> @@ -2342,7 +2342,9 @@
>  
>  	/* If the finally clause happens to be empty, set a flag so we
>             remember to just skip it. */
> -	if (BLOCK_EXPR_BODY (finally) == empty_stmt_node)
> +	if ((TREE_CODE (finally) == BLOCK
> +	     && BLOCK_EXPR_BODY (finally) == empty_stmt_node)
> +	    || TREE_CODE (finally) == NOP_EXPR)
>  	  worthwhile_finally = 0;
>  
>  	if (worthwhile_finally)

I think the test for NOP_EXPR is wrong - what if the NOP_EXPR is
wrapping a statement with side-effects?  Perhaps we don't generate
those, but it seems fragile.  Instead, my patch just tests for
'finally == empty_stmt_node'.  If there are any other variants
of "empty statement", we should fix the front-end to optimize those
to empty_stmt_node.  That is the convention in jc1.

> +	    if (BLOCK_EXPR_BODY (TREE_OPERAND (try_block, 0))
> +		!= empty_stmt_node)
> +	      emit_goto (finished_label, state);
>  	  }

I think this is wrong (but it has been wrong since last year).
Even if the try_block is empty, we still have to go to the
statement following the try-finally.

Here is my attempt.  I'll try it on the testsuite.

Index: jcf-write.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/jcf-write.c,v
retrieving revision 1.72.2.1
diff -u -p -r1.72.2.1 jcf-write.c
--- jcf-write.c	2001/03/17 21:53:11	1.72.2.1
+++ jcf-write.c	2001/03/21 01:28:09
@@ -111,7 +111,7 @@ struct chunk
 struct jcf_block
 {
   /* For blocks that that are defined, the next block (in pc order).
-     For blocks that are the not-yet-defined end label of a LABELED_BLOCK_EXPR
+     For blocks that are not-yet-defined the end label of a LABELED_BLOCK_EXPR
      or a cleanup expression (from a WITH_CLEANUP_EXPR),
      this is the next (outer) such end label, in a stack headed by
      labeled_blocks in jcf_partial. */
@@ -131,8 +131,8 @@ struct jcf_block
 
   int linenumber;
 
-  /* After finish_jcf_block is called, The actual instructions
-     contained in this block.  Before than NULL, and the instructions
+  /* After finish_jcf_block is called, the actual instructions
+     contained in this block.  Before that NULL, and the instructions
      are in state->bytecode. */
   union {
     struct chunk *chunk;
@@ -349,7 +349,7 @@ static void append_innerclasses_attribut
 
 /* Utility macros for appending (big-endian) data to a buffer.
    We assume a local variable 'ptr' points into where we want to
-   write next, and we assume enoygh space has been allocated. */
+   write next, and we assume enough space has been allocated. */
 
 #ifdef ENABLE_JC1_CHECKING
 static int CHECK_PUT PARAMS ((void *, struct jcf_partial *, int));
@@ -2331,21 +2331,21 @@ generate_bytecode_insns (exp, target, st
       break;
     case TRY_FINALLY_EXPR:
       {
-	struct jcf_block *finished_label, *finally_label, *start_label;
+	struct jcf_block *finished_label,
+	  *finally_label = NULL, *start_label = NULL;
 	struct jcf_handler *handler;
-	int worthwhile_finally = 1;
 	tree try_block = TREE_OPERAND (exp, 0);
 	tree finally = TREE_OPERAND (exp, 1);
-	tree return_link, exception_decl;
-
-	finally_label = start_label = NULL;
-	return_link = exception_decl = NULL_TREE;
-	finished_label = gen_jcf_label (state);
-
 	/* If the finally clause happens to be empty, set a flag so we
            remember to just skip it. */
-	if (BLOCK_EXPR_BODY (finally) == empty_stmt_node)
-	  worthwhile_finally = 0;
+	int worthwhile_finally = finally != empty_stmt_node;
+	tree return_link = NULL_TREE, exception_decl = NULL_TREE;
+
+	if (try_block == empty_stmt_node)
+	  {
+	    generate_bytecode_insns (finally, IGNORE_TARGET, state);
+	    break;
+	  }
 
 	if (worthwhile_finally)
 	  {
@@ -2365,24 +2365,23 @@ generate_bytecode_insns (exp, target, st
 
 	generate_bytecode_insns (try_block, target, state);
 
-	if (worthwhile_finally)
+	if (! worthwhile_finally)
+	  break;
+
+	finished_label = gen_jcf_label (state);
+
+	if (state->labeled_blocks != finally_label)
+	  abort();
+	state->labeled_blocks = finally_label->next;
+
+	if (CAN_COMPLETE_NORMALLY (try_block))
 	  {
-	    if (state->labeled_blocks != finally_label)
-	      abort();
-	    state->labeled_blocks = finally_label->next;
 	    emit_jsr (finally_label, state);
+	    emit_goto (finished_label, state);
 	  }
 
-	if (CAN_COMPLETE_NORMALLY (try_block)
-	    && TREE_CODE (try_block) == BLOCK
-	    && BLOCK_EXPR_BODY (try_block) != empty_stmt_node)
-	  emit_goto (finished_label, state);
-
 	/* Handle exceptions. */
 
-	if (!worthwhile_finally)
-	  break;
-
 	localvar_alloc (return_link, state);
 	handler = alloc_handler (start_label, NULL_PTR, state);
 	handler->end_label = handler->handler_label;
@@ -2395,7 +2394,6 @@ generate_bytecode_insns (exp, target, st
 	RESERVE (1);
 	OP1 (OPCODE_athrow);
 	NOTE_POP (1);
-	localvar_free (exception_decl, state);
 
 	/* The finally block.  First save return PC into return_link. */
 	define_jcf_label (finally_label, state);
@@ -2404,6 +2402,7 @@ generate_bytecode_insns (exp, target, st
 
 	generate_bytecode_insns (finally, IGNORE_TARGET, state);
 	maybe_wide (OPCODE_ret, DECL_LOCAL_INDEX (return_link), state);
+	localvar_free (exception_decl, state);
 	localvar_free (return_link, state);
 	define_jcf_label (finished_label, state);
       }

-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/~per/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]